Package: guix-patches;
Reported by: Sarah Morgensen <iskarian <at> mgsn.dev>
Date: Wed, 23 Jun 2021 20:50:02 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49196 in the body.
You can then email your comments to 49196 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Wed, 23 Jun 2021 20:50:02 GMT) Full text and rfc822 format available.Sarah Morgensen <iskarian <at> mgsn.dev>:guix-patches <at> gnu.org.
(Wed, 23 Jun 2021 20:50:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: guix-patches <at> gnu.org Subject: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Wed, 23 Jun 2021 13:49:05 -0700
* 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 <at> gmail.com>
;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Thu, 24 Jun 2021 12:23:02 GMT) Full text and rfc822 format available.Message #8 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: Sarah Morgensen <iskarian <at> mgsn.dev> Cc: 49196 <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Thu, 24 Jun 2021 14:21:37 +0200
Hi,
Thanks for the patch.
On mer., 23 juin 2021 at 13:49, Sarah Morgensen <iskarian <at> 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 <at> gmail.com>
> ;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
> ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
> +;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
> ;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
> ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
> +;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 04:23:02 GMT) Full text and rfc822 format available.Message #11 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: zimoun <zimon.toutoune <at> gmail.com> Cc: 49196 <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Thu, 24 Jun 2021 21:22:36 -0700
Hello,
Thanks for the review!
zimoun <zimon.toutoune <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 04:49:01 GMT) Full text and rfc822 format available.Message #14 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: 49196 <at> debbugs.gnu.org Subject: [PATCH v2] import: utils: 'recursive-import' skips unfound packages Date: Thu, 24 Jun 2021 21:48:26 -0700
* 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 <at> gmail.com>
;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 06:54:02 GMT) Full text and rfc822 format available.Message #17 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: Sarah Morgensen <iskarian <at> mgsn.dev> Cc: 49196 <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Fri, 25 Jun 2021 08:53:37 +0200
Hi, On Fri, 25 Jun 2021 at 06:22, Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 15:13:02 GMT) Full text and rfc822 format available.guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 16:39:02 GMT) Full text and rfc822 format available.Message #23 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v3 2/3] import: utils: Skip not found packages. Date: Fri, 25 Jun 2021 18:37:48 +0200
From: Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 16:39:02 GMT) Full text and rfc822 format available.Message #26 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev, zimoun <zimon.toutoune <at> gmail.com> Subject: [PATCH v3 3/3] import: go: Improve error handling. Date: Fri, 25 Jun 2021 18:37:49 +0200
* 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 <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 16:39:03 GMT) Full text and rfc822 format available.Message #29 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v3 1/3] import: go: Return false for package not found. Date: Fri, 25 Jun 2021 18:37:47 +0200
From: Sarah Morgensen <iskarian <at> 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 <at> gmail.com>
;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 25 Jun 2021 16:49:02 GMT) Full text and rfc822 format available.Message #32 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: Sarah Morgensen <iskarian <at> mgsn.dev> Cc: 49196 <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Fri, 25 Jun 2021 18:47:23 +0200
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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Sun, 27 Jun 2021 04:47:02 GMT) Full text and rfc822 format available.Message #35 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: zimoun <zimon.toutoune <at> gmail.com> Cc: 49196 <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Sat, 26 Jun 2021 21:46:10 -0700
Hi,
Apologies for the delay. Thanks for the work.
zimoun <zimon.toutoune <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Mon, 28 Jun 2021 11:43:02 GMT) Full text and rfc822 format available.Message #38 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: Sarah Morgensen <iskarian <at> mgsn.dev> Cc: 49196 <at> debbugs.gnu.org, Jérémy Korwin-Zmijowski <jeremy <at> korwin-zmijowski.fr> Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Mon, 28 Jun 2021 13:42:26 +0200
Hi,
On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Mon, 28 Jun 2021 16:21:02 GMT) Full text and rfc822 format available.Message #41 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v4 1/3] import: go: Return false for package not found. Date: Mon, 28 Jun 2021 18:20:12 +0200
From: Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Mon, 28 Jun 2021 16:21:03 GMT) Full text and rfc822 format available.Message #44 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v4 2/3] import: utils: Skip not found packages. Date: Mon, 28 Jun 2021 18:20:13 +0200
From: Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Mon, 28 Jun 2021 16:21:03 GMT) Full text and rfc822 format available.Message #47 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev, zimoun <zimon.toutoune <at> gmail.com> Subject: [PATCH v4 3/3] import: go: Improve error handling. Date: Mon, 28 Jun 2021 18:20:14 +0200
* 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 <at> gmail.com>
;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
-;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Mon, 28 Jun 2021 17:14:02 GMT) Full text and rfc822 format available.Message #50 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: zimoun <zimon.toutoune <at> gmail.com> Cc: 49196 <at> debbugs.gnu.org, Jérémy Korwin-Zmijowski <jeremy <at> korwin-zmijowski.fr> Subject: Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages Date: Mon, 28 Jun 2021 10:13:19 -0700
zimoun <zimon.toutoune <at> gmail.com> writes:
Hello,
Thanks for the v4.
> Hi,
>
> On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Tue, 29 Jun 2021 10:54:02 GMT) Full text and rfc822 format available.Message #53 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v5 2/3] import: utils: Skip not found packages. Date: Tue, 29 Jun 2021 12:52:36 +0200
From: Sarah Morgensen <iskarian <at> 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Tue, 29 Jun 2021 10:54:02 GMT) Full text and rfc822 format available.Message #56 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev Subject: [PATCH v5 1/3] import: go: Return false for package not found. Date: Tue, 29 Jun 2021 12:52:35 +0200
From: Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Tue, 29 Jun 2021 10:54:03 GMT) Full text and rfc822 format available.Message #59 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: zimoun <zimon.toutoune <at> gmail.com> To: 49196 <at> debbugs.gnu.org Cc: iskarian <at> mgsn.dev, zimoun <zimon.toutoune <at> gmail.com> Subject: [PATCH v5 3/3] import: go: Improve error handling. Date: Tue, 29 Jun 2021 12:52:37 +0200
* 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 <at> gmail.com>
;;; Copyright © 2021 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
-;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> 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
zimoun <zimon.toutoune <at> gmail.com>
to control <at> debbugs.gnu.org.
(Fri, 02 Jul 2021 16:34:03 GMT) Full text and rfc822 format available.guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 06 Aug 2021 18:05:02 GMT) Full text and rfc822 format available.Message #64 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: 49196 <at> debbugs.gnu.org Subject: [PATCH v6 1/3] import: go: Return false for package not found. Date: Fri, 6 Aug 2021 11:04:43 -0700
* 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 06 Aug 2021 18:06:01 GMT) Full text and rfc822 format available.Message #67 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: 49196 <at> debbugs.gnu.org Subject: [PATCH v6 2/3] import: utils: Skip not found packages. Date: Fri, 6 Aug 2021 11:05:16 -0700
* 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 <at> googlemail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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 <at> elephly.net>
;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
;;; Copyright © 2020 Martin Becze <mjbecze <at> riseup.net>
+;;; Copyright © 2021 Sarah Morgensen <iskarian <at> 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
guix-patches <at> gnu.org:bug#49196; Package guix-patches.
(Fri, 06 Aug 2021 18:06:02 GMT) Full text and rfc822 format available.Message #70 received at 49196 <at> debbugs.gnu.org (full text, mbox):
From: Sarah Morgensen <iskarian <at> mgsn.dev> To: 49196 <at> debbugs.gnu.org Cc: zimoun <zimon.toutoune <at> gmail.com> Subject: [PATCH v6 3/3] import: go: Improve error handling. Date: Fri, 6 Aug 2021 11:05:17 -0700
From: zimoun <zimon.toutoune <at> 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 <at> gnu.org>
;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> 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
Ludovic Courtès <ludo <at> gnu.org>:Sarah Morgensen <iskarian <at> mgsn.dev>:Message #75 received at 49196-done <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludo <at> gnu.org> To: Sarah Morgensen <iskarian <at> mgsn.dev> Cc: 49196-done <at> debbugs.gnu.org Subject: Re: bug#49196: [PATCH] "guix import go" Improve error handling Date: Wed, 01 Sep 2021 23:39:48 +0200
Hi, Sarah Morgensen <iskarian <at> 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’.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Thu, 30 Sep 2021 11:24:07 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.