[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[Sarah Morgensen via Guix-patches via]

* 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:

--8<---------------cut here---------------start------------->8---
$ ./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" ())'.
--8<---------------cut here---------------end--------------->8---

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(-)

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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[zimoun]

Hi,

Thanks for the patch.

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

> 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>


> 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...

>         (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:

--8<---------------cut here---------------start------------->8---
$ 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" ())'.
--8<---------------cut here---------------end--------------->8---

with:

--8<---------------cut here---------------start------------->8---
$ ./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
--8<---------------cut here---------------end--------------->8---


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

--8<---------------cut here---------------start------------->8---
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
--8<---------------cut here---------------end--------------->8---

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. ;-)

> 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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[Sarah Morgensen via Guix-patches via]

Hello,

Thanks for the review!

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

> 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.

> ...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...

--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---

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

--8<---------------cut here---------------start------------->8---
~/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.
--8<---------------cut here---------------end--------------->8---

> 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):

> 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

[bug#49196] [PATCH v2] import: utils: 'recursive-import' skips unfound packages

[Sarah Morgensen via Guix-patches via]

* 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(-)

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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[zimoun]

Hi,

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

> 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.

> 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!

> 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. ;-)

> I will follow up with an updated patch.

Cool, thanks!

Cheers,
simon

[bug#49196] [PATCH v3 1/3] import: go: Return false for package not found.

[zimoun]

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(-)

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

[bug#49196] [PATCH v3 2/3] import: utils: Skip not found packages.

[zimoun]

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(-)

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

[bug#49196] [PATCH v3 3/3] import: go: Improve error handling.

[zimoun]

* 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(-)

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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[zimoun]

Hi Sarah,

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

--8<---------------cut here---------------start------------->8---
$ ./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)))
--8<---------------cut here---------------end--------------->8---

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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[Sarah Morgensen via Guix-patches via]

Hi,

Apologies for the delay. Thanks for the work.

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

> -  #: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?

> +(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.

> +               (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...

--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.

Too much, perhaps?

Sarah

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[zimoun]

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).

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

> 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>

> --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

[bug#49196] [PATCH v4 1/3] import: go: Return false for package not found.

[zimoun]

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(-)

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

[bug#49196] [PATCH v4 2/3] import: utils: Skip not found packages.

[zimoun]

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(-)

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

[bug#49196] [PATCH v4 3/3] import: go: Improve error handling.

[zimoun]

* 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(-)

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

[bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages

[Sarah Morgensen via Guix-patches via]

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

Hello,

Thanks for the v4.

> 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 ;)

> 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.

>> 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.
 
> 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!

> 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...

>    #: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

[bug#49196] [PATCH v5 1/3] import: go: Return false for package not found.

[zimoun]

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(-)

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

[bug#49196] [PATCH v5 2/3] import: utils: Skip not found packages.

[zimoun]

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(-)

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

[bug#49196] [PATCH v5 3/3] import: go: Improve error handling.

[zimoun]

* 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(-)

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

[bug#49196] [PATCH v6 1/3] import: go: Return false for package not found.

[Sarah Morgensen]

* 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(-)

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

[bug#49196] [PATCH v6 2/3] import: utils: Skip not found packages.

[Sarah Morgensen]

* 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(-)

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

[bug#49196] [PATCH v6 3/3] import: go: Improve error handling.

[Sarah Morgensen]

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(-)

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

bug#49196: [PATCH] "guix import go" Improve error handling

[Ludovic Courtès]

Hi,

Sarah Morgensen <iskarian@mgsn.dev> skribis:

> * 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’.