From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id sE3PCnY4E2AoZgAA0tVLHw (envelope-from ) for ; Thu, 28 Jan 2021 22:19:34 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id mEmVBnY4E2DAaQAAB5/wlQ (envelope-from ) for ; Thu, 28 Jan 2021 22:19:34 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 84EE494042B for ; Thu, 28 Jan 2021 22:19:33 +0000 (UTC) Received: from localhost ([::1]:32906 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l5FdU-0008Ab-7V for larch@yhetil.org; Thu, 28 Jan 2021 17:19:32 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:56648) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l5Fd1-00087k-NM for guix-patches@gnu.org; Thu, 28 Jan 2021 17:19:07 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:38965) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1l5Fd0-0000vH-7S for guix-patches@gnu.org; Thu, 28 Jan 2021 17:19:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1l5Fcz-0008Q4-VO for guix-patches@gnu.org; Thu, 28 Jan 2021 17:19:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#45984] [PATCH 0/5] Fix recursive importers Resent-From: zimoun Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 28 Jan 2021 22:19:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 45984 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Received: via spool by 45984-submit@debbugs.gnu.org id=B45984.161187230132314 (code B ref 45984); Thu, 28 Jan 2021 22:19:01 +0000 Received: (at 45984) by debbugs.gnu.org; 28 Jan 2021 22:18:21 +0000 Received: from localhost ([127.0.0.1]:50509 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l5FcK-0008P7-SS for submit@debbugs.gnu.org; Thu, 28 Jan 2021 17:18:21 -0500 Received: from mail-wr1-f54.google.com ([209.85.221.54]:36098) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l5FcI-0008Ou-T5 for 45984@debbugs.gnu.org; Thu, 28 Jan 2021 17:18:19 -0500 Received: by mail-wr1-f54.google.com with SMTP id 6so6964377wri.3 for <45984@debbugs.gnu.org>; Thu, 28 Jan 2021 14:18:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-transfer-encoding; bh=3udMd4h15tGieAtMaVIcjTUFTeGpKnfOmu1COSfLVtg=; b=ki54o79BMFpHdsWbk2IW9zbP6Shuax7EoVROW/H6yMdudX3xXkC8xD4TzqmoSlW3CM ZMEPffnDmHMN9wv9kLAjFNt0BozbruSgsSjOJ0SmMG6W+QNXhv1/pED4JMOvQJNUwQJr f4pOidTj0RX22RwZz72om/66UkAU5nHpVGn3XayR+6eUvcytJnG2DMLSkZ9t0uSH0zsf MXvbhHthWqypOvEQSZeteXX5IE4eLypcCYxzbaH2/4h62VJ2rdko2KGtwJ9xd5d//GUd 2LsoTUuEtd5P+lEokMa6kvvvsG5CW8YWcaFVEUIVQNzl7srv47iq+3SBrIhlTjwljSpn TXDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=3udMd4h15tGieAtMaVIcjTUFTeGpKnfOmu1COSfLVtg=; b=hKqrp93V4ZM5cLRSQVwhJJgOJvx8zDRg56acq6Bm6PbJNDSVR208f+dl/9Z0meBjsO Rni4v4ydt3O0wAVxwWQvUdab96JvSUeop24W3mSPlDIlrPE7wiPMDynCOI9h+sKpQxVp 4SvE1V2XyhkWbSh4lXtmfE91LADJBJ39vZPRayltQH625wqhO4N2Q5vNgtbJRH3EgmS6 KM+gLz9zHj/5o8a5TgHGg9KyUvwwApRIc/y4qAY+e2hjB2Q02LQF0IyGo6baVp7ZMhiI 2YhHeECGiNja6Lugg3DcQ0iOxX3gvxrlKqVosgfIUAvG5ZgGIOeEQGWPj0oUvAJ9tCJq BhJw== X-Gm-Message-State: AOAM531NxksZMECsPZd0vKk/gfO9Vrh94xMwLxGG+rUAaYfzf7eRXSJb +qmDdqziXHCB+3WK1pf1sQb41CkUNe8= X-Google-Smtp-Source: ABdhPJw40P65w2zh/+ohQINl7PZxD0DzORbWtdOH14TXOXg75iwCHVMQ6EHaPlmMfHnVgkxGtkRHAQ== X-Received: by 2002:a5d:4203:: with SMTP id n3mr1213496wrq.49.1611872292846; Thu, 28 Jan 2021 14:18:12 -0800 (PST) Received: from lili ([2a01:e0a:59b:9120:65d2:2476:f637:db1e]) by smtp.gmail.com with ESMTPSA id b132sm7403558wmh.21.2021.01.28.14.18.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 14:18:12 -0800 (PST) From: zimoun In-Reply-To: <87r1m5p56j.fsf@gnu.org> References: <20210119154525.11230-1-zimon.toutoune@gmail.com> <87ft2nwd3b.fsf@gnu.org> <86im7ji90s.fsf@gmail.com> <87r1m5p56j.fsf@gnu.org> Date: Thu, 28 Jan 2021 23:07:18 +0100 Message-ID: <86lfcchg15.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: 45984@debbugs.gnu.org Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -1.25 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20161025 header.b=ki54o79B; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 84EE494042B X-Spam-Score: -1.25 X-Migadu-Scanner: scn1.migadu.com X-TUID: SUfeTKXqlHgo Hi Ludo, Thanks for look at it. On Thu, 28 Jan 2021 at 14:22, Ludovic Court=C3=A8s wrote: > I was looking at hunks like this one: > > (match (fetch-elpa-package name repo) > (#false > - (raise (condition > - (&message > - (message (format #false > - "couldn't find meta-data for ELPA package= `~a'." > - name)))))) > + (values #f '())) > > =E2=80=A6 and I interpreted it as meaning failures would now be silently > ignored. > > But I guess what happens is that #f is interpreted by the caller as a > failure, and that=E2=80=99s how we get the =E2=80=9Cfailed to download me= ta-data=E2=80=9D > message, right? The idea is to remove these error messages in each importer=E2=80=94=E2=80= =93here =E2=80=99elpa->guix-package=E2=80=99 from where the hunk is extracted=E2=80= =93=E2=80=93and have only one error message. For 3 reasons: 1. because it is simpler 2. because the message should not be in guix/import/ but guix/scripts/ 3. because it eases the recursive importer error message, IMHO. Basically, the current situation is: --8<---------------cut here---------------start------------->8--- $ guix import elpa do-not-exist Backtrace: 3 (primitive-load "/home/simon/.config/guix/current/bin/guix") In guix/ui.scm: 2154:12 2 (run-guix-command _ . _) In guix/scripts/import.scm: 120:11 1 (guix-import . _) In guix/scripts/import/elpa.scm: 107:23 0 (guix-import-elpa . _) guix/scripts/import/elpa.scm:107:23: In procedure guix-import-elpa: ERROR: 1. &message: "couldn't find meta-data for ELPA package `do-not-exist'." --8<---------------cut here---------------end--------------->8--- because the function =E2=80=99elpa->guix-package=E2=80=99 raises an error p= oorly handled. With the patch, the situation becomes: --8<---------------cut here---------------start------------->8--- $ ./pre-inst-env guix import elpa do-not-exist guix import: error: failed to download package 'do-not-exist' --8<---------------cut here---------------end--------------->8--- And the error message is handled by =E2=80=99guix/scripts/elpa.scm=E2=80=99= with: --8<---------------cut here---------------start------------->8--- (unless sexp (leave (G_ "failed to download package '~a'~%") package-name= )) sexp))) --8<---------------cut here---------------end--------------->8--- Does it make sense? Now, about the #3. The current situation is: --8<---------------cut here---------------start------------->8--- $ guix import elpa do-not-exist -r guix import: error: couldn't find meta-data for ELPA package `do-not-exist'. --8<---------------cut here---------------end--------------->8--- So here, the error is correctly handled. But it means to add error handler and message to all =E2=80=9Cguix/import/*.scm=E2=80=9C which is IMH= O a bad idea. Let take the example of =E2=80=99pypi->guix-package=E2=80=99 to underline m= y point. The current situation is: --8<---------------cut here---------------start------------->8--- $ guix import pypi do-not-exist following redirection to `https://pypi.org/pypi/do-not-exist/json/'... guix import: error: failed to download meta-data for package 'do-not-exist' --8<---------------cut here---------------end--------------->8--- and the error message comes from =E2=80=99guix/scripts/pypi.scm=E2=80=99. = However, the recursive fails with: --8<---------------cut here---------------start------------->8--- $ guix import pypi do-not-exist -r following redirection to `https://pypi.org/pypi/do-not-exist/json/'... Backtrace: 5 (primitive-load "/home/simon/.config/guix/current/bin/guix") In guix/ui.scm: 2154:12 4 (run-guix-command _ . _) In guix/scripts/import.scm: 120:11 3 (guix-import . _) In guix/scripts/import/pypi.scm: 97:16 2 (guix-import-pypi . _) In guix/import/utils.scm: 462:31 1 (recursive-import "do-not-exist" #:repo->guix-package _ #:guix= -name _ #:version _ #:repo =E2=80=A6) 453:33 0 (lookup-node "do-not-exist" #f) guix/import/utils.scm:453:33: In procedure lookup-node: Wrong number of values returned to continuation (expected 2) --8<---------------cut here---------------end--------------->8--- The reason is that the =E2=80=99lookup-node=E2=80=99 function in =E2=80=99r= ecursive-import=E2=80=99 is expecting =E2=80=99values=E2=80=99 when =E2=80=99pypi->guix-package=E2=80= =99 return just =E2=80=99#f=E2=80=99. --8<---------------cut here---------------start------------->8--- (define (lookup-node name version) (let* ((package dependencies (repo->guix-package name #:version version #:repo repo)) --8<---------------cut here---------------end--------------->8--- where =C2=ABrepo->guix-package =3D=3D pypi->guix-package=C2=BB. And this =E2=80=99lookup-node=E2=80=99 happens in =E2=80=99topological-sort=E2=80=99= inside a =E2=80=99map=E2=80=99. With the patch, the situation for non-recursive is not changed and for the recursive it becomes: --8<---------------cut here---------------start------------->8--- $ ./pre-inst-env guix import pypi do-not-exist -r following redirection to `https://pypi.org/pypi/do-not-exist/json/'... #f --8<---------------cut here---------------end--------------->8--- where this =E2=80=99#f=E2=80=99 is from =E2=80=99guix/scripts/pypi.scm=E2= =80=99. The error message could be handled here. An example is done for the =E2=80=99gem=E2=80=99 im= porter with the patch: =C2=ABscripts: import: gem: Fix recursive error handling.=C2=BB Does it make sense? Well, this patch set are trivial changes that quickly fix the current broken situation without a deep revamp. All in all, it is worth to rethink all that. Maybe let drop this patch set and I could come back with a clean fix. If no one beats me. :-) To avoid unnecessary boring work, do we agree that, for these cases, error messages should happen only in =E2=80=99guix/scripts/import/=E2=80=99? Cheers, simon PS: The error with recursive importer would be raised at compile time by a =E2=80=9Ctyped language=E2=80=9D as Typed Racket (to not name OCaml or Hask= ell). Just to point an use case about =C2=ABtyped vs weak typed=C2=BB that we dis= cussed once on December 2018, drinking a beer pre-Reproducible event. Ah good ol=E2=80=99 time with beers in bars, you are missing. ;-)