[PATCH] "guix import go" Improve error handling

DoneSubmitted by Sarah Morgensen.
Details
3 participants
  • Sarah Morgensen
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Severity
normal
S
S
Sarah Morgensen wrote on 23 Jun 2021 22:49
Download
[PATCH] import: utils: 'recursive-import' skips unfound packages
(address . guix-patches@gnu.org)
07d7149fc0f89f7f2d11fba47e1b0b2db5ceb809.1624479231.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by repo->guix-package is false.
* guix/import/go.scm (go-module-recursive-import): Explicitly return
false in 'repo->guix-package' when packages are not found.
* tests/import-utils.scm ("recursive-import: skip false packages"): New
test.
---
Hello Guix,

While trying to recursively import a go package I encountered the following
error when it was unable to find one of the dependencies:

Toggle snippet (34 lines)
$ ./pre-inst-env guix import go -r git.sr.ht/~sircmpwn/aerc
guix import: warning: Failed to import package "github.com/brunnre8/go.notmuch".
reason: "https://pkg.go.dev/github.com/brunnre8/go.notmuch" could not be fetched: HTTP error 404 ("Not Found").
This package and its dependencies won't be imported.
following redirection to `https://github.com/ProtonMail/go-crypto?go-get=1'...
following redirection to `https://github.com/jtolio/gls?go-get=1'...
Backtrace:
In srfi/srfi-1.scm:
586:29 19 (map1 ((package (name "go-github-com-rivo-uniseg") …) …))
586:29 18 (map1 ((package (name "go-github-com-mattn-go-r…") …) …))
586:29 17 (map1 ((package (name "go-github-com-miolini-da…") …) …))
586:29 16 (map1 ((package (name "go-github-com-riywo-logi…") …) …))
586:29 15 (map1 ((package (name "go-github-com-neelance-a…") …) …))
586:29 14 (map1 ((package (name "go-github-com-neelance-s…") …) …))
586:29 13 (map1 ((package (name "go-github-com-shurcool-go") …) …))
586:29 12 (map1 ((package (name "go-github-com-shurcool-h…") …) …))
586:29 11 (map1 ((package (name "go-github-com-shurcool-v…") …) …))
586:29 10 (map1 ((package (name "go-github-com-gopherjs-g…") …) …))
586:29 9 (map1 ((package (name "go-github-com-jtolds-gls") # …) …))
586:29 8 (map1 ((package (name "go-github-com-smartystre…") …) …))
586:29 7 (map1 ((package (name "go-github-com-smartystre…") …) …))
586:29 6 (map1 ((package (name "go-github-com-google-go-…") …) …))
586:29 5 (map1 ((package (name "go-google-golang-org-pro…") …) …))
586:29 4 (map1 ((package (name "go-github-com-golang-pro…") …) …))
586:29 3 (map1 ((package (name "go-google-golang-org-app…") …) …))
586:29 2 (map1 ((package (name "go-github-com-protonmail…") …) …))
586:17 1 (map1 (() (package (name "go-git-sr-ht-~sircmpwn…") …)))
In guix/import/utils.scm:
280:2 0 (package->definition _ _)

guix/import/utils.scm:280:2: In procedure package->definition:
Throw to key `match-error' with args `("match" "no matching pattern" ())'.

It seems like several importers (gem, egg, go) expect to be able to return #f or
'() in #:repo->guix-package when a package is not found (while printing a
warning to the user) but recursive-import doesn't look like it handles it. I
suppose this worked before but was silently broken at some point. This patch
(re-)enables this behavior. I added a test for this as well.

Hope this helps,
Sarah

guix/import/go.scm | 3 ++-
guix/import/utils.scm | 14 ++++++++------
tests/import-utils.scm | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 7 deletions(-)

Toggle diff (97 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index d110954664..c859098492 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..49f38cfa2a 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -469,16 +470,17 @@ to obtain the Guix package name corresponding to the upstream name."
            (normalized-deps (map (match-lambda
                                    ((name version) (list name version))
                                    (name (list name #f))) dependencies)))
-      (make-node name version package normalized-deps)))
+      (and package
+           (make-node name version package normalized-deps))))
 
   (map node-package
        (topological-sort (list (lookup-node package-name version))
                          (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
+                           (filter-map (lambda (name-version)
+                                         (apply lookup-node name-version))
+                                       (remove (lambda (name-version)
+                                                 (apply exists? name-version))
+                                               (node-dependencies node))))
                          (lambda (node)
                            (string-append
                             (node-name node)
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..1b728104e0 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,23 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")

base-commit: acfa55a581ca4e688ee4b8f860fe365b1f153ef9
-- 
2.31.1
Z
Z
zimoun wrote on 24 Jun 2021 14:21
Download
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
854kdna1wu.fsf@gmail.com
Download
Hi,

Thanks for the patch.

On mer., 23 juin 2021 at 13:49, Sarah Morgensen <iskarian@mgsn.dev> wrote:

Toggle quote (6 lines)
> It seems like several importers (gem, egg, go) expect to be able to return #f or
> '() in #:repo->guix-package when a package is not found (while printing a
> warning to the user) but recursive-import doesn't look like it handles it. I
> suppose this worked before but was silently broken at some point. This patch
> (re-)enables this behavior. I added a test for this as well.

Indeed, there is an inconsistency betweem all the recursive importers.
An attempt to fix this is done by [1].

1: http://issues.guix.gnu.org/issue/45984


Toggle quote (19 lines)
> diff --git a/guix/import/go.scm b/guix/import/go.scm
> index d110954664..c859098492 100644
> --- a/guix/import/go.scm
> +++ b/guix/import/go.scm
> @@ -5,6 +5,7 @@
> ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
> ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
> +;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%")
> (uri->string (http-get-error-uri c))
> (http-get-error-code c)
> (http-get-error-reason c))
> - (values '() '())))
> + (values #f '())))

Yes, there is an inconsistency...

Toggle quote (37 lines)
> (receive (package-sexp dependencies)
> (go-module->guix-package* name #:goproxy goproxy
> #:version version
> diff --git a/guix/import/utils.scm b/guix/import/utils.scm
> index d817318a91..49f38cfa2a 100644
> --- a/guix/import/utils.scm
> +++ b/guix/import/utils.scm
> @@ -8,6 +8,7 @@
> ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
> ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> +;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -469,16 +470,17 @@ to obtain the Guix package name corresponding to the upstream name."
> (normalized-deps (map (match-lambda
> ((name version) (list name version))
> (name (list name #f))) dependencies)))
> - (make-node name version package normalized-deps)))
> + (and package
> + (make-node name version package normalized-deps))))
>
> (map node-package
> (topological-sort (list (lookup-node package-name version))
> (lambda (node)
> - (map (lambda (name-version)
> - (apply lookup-node name-version))
> - (remove (lambda (name-version)
> - (apply exists? name-version))
> - (node-dependencies node))))
> + (filter-map (lambda (name-version)
> + (apply lookup-node name-version))
> + (remove (lambda (name-version)
> + (apply exists? name-version))
> + (node-dependencies node))))

...however, I am not convinced this fixes the issue. Compare:

Toggle snippet (19 lines)
$ guix import go do-not-exist -r
guix import: warning: Failed to import package "do-not-exist".
reason: "https://proxy.golang.org/do-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").
This package and its dependencies won't be imported.
Backtrace:
4 (primitive-load "/home/sitour/.config/guix/current/bin/guix")
In guix/ui.scm:
2147:12 3 (run-guix-command _ . _)
In guix/scripts/import.scm:
120:11 2 (guix-import . _)
In srfi/srfi-1.scm:
586:17 1 (map1 (()))
In guix/import/utils.scm:
280:2 0 (package->definition _ _)

guix/import/utils.scm:280:2: In procedure package->definition:
Throw to key `match-error' with args `("match" "no matching pattern" ())'.

with:

Toggle snippet (27 lines)
$ ./pre-inst-env guix import go do-not-exist -r
guix import: warning: Failed to import package "do-not-exist".
reason: "https://proxy.golang.org/do-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").
This package and its dependencies won't be imported.
Backtrace:
In ice-9/boot-9.scm:
1752:10 8 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In unknown file:
7 (apply-smob/0 #<thunk 7f3ca6977f60>)
In ice-9/boot-9.scm:
724:2 6 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)
In ice-9/eval.scm:
619:8 5 (_ #(#(#<directory (guile-user) 7f3ca6971c80>)))
In guix/ui.scm:
2147:12 4 (run-guix-command _ . _)
In guix/scripts/import.scm:
120:11 3 (guix-import . _)
In guix/scripts/import/go.scm:
116:20 2 (guix-import-go . _)
In guix/import/utils.scm:
440:34 1 (recursive-import _ #:repo->guix-package _ #:guix-name _ #:version _ #:repo _)
486:28 0 (_ #f)

guix/import/utils.scm:486:28: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f


Then, the patch introduces issues against other importers, for instance:

Toggle snippet (26 lines)
r@bioinfomeary01-Precision-7820-Tower$ guix import gem do-not-exist -r
#f

$ ./pre-inst-env guix import gem do-not-exist -r
Backtrace:
In ice-9/boot-9.scm:
1752:10 8 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In unknown file:
7 (apply-smob/0 #<thunk 7fd3a8934f60>)
In ice-9/boot-9.scm:
724:2 6 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)
In ice-9/eval.scm:
619:8 5 (_ #(#(#<directory (guile-user) 7fd3a892ec80>)))
In guix/ui.scm:
2147:12 4 (run-guix-command _ . _)
In guix/scripts/import.scm:
120:11 3 (guix-import . _)
In guix/scripts/import/gem.scm:
97:16 2 (guix-import-gem . _)
In guix/import/utils.scm:
440:34 1 (recursive-import _ #:repo->guix-package _ #:guix-name _ #:version _ #:repo _)
486:28 0 (_ #f)

guix/import/utils.scm:486:28: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f

Well, it is not worse for most of the importers. And perhaps this patch
is worth. As explained in [1], all the recursive importers should be
unified and the errors correctly handled, IMHO. With jeko, we are
pair-programming to work on it... but we are really slow. ;-)

Toggle quote (5 lines)
> diff --git a/tests/import-utils.scm b/tests/import-utils.scm
> index 874816442e..1b728104e0 100644
> --- a/tests/import-utils.scm
> +++ b/tests/import-utils.scm

Thanks for adding a test. :-)


Cheers,
simon
S
S
Sarah Morgensen wrote on 25 Jun 2021 06:22
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 49196@debbugs.gnu.org)
86lf6y7eur.fsf_-_@mgsn.dev
Download
Hello,

Thanks for the review!

zimoun <zimon.toutoune@gmail.com> writes:

Toggle quote (5 lines)
> Indeed, there is an inconsistency betweem all the recursive importers.
> An attempt to fix this is done by [1].
>
> 1: <http://issues.guix.gnu.org/issue/45984>

Thanks, that was a good read. With context, I see where you're coming
from. I agree that the direction to take with these importers is to
unify and standardize.

The goal of this patch is just to allow recursive import to provide a
usable result despite some failures, when the importer supports it. I'd
rather hunt down one package than 20+ :) This may make reporting errors
more difficult, but I think the use-case is worth it.

Toggle quote (8 lines)
> ...however, I am not convinced this fixes the issue. Compare:
>
> $ guix import go do-not-exist -r
>
> with:
>
> $ ./pre-inst-env guix import go do-not-exist -r

Good catch. I did not think to handle the toplevel package not being
found! This actually leads to making this a much simpler patch...

Toggle snippet (5 lines)
- (map node-package
+ (filter-map node-package
(topological-sort (list (lookup-node package-name version))

...which also works for other importers which return (values #f ...):

Toggle snippet (20 lines)
~/guix$ for importer in stackage elpa gem cran go ; do printf "\n### $importer\n" ; ./pre-inst-env guix import $importer really-does-not-exist -r ;done

### stackage
guix import: error: really-does-not-exist: Stackage package not found

### elpa
guix import: error: couldn't find meta-data for ELPA package `really-does-not-exist'.

### gem

### cran
error: failed to retrieve package information from "https://cran.r-project.org/web/packages/really-does-not-exist/DESCRIPTION": 404 ("Not Found")
guix import: error: couldn't find meta-data for R package

### go
guix import: warning: Failed to import package "really-does-not-exist".
reason: "https://proxy.golang.org/really-does-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").
This package and its dependencies won't be imported.

Toggle quote (5 lines)
> Well, it is not worse for most of the importers. And perhaps this patch
> is worth. As explained in [1], all the recursive importers should be
> unified and the errors correctly handled, IMHO. With jeko, we are
> pair-programming to work on it... but we are really slow. ;-)

Yes, this is very much just a stopgap. In your words (from #45984):

Toggle quote (3 lines)
> Well, this patch set are trivial changes that quickly fix the current
> broken situation without a deep revamp.

I will follow up with an updated patch.

Regards,
Sarah
S
S
Sarah Morgensen wrote on 25 Jun 2021 06:48
Download
[PATCH v2] import: utils: 'recursive-import' skips unfound packages
(address . 49196@debbugs.gnu.org)
770a40d86bb1b67d628b23ef3e4ae7668b7e522f.1624595313.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by repo->guix-package is false.
* guix/import/go.scm (go-module-recursive-import): Explicitly return
false in 'repo->guix-package' when packages are not found.
* tests/import-utils.scm ("recursive-import: skip false packages"): New
test.
---
guix/import/go.scm | 3 ++-
guix/import/utils.scm | 26 ++++++++++++++------------
tests/import-utils.scm | 28 ++++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 13 deletions(-)

Toggle diff (112 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index d110954664..c859098492 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..7f2e7ecb46 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -471,15 +472,16 @@ to obtain the Guix package name corresponding to the upstream name."
                                    (name (list name #f))) dependencies)))
       (make-node name version package normalized-deps)))
 
-  (map node-package
-       (topological-sort (list (lookup-node package-name version))
-                         (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
-                         (lambda (node)
-                           (string-append
-                            (node-name node)
-                            (or (node-version node) ""))))))
+  (filter-map
+   node-package
+   (topological-sort (list (lookup-node package-name version))
+                     (lambda (node)
+                       (map (lambda (name-version)
+                              (apply lookup-node name-version))
+                            (remove (lambda (name-version)
+                                      (apply exists? name-version))
+                                    (node-dependencies node))))
+                     (lambda (node)
+                       (string-append
+                        (node-name node)
+                        (or (node-version node) ""))))))
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..7c6c782917 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,33 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages (toplevel)"
+  '()
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
+(test-equal "recursive-import: skip false packages (dependency)"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")

base-commit: c7804cd97b28ef012acc20c1d861904e9592382b
-- 
2.31.1
Z
Z
zimoun wrote on 25 Jun 2021 08:53
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
CAJ3okZ0EtJByrvZ8efBa_fmY8aNquStwS9X6xDVMGNoHs84s3A@mail.gmail.com
Download
Hi,

On Fri, 25 Jun 2021 at 06:22, Sarah Morgensen <iskarian@mgsn.dev> wrote:

Toggle quote (5 lines)
> The goal of this patch is just to allow recursive import to provide a
> usable result despite some failures, when the importer supports it. I'd
> rather hunt down one package than 20+ :) This may make reporting errors
> more difficult, but I think the use-case is worth it.

I agree.

Toggle quote (9 lines)
> Good catch. I did not think to handle the toplevel package not being
> found! This actually leads to making this a much simpler patch...
>
> --8<---------------cut here---------------start------------->8---
> - (map node-package
> + (filter-map node-package
> (topological-sort (list (lookup-node package-name version))
> --8<---------------cut here---------------end--------------->8---

Cool!

Toggle quote (5 lines)
> Yes, this is very much just a stopgap. In your words (from #45984):
>
> > Well, this patch set are trivial changes that quickly fix the current
> > broken situation without a deep revamp.

I agree. Heh! I am consistent with my words. ;-)

Toggle quote (2 lines)
> I will follow up with an updated patch.

Cool, thanks!

Cheers,
simon
Z
Z
zimoun wrote on 25 Jun 2021 18:37
Download
[PATCH v3 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210625163749.65196-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by 'repo->guix-package' is false.
* tests/import-utils.scm: New tests.
---
guix/import/utils.scm | 26 ++++++++++++++------------
tests/import-utils.scm | 28 ++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 12 deletions(-)

Toggle diff (89 lines)
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..7f2e7ecb46 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -471,15 +472,16 @@ to obtain the Guix package name corresponding to the upstream name."
                                    (name (list name #f))) dependencies)))
       (make-node name version package normalized-deps)))
 
-  (map node-package
-       (topological-sort (list (lookup-node package-name version))
-                         (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
-                         (lambda (node)
-                           (string-append
-                            (node-name node)
-                            (or (node-version node) ""))))))
+  (filter-map
+   node-package
+   (topological-sort (list (lookup-node package-name version))
+                     (lambda (node)
+                       (map (lambda (name-version)
+                              (apply lookup-node name-version))
+                            (remove (lambda (name-version)
+                                      (apply exists? name-version))
+                                    (node-dependencies node))))
+                     (lambda (node)
+                       (string-append
+                        (node-name node)
+                        (or (node-version node) ""))))))
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..7c6c782917 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,33 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages (toplevel)"
+  '()
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
+(test-equal "recursive-import: skip false packages (dependency)"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")
-- 
2.32.0
Z
Z
zimoun wrote on 25 Jun 2021 18:37
Download
[PATCH v3 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210625163749.65196-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors.
(go-module-recursive-import): Remove 'guard'.
* guix/scripts/import/go.scm (guix-import-go): Adjust.
* tests/go.scm: Adjust.
---
guix/import/go.scm | 43 ++++++++++++++++++++++----------------
guix/scripts/import/go.scm | 6 +++---
tests/go.scm | 2 +-
3 files changed, 29 insertions(+), 22 deletions(-)

Toggle diff (110 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index c859098492..c559f02e5b 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -61,7 +62,7 @@
   #:use-module (web response)
   #:use-module (web uri)
 
-  #:export (go-module->guix-package
+  #:export (go-module->guix-package*
             go-module-recursive-import))
 
 ;;; Commentary:
@@ -630,17 +631,9 @@ hint: use one of the following available versions ~a\n"
          dependencies+versions
          dependencies))))
 
-(define go-module->guix-package* (memoize go-module->guix-package))
-
-(define* (go-module-recursive-import package-name
-                                     #:key (goproxy "https://proxy.golang.org")
-                                     version
-                                     pin-versions?)
-
-  (recursive-import
-   package-name
-   #:repo->guix-package
-   (lambda* (name #:key version repo)
+(define go-module->guix-package*
+  (memoize
+   (lambda args
      ;; Disable output buffering so that the following warning gets printed
      ;; consistently.
      (setvbuf (current-error-port) 'none)
@@ -648,15 +641,29 @@ hint: use one of the following available versions ~a\n"
                 (warning (G_ "Failed to import package ~s.
 reason: ~s could not be fetched: HTTP error ~a (~s).
 This package and its dependencies won't be imported.~%")
-                         name
+                         (match args ((name _ ...) name))
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
+                (values #f '()))
+               (else
+                (warning (G_ "Something went wrong with ~s.~%") args)
                 (values #f '())))
-       (receive (package-sexp dependencies)
-           (go-module->guix-package* name #:goproxy goproxy
-                                     #:version version
-                                     #:pin-versions? pin-versions?)
-         (values package-sexp dependencies))))
+       (apply go-module->guix-package args)))))
+
+(define* (go-module-recursive-import package-name
+                                     #:key (goproxy "https://proxy.golang.org")
+                                     version
+                                     pin-versions?)
+
+  (recursive-import
+   package-name
+   #:repo->guix-package
+   (lambda* (name #:key version repo)
+     (receive (package-sexp dependencies)
+         (go-module->guix-package* name #:goproxy goproxy
+                                   #:version version
+                                   #:pin-versions? pin-versions?)
+       (values package-sexp dependencies)))
    #:guix-name go-module->guix-package-name
    #:version version))
diff --git a/guix/scripts/import/go.scm b/guix/scripts/import/go.scm
index 74e8e60cce..071ecdb2ef 100644
--- a/guix/scripts/import/go.scm
+++ b/guix/scripts/import/go.scm
@@ -115,10 +115,10 @@ that are not yet in Guix"))
                (map package->definition*
                     (apply go-module-recursive-import arguments))
                ;; Single import.
-               (let ((sexp (apply go-module->guix-package arguments)))
+               (let ((sexp (apply go-module->guix-package* arguments)))
                  (unless sexp
-                   (leave (G_ "failed to download meta-data for module '~a'~%")
-                          module-name))
+                   (leave (G_ "failed to download meta-data for module '~a'.~%")
+                          name))
                  (package->definition* sexp))))))
       (()
        (leave (G_ "too few arguments~%")))
diff --git a/tests/go.scm b/tests/go.scm
index b088ab50d2..929a8b39d1 100644
--- a/tests/go.scm
+++ b/tests/go.scm
@@ -286,6 +286,6 @@ package.")
                               (nix-base32-string->bytevector
                                "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5")
                               #f)))
-                 (go-module->guix-package "github.com/go-check/check")))))))
+                 (go-module->guix-package* "github.com/go-check/check")))))))
 
 (test-end "go")
-- 
2.32.0
Z
Z
zimoun wrote on 25 Jun 2021 18:37
Download
[PATCH v3 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210625163749.65196-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/go.scm (go-module-recursive-import): Explicitly return
false when packages are not found.
---
guix/import/go.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (25 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index d110954664..c859098492 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version

base-commit: 88c7c739740b56cab132cf1a3f16392c434408f7
-- 
2.32.0
Z
Z
zimoun wrote on 25 Jun 2021 18:47
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
85v96199ic.fsf_-_@gmail.com
Download
Hi Sarah,

I sent a v3 where your v2 is split into 2 parts. Then I move the
guard. Now, your initial example returns:

Toggle snippet (26 lines)
$ ./pre-inst-env guix import go git.sr.ht/~sircmpwn/aerc -r
following redirection to `https://pkg.go.dev/github.com/zenhack/go.notmuch'...
following redirection to `https://pkg.go.dev/github.com/zenhack/go.notmuch'...
following redirection to `https://github.com/ProtonMail/go-crypto?go-get=1'...
following redirection to `https://github.com/jtolio/gls?go-get=1'...
guix import: warning: Something went wrong with ("github.com/neelance/sourcemap" #:goproxy "https://proxy.golang.org" #:version #f #:pin-versions? #f).
guix import: warning: Something went wrong with ("github.com/neelance/astrewrite" #:goproxy "https://proxy.golang.org" #:version #f #:pin-versions? #f).
(define-public go-git-sr-ht-~sircmpwn-getopt
(package
(name "go-git-sr-ht-~sircmpwn-getopt")
(version "0.0.0-20201218204720-9961a9c6298f")
(source

[...]

("go-github-com-creack-pty"
,go-github-com-creack-pty)
("go-git-sr-ht-~sircmpwn-getopt"
,go-git-sr-ht-~sircmpwn-getopt)))
(home-page "https://git.sr.ht/~sircmpwn/aerc")
(synopsis "aerc")
(description
"aerc is an email client for your terminal.")
(license license:expat)))

WDYT?

Well, I do not know if it is good idea to catch the 'else' case because
it hiddes what is wrong by the importer. On the other hand, it seems a
bit friendler for the end-user. :-)


Cheers,
simon
S
S
Sarah Morgensen wrote on 27 Jun 2021 06:46
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 49196@debbugs.gnu.org)
868s2v7w4t.fsf_-_@mgsn.dev
Download
Hi,

Apologies for the delay. Thanks for the work.

zimoun <zimon.toutoune@gmail.com> writes:

Toggle quote (3 lines)
> - #:export (go-module->guix-package
> + #:export (go-module->guix-package*

Would it be better to export both, in case a callee wants access to the
actual errors?

Toggle quote (4 lines)
> +(define go-module->guix-package*
> + (memoize
> + (lambda args

I would remove the memoize from this method (to put it back in later
on), because multiple invocations with errors would only yield one error
reported. I do not think this makes sense if another tool is using this.

Toggle quote (3 lines)
> + (else
> + (warning (G_ "Something went wrong with ~s.~%") args)

A catch-all is fine, but we should at least report the actual error,
even if it's not pretty:

(warning (G_ "Failed to import package ~s.~% reason: ~s")
package-name (exception-args c))

However, if we want to move in the direction of proper error handling
like guix/packages.scm and guix/ui.scm, we can do something like...

Toggle snippet (33 lines)
(define (report-import-error package-name error)
(cond
((http-get-error? error)
[...]
(else
[...]))))

(define* (go-module->guix-package* module-path
#:key (on-error report-import-error)
#:allow-other-keys #:rest args)
(with-exception-handler
(lambda (error)
(on-error module-path error)
(values #f '()))
(lambda () (apply go-module->guix-package module-path args))
#:unwind? #t))

(define* (go-module-recursive-import package-name
#:key (goproxy "https://proxy.golang.org")
version
pin-versions?)
(recursive-import
package-name
#:repo->guix-package
(memoize
(lambda* (name #:key version repo)
(go-module->guix-package* name #:goproxy goproxy
#:version version
#:pin-versions? pin-versions?)))
#:guix-name go-module->guix-package-name
#:version version))

Looks like I got a little carried away :) But breaking out the error
reporting gives us the future option of "plugging in" other error
reporting strategies, such as collating them and returning them
alongside a programmatic recursive import, or being able to provide a
list of packages which failed to import at the end. This will be much
more useful once we have a proper set of import error conditions.

Too much, perhaps?

Sarah
Z
Z
zimoun wrote on 28 Jun 2021 13:42
Download
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
CAJ3okZ10Ocf8Wedv1Ep4OiGBxrbFpNDi-_-037iy=XeezOHDjw@mail.gmail.com
Download
Hi,

On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian@mgsn.dev> wrote:

Toggle quote (6 lines)
> A catch-all is fine, but we should at least report the actual error,
> even if it's not pretty:
>
> (warning (G_ "Failed to import package ~s.~% reason: ~s")
> package-name (exception-args c))

Well, why not, even if I am not convinced the reason is meaningful
because it is mostly an incorrect parsing which returns:

reason: ("match" "no matching pattern" #f).

and I think it is better to display the complete 'args' instead of
extract the URL (package-name) from 'args'.

Toggle quote (3 lines)
> However, if we want to move in the direction of proper error handling
> like guix/packages.scm and guix/ui.scm, we can do something like...

Thanks for the idea. As explained patch#45984 [1], all the UI
messages must be in guix/scripts/import and not in guix/import and
therefore, indeed, error reporting needs to be unified between all the
importers and raised accordingly; that's what we are working on with
jeko (Jérémy Korwin-Zmijowski) as pair-programming exercise. :-)

1: http://issues.guix.gnu.org/issue/45984

Toggle quote (41 lines)
> --8<---------------cut here---------------start------------->8---
> (define (report-import-error package-name error)
> (cond
> ((http-get-error? error)
> [...]
> (else
> [...]))))
>
> (define* (go-module->guix-package* module-path
> #:key (on-error report-import-error)
> #:allow-other-keys #:rest args)
> (with-exception-handler
> (lambda (error)
> (on-error module-path error)
> (values #f '()))
> (lambda () (apply go-module->guix-package module-path args))
> #:unwind? #t))
>
> (define* (go-module-recursive-import package-name
> #:key (goproxy "https://proxy.golang.org")
> version
> pin-versions?)
> (recursive-import
> package-name
> #:repo->guix-package
> (memoize
> (lambda* (name #:key version repo)
> (go-module->guix-package* name #:goproxy goproxy
> #:version version
> #:pin-versions? pin-versions?)))
> #:guix-name go-module->guix-package-name
> #:version version))
> --8<---------------cut here---------------end--------------->8---
>
> Looks like I got a little carried away :) But breaking out the error
> reporting gives us the future option of "plugging in" other error
> reporting strategies, such as collating them and returning them
> alongside a programmatic recursive import, or being able to provide a
> list of packages which failed to import at the end. This will be much
> more useful once we have a proper set of import error conditions.

Back to the initial patch, I think it is better to simply fix with the
minor improvements of v3 your proposed and let this last proposal for
#45984; feel free to comment there. ;-)

Cheers,
simon
Z
Z
zimoun wrote on 28 Jun 2021 18:20
Download
[PATCH v4 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210628162014.15319-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/go.scm (go-module-recursive-import): Explicitly return
false when packages are not found.
---
guix/import/go.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (17 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 5e23d6a2b3..a2863c79ad 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -653,7 +653,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version

base-commit: 77c9c5c103ed2d09a43709bb78bc0b13a399e797
-- 
2.32.0
Z
Z
zimoun wrote on 28 Jun 2021 18:20
Download
[PATCH v4 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210628162014.15319-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by 'repo->guix-package' is false.
* tests/import-utils.scm: New tests.
---
guix/import/utils.scm | 26 ++++++++++++++------------
tests/import-utils.scm | 28 ++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 12 deletions(-)

Toggle diff (89 lines)
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..7f2e7ecb46 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -471,15 +472,16 @@ to obtain the Guix package name corresponding to the upstream name."
                                    (name (list name #f))) dependencies)))
       (make-node name version package normalized-deps)))
 
-  (map node-package
-       (topological-sort (list (lookup-node package-name version))
-                         (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
-                         (lambda (node)
-                           (string-append
-                            (node-name node)
-                            (or (node-version node) ""))))))
+  (filter-map
+   node-package
+   (topological-sort (list (lookup-node package-name version))
+                     (lambda (node)
+                       (map (lambda (name-version)
+                              (apply lookup-node name-version))
+                            (remove (lambda (name-version)
+                                      (apply exists? name-version))
+                                    (node-dependencies node))))
+                     (lambda (node)
+                       (string-append
+                        (node-name node)
+                        (or (node-version node) ""))))))
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..7c6c782917 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,33 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages (toplevel)"
+  '()
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
+(test-equal "recursive-import: skip false packages (dependency)"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")
-- 
2.32.0
Z
Z
zimoun wrote on 28 Jun 2021 18:20
Download
[PATCH v4 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210628162014.15319-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors.
(go-module-recursive-import): Remove 'guard'.
* guix/scripts/import/go.scm (guix-import-go): Adjust.
* tests/go.scm: Adjust.
---
guix/import/go.scm | 49 +++++++++++++++++++++++---------------
guix/scripts/import/go.scm | 6 ++---
tests/go.scm | 2 +-
3 files changed, 34 insertions(+), 23 deletions(-)

Toggle diff (112 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index a2863c79ad..e7d8bc3959 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -5,7 +5,8 @@
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
-;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -62,6 +63,7 @@
   #:use-module (web uri)
 
   #:export (go-module->guix-package
+            go-module->guix-package*
             go-module-recursive-import))
 
 ;;; Commentary:
@@ -631,7 +633,28 @@ hint: use one of the following available versions ~a\n"
          dependencies+versions
          dependencies))))
 
-(define go-module->guix-package* (memoize go-module->guix-package))
+(define go-module->guix-package*
+  (lambda args
+    ;; Disable output buffering so that the following warning gets printed
+    ;; consistently.
+    (setvbuf (current-error-port) 'none)
+    (let ((package-name (match args ((name _ ...) name))))
+      (guard (c ((http-get-error? c)
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s could not be fetched: HTTP error ~a (~s).
+This package and its dependencies won't be imported.~%")
+                          package-name
+                          (uri->string (http-get-error-uri c))
+                          (http-get-error-code c)
+                          (http-get-error-reason c))
+                 (values #f '()))
+                (else
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s.~%")
+                          package-name
+                          (exception-args c))
+                 (values #f '())))
+        (apply go-module->guix-package args)))))
 
 (define* (go-module-recursive-import package-name
                                      #:key (goproxy "https://proxy.golang.org")
@@ -642,22 +665,10 @@ hint: use one of the following available versions ~a\n"
    package-name
    #:repo->guix-package
    (lambda* (name #:key version repo)
-     ;; Disable output buffering so that the following warning gets printed
-     ;; consistently.
-     (setvbuf (current-error-port) 'none)
-     (guard (c ((http-get-error? c)
-                (warning (G_ "Failed to import package ~s.
-reason: ~s could not be fetched: HTTP error ~a (~s).
-This package and its dependencies won't be imported.~%")
-                         name
-                         (uri->string (http-get-error-uri c))
-                         (http-get-error-code c)
-                         (http-get-error-reason c))
-                (values #f '())))
-       (receive (package-sexp dependencies)
-           (go-module->guix-package* name #:goproxy goproxy
-                                     #:version version
-                                     #:pin-versions? pin-versions?)
-         (values package-sexp dependencies))))
+     (receive (package-sexp dependencies)
+         (go-module->guix-package* name #:goproxy goproxy
+                                   #:version version
+                                   #:pin-versions? pin-versions?)
+       (values package-sexp dependencies)))
    #:guix-name go-module->guix-package-name
    #:version version))
diff --git a/guix/scripts/import/go.scm b/guix/scripts/import/go.scm
index 74e8e60cce..071ecdb2ef 100644
--- a/guix/scripts/import/go.scm
+++ b/guix/scripts/import/go.scm
@@ -115,10 +115,10 @@ that are not yet in Guix"))
                (map package->definition*
                     (apply go-module-recursive-import arguments))
                ;; Single import.
-               (let ((sexp (apply go-module->guix-package arguments)))
+               (let ((sexp (apply go-module->guix-package* arguments)))
                  (unless sexp
-                   (leave (G_ "failed to download meta-data for module '~a'~%")
-                          module-name))
+                   (leave (G_ "failed to download meta-data for module '~a'.~%")
+                          name))
                  (package->definition* sexp))))))
       (()
        (leave (G_ "too few arguments~%")))
diff --git a/tests/go.scm b/tests/go.scm
index b088ab50d2..929a8b39d1 100644
--- a/tests/go.scm
+++ b/tests/go.scm
@@ -286,6 +286,6 @@ package.")
                               (nix-base32-string->bytevector
                                "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5")
                               #f)))
-                 (go-module->guix-package "github.com/go-check/check")))))))
+                 (go-module->guix-package* "github.com/go-check/check")))))))
 
 (test-end "go")
-- 
2.32.0
S
S
Sarah Morgensen wrote on 28 Jun 2021 19:13
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . zimoun)(address . zimon.toutoune@gmail.com)
86lf6t7w0g.fsf_-_@mgsn.dev
Download
zimoun <zimon.toutoune@gmail.com> writes:

Hello,

Thanks for the v4.

Toggle quote (16 lines)
> Hi,
>
> On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian@mgsn.dev> wrote:
>
>> A catch-all is fine, but we should at least report the actual error,
>> even if it's not pretty:
>>
>> (warning (G_ "Failed to import package ~s.~% reason: ~s")
>> package-name (exception-args c))
>
> Well, why not, even if I am not convinced the reason is meaningful
> because it is mostly an incorrect parsing which returns:
>
> reason: ("match" "no matching pattern" #f).
>

Yes, it is a less than ideal compromise... I could not quickly figure
out how to properly format it without a lot of complexity (like
guix/ui.scm does in 'call-with-error-handling'). I found it hard to read
the full exception object, but I would not object strongly to printing
the full exception object either. As you say, your patch will fix it
anyway ;)

Toggle quote (3 lines)
> and I think it is better to display the complete 'args' instead of
> extract the URL (package-name) from 'args'.

You're not wrong; I was just trying to keep it somewhat consistent with
the other error message.

Toggle quote (6 lines)
>> However, if we want to move in the direction of proper error handling
>> like guix/packages.scm and guix/ui.scm, we can do something like...
>
> Thanks for the idea. As explained patch#45984 [1], all the UI
> messages must be in guix/scripts/import and not in guix/import and

Yes, this was my secret trick: having separated out the error reporting,
it could be easily be moved to scripts/import later.
Toggle quote (4 lines)
> therefore, indeed, error reporting needs to be unified between all the
> importers and raised accordingly; that's what we are working on with
> jeko (Jérémy Korwin-Zmijowski) as pair-programming exercise. :-)

I look forward to the results!

Toggle quote (4 lines)
> Back to the initial patch, I think it is better to simply fix with the
> minor improvements of v3 your proposed and let this last proposal for
> #45984; feel free to comment there. ;-)

I agree. Your v4 looks good to me, except...

Toggle quote (3 lines)
> #:repo->guix-package
> (lambda* (name #:key version repo)

I apologize for not being clear earlier; by "put [memoize] back in later
on" I meant "later on in the call chain," e.g.

#:repo->guix-package
+ (memoize
(lambda* (name #:key version repo)

That's my only nit this time! ;) Thanks for bearing with me.

Regards,
Sarah
Z
Z
zimoun wrote on 29 Jun 2021 12:52
Download
[PATCH v5 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210629105237.45517-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by 'repo->guix-package' is false.
* tests/import-utils.scm: New tests.
---
guix/import/utils.scm | 26 ++++++++++++++------------
tests/import-utils.scm | 28 ++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 12 deletions(-)

Toggle diff (89 lines)
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..7f2e7ecb46 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -471,15 +472,16 @@ to obtain the Guix package name corresponding to the upstream name."
                                    (name (list name #f))) dependencies)))
       (make-node name version package normalized-deps)))
 
-  (map node-package
-       (topological-sort (list (lookup-node package-name version))
-                         (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
-                         (lambda (node)
-                           (string-append
-                            (node-name node)
-                            (or (node-version node) ""))))))
+  (filter-map
+   node-package
+   (topological-sort (list (lookup-node package-name version))
+                     (lambda (node)
+                       (map (lambda (name-version)
+                              (apply lookup-node name-version))
+                            (remove (lambda (name-version)
+                                      (apply exists? name-version))
+                                    (node-dependencies node))))
+                     (lambda (node)
+                       (string-append
+                        (node-name node)
+                        (or (node-version node) ""))))))
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..7c6c782917 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,33 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages (toplevel)"
+  '()
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
+(test-equal "recursive-import: skip false packages (dependency)"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")
-- 
2.32.0
Z
Z
zimoun wrote on 29 Jun 2021 12:52
Download
[PATCH v5 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210629105237.45517-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>

* guix/import/go.scm (go-module-recursive-import): Explicitly return
false when packages are not found.
---
guix/import/go.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (17 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 5e23d6a2b3..a2863c79ad 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -653,7 +653,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version

base-commit: 5ed105a8bb1a812975496dc3a091596355a0234c
-- 
2.32.0
Z
Z
zimoun wrote on 29 Jun 2021 12:52
Download
[PATCH v5 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210629105237.45517-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors, remove
memoize.
(go-module-recursive-import): Remove 'guard', add memoize.
* guix/scripts/import/go.scm (guix-import-go): Adjust.
* tests/go.scm: Adjust.
---
guix/import/go.scm | 52 +++++++++++++++++++++++---------------
guix/scripts/import/go.scm | 6 ++---
tests/go.scm | 2 +-
3 files changed, 36 insertions(+), 24 deletions(-)

Toggle diff (115 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index a2863c79ad..5bec31201d 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -5,7 +5,8 @@
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
-;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -62,6 +63,7 @@
   #:use-module (web uri)
 
   #:export (go-module->guix-package
+            go-module->guix-package*
             go-module-recursive-import))
 
 ;;; Commentary:
@@ -631,7 +633,28 @@ hint: use one of the following available versions ~a\n"
          dependencies+versions
          dependencies))))
 
-(define go-module->guix-package* (memoize go-module->guix-package))
+(define go-module->guix-package*
+  (lambda args
+    ;; Disable output buffering so that the following warning gets printed
+    ;; consistently.
+    (setvbuf (current-error-port) 'none)
+    (let ((package-name (match args ((name _ ...) name))))
+      (guard (c ((http-get-error? c)
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s could not be fetched: HTTP error ~a (~s).
+This package and its dependencies won't be imported.~%")
+                          package-name
+                          (uri->string (http-get-error-uri c))
+                          (http-get-error-code c)
+                          (http-get-error-reason c))
+                 (values #f '()))
+                (else
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s.~%")
+                          package-name
+                          (exception-args c))
+                 (values #f '())))
+        (apply go-module->guix-package args)))))
 
 (define* (go-module-recursive-import package-name
                                      #:key (goproxy "https://proxy.golang.org")
@@ -641,23 +664,12 @@ hint: use one of the following available versions ~a\n"
   (recursive-import
    package-name
    #:repo->guix-package
-   (lambda* (name #:key version repo)
-     ;; Disable output buffering so that the following warning gets printed
-     ;; consistently.
-     (setvbuf (current-error-port) 'none)
-     (guard (c ((http-get-error? c)
-                (warning (G_ "Failed to import package ~s.
-reason: ~s could not be fetched: HTTP error ~a (~s).
-This package and its dependencies won't be imported.~%")
-                         name
-                         (uri->string (http-get-error-uri c))
-                         (http-get-error-code c)
-                         (http-get-error-reason c))
-                (values #f '())))
-       (receive (package-sexp dependencies)
-           (go-module->guix-package* name #:goproxy goproxy
-                                     #:version version
-                                     #:pin-versions? pin-versions?)
-         (values package-sexp dependencies))))
+   (memoize
+    (lambda* (name #:key version repo)
+      (receive (package-sexp dependencies)
+          (go-module->guix-package* name #:goproxy goproxy
+                                    #:version version
+                                    #:pin-versions? pin-versions?)
+        (values package-sexp dependencies))))
    #:guix-name go-module->guix-package-name
    #:version version))
diff --git a/guix/scripts/import/go.scm b/guix/scripts/import/go.scm
index 74e8e60cce..071ecdb2ef 100644
--- a/guix/scripts/import/go.scm
+++ b/guix/scripts/import/go.scm
@@ -115,10 +115,10 @@ that are not yet in Guix"))
                (map package->definition*
                     (apply go-module-recursive-import arguments))
                ;; Single import.
-               (let ((sexp (apply go-module->guix-package arguments)))
+               (let ((sexp (apply go-module->guix-package* arguments)))
                  (unless sexp
-                   (leave (G_ "failed to download meta-data for module '~a'~%")
-                          module-name))
+                   (leave (G_ "failed to download meta-data for module '~a'.~%")
+                          name))
                  (package->definition* sexp))))))
       (()
        (leave (G_ "too few arguments~%")))
diff --git a/tests/go.scm b/tests/go.scm
index b088ab50d2..929a8b39d1 100644
--- a/tests/go.scm
+++ b/tests/go.scm
@@ -286,6 +286,6 @@ package.")
                               (nix-base32-string->bytevector
                                "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5")
                               #f)))
-                 (go-module->guix-package "github.com/go-check/check")))))))
+                 (go-module->guix-package* "github.com/go-check/check")))))))
 
 (test-end "go")
-- 
2.32.0
Z
Z
zimoun wrote on 2 Jul 2021 18:26
Download
control message for bug #49196
(address . control@debbugs.gnu.org)
87lf6on0l9.fsf@gmail.com
Download
retitle 49196 [PATCH] "guix import go" Improve error handling
quit
S
S
Sarah Morgensen wrote on 6 Aug 2021 20:04
Download
[PATCH v6 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)
5d11fc04400d26de06efba709cf2e1b14a1d5949.1628272957.git.iskarian@mgsn.dev
Download
* guix/import/go.scm (go-module-recursive-import): Explicitly return
false when packages are not found.
---
Hello Guix,

I took the liberty of rebasing this patchset since it no longer applies on
master.

Is there any one else who would like to comment? If not, I believe this has
reached a final state.

--
Sarah

guix/import/go.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (17 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 617a0d0e23..a4775f973f 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -668,7 +668,7 @@ This package and its dependencies won't be imported.~%")
                          (uri->string (http-get-error-uri c))
                          (http-get-error-code c)
                          (http-get-error-reason c))
-                (values '() '())))
+                (values #f '())))
        (receive (package-sexp dependencies)
            (go-module->guix-package* name #:goproxy goproxy
                                      #:version version

base-commit: c8e2be3b32fe784a9db52d8a1a12902ab12ae7cb
-- 
2.31.1
S
S
Sarah Morgensen wrote on 6 Aug 2021 20:05
Download
[PATCH v6 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)
ae55285e772acc7fa3ec5b95d6de8f1433860a4c.1628272957.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when the
package returned by 'repo->guix-package' is false.
* tests/import-utils.scm: New tests.
---
guix/import/utils.scm | 26 ++++++++++++++------------
tests/import-utils.scm | 28 ++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 12 deletions(-)

Toggle diff (89 lines)
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index d817318a91..7f2e7ecb46 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -471,15 +472,16 @@ to obtain the Guix package name corresponding to the upstream name."
                                    (name (list name #f))) dependencies)))
       (make-node name version package normalized-deps)))
 
-  (map node-package
-       (topological-sort (list (lookup-node package-name version))
-                         (lambda (node)
-                           (map (lambda (name-version)
-                                  (apply lookup-node name-version))
-                                (remove (lambda (name-version)
-                                          (apply exists? name-version))
-                                        (node-dependencies node))))
-                         (lambda (node)
-                           (string-append
-                            (node-name node)
-                            (or (node-version node) ""))))))
+  (filter-map
+   node-package
+   (topological-sort (list (lookup-node package-name version))
+                     (lambda (node)
+                       (map (lambda (name-version)
+                              (apply lookup-node name-version))
+                            (remove (lambda (name-version)
+                                      (apply exists? name-version))
+                                    (node-dependencies node))))
+                     (lambda (node)
+                       (string-append
+                        (node-name node)
+                        (or (node-version node) ""))))))
diff --git a/tests/import-utils.scm b/tests/import-utils.scm
index 874816442e..7c6c782917 100644
--- a/tests/import-utils.scm
+++ b/tests/import-utils.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -64,6 +65,33 @@
                                '())))
                     #:guix-name identity))
 
+(test-equal "recursive-import: skip false packages (toplevel)"
+  '()
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
+(test-equal "recursive-import: skip false packages (dependency)"
+  '((package
+      (name "foo")
+      (inputs `(("bar" ,bar)))))
+  (recursive-import "foo"
+                    #:repo 'repo
+                    #:repo->guix-package
+                    (match-lambda*
+                      (("foo" #:version #f #:repo 'repo)
+                       (values '(package
+                                  (name "foo")
+                                  (inputs `(("bar" ,bar))))
+                               '("bar")))
+                      (("bar" #:version #f #:repo 'repo)
+                       (values #f '())))
+                    #:guix-name identity))
+
 (test-assert "alist->package with simple source"
   (let* ((meta '(("name" . "hello")
                  ("version" . "2.10")
-- 
2.31.1
S
S
Sarah Morgensen wrote on 6 Aug 2021 20:05
Download
[PATCH v6 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
a4cee8f58e2187619e7a7fc3fc9b73ca04ae696f.1628272957.git.iskarian@mgsn.dev
Download
From: zimoun <zimon.toutoune@gmail.com>

* guix/import/go.scm (go-module->guix-package*): Handle errors, remove
memoize.
(go-module-recursive-import): Remove 'guard', add memoize.
* guix/scripts/import/go.scm (guix-import-go): Adjust.
* tests/go.scm: Adjust.
---
guix/import/go.scm | 50 +++++++++++++++++++++++---------------
guix/scripts/import/go.scm | 6 ++---
tests/go.scm | 2 +-
3 files changed, 35 insertions(+), 23 deletions(-)

Toggle diff (113 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index a4775f973f..4755571209 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -63,6 +64,7 @@
   #:use-module (web uri)
 
   #:export (go-module->guix-package
+            go-module->guix-package*
             go-module-recursive-import))
 
 ;;; Commentary:
@@ -646,7 +648,28 @@ hint: use one of the following available versions ~a\n"
          dependencies+versions
          dependencies))))
 
-(define go-module->guix-package* (memoize go-module->guix-package))
+(define go-module->guix-package*
+  (lambda args
+    ;; Disable output buffering so that the following warning gets printed
+    ;; consistently.
+    (setvbuf (current-error-port) 'none)
+    (let ((package-name (match args ((name _ ...) name))))
+      (guard (c ((http-get-error? c)
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s could not be fetched: HTTP error ~a (~s).
+This package and its dependencies won't be imported.~%")
+                          package-name
+                          (uri->string (http-get-error-uri c))
+                          (http-get-error-code c)
+                          (http-get-error-reason c))
+                 (values #f '()))
+                (else
+                 (warning (G_ "Failed to import package ~s.
+reason: ~s.~%")
+                          package-name
+                          (exception-args c))
+                 (values #f '())))
+        (apply go-module->guix-package args)))))
 
 (define* (go-module-recursive-import package-name
                                      #:key (goproxy "https://proxy.golang.org")
@@ -656,23 +679,12 @@ hint: use one of the following available versions ~a\n"
   (recursive-import
    package-name
    #:repo->guix-package
-   (lambda* (name #:key version repo)
-     ;; Disable output buffering so that the following warning gets printed
-     ;; consistently.
-     (setvbuf (current-error-port) 'none)
-     (guard (c ((http-get-error? c)
-                (warning (G_ "Failed to import package ~s.
-reason: ~s could not be fetched: HTTP error ~a (~s).
-This package and its dependencies won't be imported.~%")
-                         name
-                         (uri->string (http-get-error-uri c))
-                         (http-get-error-code c)
-                         (http-get-error-reason c))
-                (values #f '())))
-       (receive (package-sexp dependencies)
-           (go-module->guix-package* name #:goproxy goproxy
-                                     #:version version
-                                     #:pin-versions? pin-versions?)
-         (values package-sexp dependencies))))
+   (memoize
+    (lambda* (name #:key version repo)
+      (receive (package-sexp dependencies)
+          (go-module->guix-package* name #:goproxy goproxy
+                                    #:version version
+                                    #:pin-versions? pin-versions?)
+        (values package-sexp dependencies))))
    #:guix-name go-module->guix-package-name
    #:version version))
diff --git a/guix/scripts/import/go.scm b/guix/scripts/import/go.scm
index e08a1e427e..f5cfea8683 100644
--- a/guix/scripts/import/go.scm
+++ b/guix/scripts/import/go.scm
@@ -112,10 +112,10 @@ that are not yet in Guix"))
                (map package->definition*
                     (apply go-module-recursive-import arguments))
                ;; Single import.
-               (let ((sexp (apply go-module->guix-package arguments)))
+               (let ((sexp (apply go-module->guix-package* arguments)))
                  (unless sexp
-                   (leave (G_ "failed to download meta-data for module '~a'~%")
-                          module-name))
+                   (leave (G_ "failed to download meta-data for module '~a'.~%")
+                          name))
                  (package->definition* sexp))))))
       (()
        (leave (G_ "too few arguments~%")))
diff --git a/tests/go.scm b/tests/go.scm
index 6749f4585f..9e7223ff7c 100644
--- a/tests/go.scm
+++ b/tests/go.scm
@@ -410,6 +410,6 @@ package.")
                               (nix-base32-string->bytevector
                                "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5")
                               #f)))
-                 (go-module->guix-package "github.com/go-check/check")))))))
+                 (go-module->guix-package* "github.com/go-check/check")))))))
 
 (test-end "go")
-- 
2.31.1
L
L
Ludovic Courtès wrote on 1 Sep 2021 23:39
Download
Re: bug#49196: [PATCH] "guix import go" Improve error handling
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196-done@debbugs.gnu.org)
87k0k0561n.fsf_-_@gnu.org
Download
Hi,

Sarah Morgensen <iskarian@mgsn.dev> skribis:

Toggle quote (11 lines)
> * guix/import/go.scm (go-module-recursive-import): Explicitly return
> false when packages are not found.
> ---
> Hello Guix,
>
> I took the liberty of rebasing this patchset since it no longer applies on
> master.
>
> Is there any one else who would like to comment? If not, I believe this has
> reached a final state.

I think so too. :-)

Applied; thank you and thanks zimoun, and apologies for the delay!

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 49196@debbugs.gnu.org