[PATCH] "guix import go" Improve error handling

DoneSubmitted by Sarah Morgensen.
Details
3 participants
  • Sarah Morgensen
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Severity
normal
S
S
Sarah Morgensen wrote on 23 Jun 22:49 +0200
Download
[PATCH] import: utils: 'recursive-import' skips unfound packages
(address . guix-patches@gnu.org)
07d7149fc0f89f7f2d11fba47e1b0b2db5ceb809.1624479231.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by repo->guix-package is false.* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse in 'repo->guix-package' when packages are not found.* tests/import-utils.scm ("recursive-import: skip false packages"): Newtest.---Hello Guix,
While trying to recursively import a go package I encountered the followingerror when it was unable to find one of the dependencies:
Toggle snippet (34 lines)$ ./pre-inst-env guix import go -r git.sr.ht/~sircmpwn/aercguix import: warning: Failed to import package "github.com/brunnre8/go.notmuch".reason: "https://pkg.go.dev/github.com/brunnre8/go.notmuch" could not be fetched: HTTP error 404 ("Not Found").This package and its dependencies won't be imported.following redirection to `https://github.com/ProtonMail/go-crypto?go-get=1'...following redirection to `https://github.com/jtolio/gls?go-get=1'...Backtrace:In srfi/srfi-1.scm: 586:29 19 (map1 ((package (name "go-github-com-rivo-uniseg") …) …)) 586:29 18 (map1 ((package (name "go-github-com-mattn-go-r…") …) …)) 586:29 17 (map1 ((package (name "go-github-com-miolini-da…") …) …)) 586:29 16 (map1 ((package (name "go-github-com-riywo-logi…") …) …)) 586:29 15 (map1 ((package (name "go-github-com-neelance-a…") …) …)) 586:29 14 (map1 ((package (name "go-github-com-neelance-s…") …) …)) 586:29 13 (map1 ((package (name "go-github-com-shurcool-go") …) …)) 586:29 12 (map1 ((package (name "go-github-com-shurcool-h…") …) …)) 586:29 11 (map1 ((package (name "go-github-com-shurcool-v…") …) …)) 586:29 10 (map1 ((package (name "go-github-com-gopherjs-g…") …) …)) 586:29 9 (map1 ((package (name "go-github-com-jtolds-gls") # …) …)) 586:29 8 (map1 ((package (name "go-github-com-smartystre…") …) …)) 586:29 7 (map1 ((package (name "go-github-com-smartystre…") …) …)) 586:29 6 (map1 ((package (name "go-github-com-google-go-…") …) …)) 586:29 5 (map1 ((package (name "go-google-golang-org-pro…") …) …)) 586:29 4 (map1 ((package (name "go-github-com-golang-pro…") …) …)) 586:29 3 (map1 ((package (name "go-google-golang-org-app…") …) …)) 586:29 2 (map1 ((package (name "go-github-com-protonmail…") …) …)) 586:17 1 (map1 (() (package (name "go-git-sr-ht-~sircmpwn…") …)))In guix/import/utils.scm: 280:2 0 (package->definition _ _)
guix/import/utils.scm:280:2: In procedure package->definition:Throw to key `match-error' with args `("match" "no matching pattern" ())'.
It seems like several importers (gem, egg, go) expect to be able to return #f or'() in #:repo->guix-package when a package is not found (while printing awarning to the user) but recursive-import doesn't look like it handles it. Isuppose this worked before but was silently broken at some point. This patch(re-)enables this behavior. I added a test for this as well.
Hope this helps,Sarah
guix/import/go.scm | 3 ++- guix/import/utils.scm | 14 ++++++++------ tests/import-utils.scm | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 7 deletions(-)
Toggle diff (97 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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 versiondiff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..1b728104e0 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,23 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")
base-commit: acfa55a581ca4e688ee4b8f860fe365b1f153ef9-- 2.31.1
Z
Z
zimoun wrote on 24 Jun 14:21 +0200
Download
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
854kdna1wu.fsf@gmail.com
Download
Hi,
Thanks for the patch.
On mer., 23 juin 2021 at 13:49, Sarah Morgensen <iskarian@mgsn.dev> wrote:
Toggle quote (6 lines)> It seems like several importers (gem, egg, go) expect to be able to return #f or> '() in #:repo->guix-package when a package is not found (while printing a> warning to the user) but recursive-import doesn't look like it handles it. I> suppose this worked before but was silently broken at some point. This patch> (re-)enables this behavior. I added a test for this as well.
Indeed, there is an inconsistency betweem all the recursive importers.An attempt to fix this is done by [1].
1: http://issues.guix.gnu.org/issue/45984

Toggle quote (19 lines)> diff --git a/guix/import/go.scm b/guix/import/go.scm> index d110954664..c859098492 100644> --- a/guix/import/go.scm> +++ b/guix/import/go.scm> @@ -5,6 +5,7 @@> ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>> ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>> ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>> +;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>> ;;;> ;;; This file is part of GNU Guix.> ;;;> @@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%")> (uri->string (http-get-error-uri c))> (http-get-error-code c)> (http-get-error-reason c))> - (values '() '())))> + (values #f '())))
Yes, there is an inconsistency...
Toggle quote (37 lines)> (receive (package-sexp dependencies)> (go-module->guix-package* name #:goproxy goproxy> #:version version> diff --git a/guix/import/utils.scm b/guix/import/utils.scm> index d817318a91..49f38cfa2a 100644> --- a/guix/import/utils.scm> +++ b/guix/import/utils.scm> @@ -8,6 +8,7 @@> ;;; Copyright © 2020 Helio Machado <0x2b3bfa0+guix@googlemail.com>> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>> ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>> +;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>> ;;;> ;;; This file is part of GNU Guix.> ;;;> @@ -469,16 +470,17 @@ to obtain the Guix package name corresponding to the upstream name."> (normalized-deps (map (match-lambda> ((name version) (list name version))> (name (list name #f))) dependencies)))> - (make-node name version package normalized-deps)))> + (and package> + (make-node name version package normalized-deps))))>> (map node-package> (topological-sort (list (lookup-node package-name version))> (lambda (node)> - (map (lambda (name-version)> - (apply lookup-node name-version))> - (remove (lambda (name-version)> - (apply exists? name-version))> - (node-dependencies node))))> + (filter-map (lambda (name-version)> + (apply lookup-node name-version))> + (remove (lambda (name-version)> + (apply exists? name-version))> + (node-dependencies node))))
...however, I am not convinced this fixes the issue. Compare:
Toggle snippet (19 lines)$ guix import go do-not-exist -rguix import: warning: Failed to import package "do-not-exist".reason: "https://proxy.golang.org/do-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").This package and its dependencies won't be imported.Backtrace: 4 (primitive-load "/home/sitour/.config/guix/current/bin/guix")In guix/ui.scm: 2147:12 3 (run-guix-command _ . _)In guix/scripts/import.scm: 120:11 2 (guix-import . _)In srfi/srfi-1.scm: 586:17 1 (map1 (()))In guix/import/utils.scm: 280:2 0 (package->definition _ _)
guix/import/utils.scm:280:2: In procedure package->definition:Throw to key `match-error' with args `("match" "no matching pattern" ())'.
with:
Toggle snippet (27 lines)$ ./pre-inst-env guix import go do-not-exist -rguix import: warning: Failed to import package "do-not-exist".reason: "https://proxy.golang.org/do-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").This package and its dependencies won't be imported.Backtrace:In ice-9/boot-9.scm: 1752:10 8 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)In unknown file: 7 (apply-smob/0 #<thunk 7f3ca6977f60>)In ice-9/boot-9.scm: 724:2 6 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)In ice-9/eval.scm: 619:8 5 (_ #(#(#<directory (guile-user) 7f3ca6971c80>)))In guix/ui.scm: 2147:12 4 (run-guix-command _ . _)In guix/scripts/import.scm: 120:11 3 (guix-import . _)In guix/scripts/import/go.scm: 116:20 2 (guix-import-go . _)In guix/import/utils.scm: 440:34 1 (recursive-import _ #:repo->guix-package _ #:guix-name _ #:version _ #:repo _) 486:28 0 (_ #f)
guix/import/utils.scm:486:28: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f

Then, the patch introduces issues against other importers, for instance:
Toggle snippet (26 lines)r@bioinfomeary01-Precision-7820-Tower$ guix import gem do-not-exist -r#f
$ ./pre-inst-env guix import gem do-not-exist -rBacktrace:In ice-9/boot-9.scm: 1752:10 8 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)In unknown file: 7 (apply-smob/0 #<thunk 7fd3a8934f60>)In ice-9/boot-9.scm: 724:2 6 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)In ice-9/eval.scm: 619:8 5 (_ #(#(#<directory (guile-user) 7fd3a892ec80>)))In guix/ui.scm: 2147:12 4 (run-guix-command _ . _)In guix/scripts/import.scm: 120:11 3 (guix-import . _)In guix/scripts/import/gem.scm: 97:16 2 (guix-import-gem . _)In guix/import/utils.scm: 440:34 1 (recursive-import _ #:repo->guix-package _ #:guix-name _ #:version _ #:repo _) 486:28 0 (_ #f)
guix/import/utils.scm:486:28: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f
Well, it is not worse for most of the importers. And perhaps this patchis worth. As explained in [1], all the recursive importers should beunified and the errors correctly handled, IMHO. With jeko, we arepair-programming to work on it... but we are really slow. ;-)
Toggle quote (5 lines)> diff --git a/tests/import-utils.scm b/tests/import-utils.scm> index 874816442e..1b728104e0 100644> --- a/tests/import-utils.scm> +++ b/tests/import-utils.scm
Thanks for adding a test. :-)

Cheers,simon
S
S
Sarah Morgensen wrote on 25 Jun 06:22 +0200
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 49196@debbugs.gnu.org)
86lf6y7eur.fsf_-_@mgsn.dev
Download
Hello,
Thanks for the review!
zimoun <zimon.toutoune@gmail.com> writes:
Toggle quote (5 lines)> Indeed, there is an inconsistency betweem all the recursive importers.> An attempt to fix this is done by [1].>> 1: <http://issues.guix.gnu.org/issue/45984>
Thanks, that was a good read. With context, I see where you're comingfrom. I agree that the direction to take with these importers is tounify and standardize.
The goal of this patch is just to allow recursive import to provide ausable result despite some failures, when the importer supports it. I'drather hunt down one package than 20+ :) This may make reporting errorsmore difficult, but I think the use-case is worth it.
Toggle quote (8 lines)> ...however, I am not convinced this fixes the issue. Compare:>> $ guix import go do-not-exist -r>> with:>> $ ./pre-inst-env guix import go do-not-exist -r
Good catch. I did not think to handle the toplevel package not beingfound! This actually leads to making this a much simpler patch...
Toggle snippet (5 lines)- (map node-package+ (filter-map node-package (topological-sort (list (lookup-node package-name version))
...which also works for other importers which return (values #f ...):
Toggle snippet (20 lines)~/guix$ for importer in stackage elpa gem cran go ; do printf "\n### $importer\n" ; ./pre-inst-env guix import $importer really-does-not-exist -r ;done
### stackageguix import: error: really-does-not-exist: Stackage package not found
### elpaguix import: error: couldn't find meta-data for ELPA package `really-does-not-exist'.
### gem
### cranerror: 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
### goguix import: warning: Failed to import package "really-does-not-exist".reason: "https://proxy.golang.org/really-does-not-exist/@v/list" could not be fetched: HTTP error 410 ("Gone").This package and its dependencies won't be imported.
Toggle quote (5 lines)> Well, it is not worse for most of the importers. And perhaps this patch> is worth. As explained in [1], all the recursive importers should be> unified and the errors correctly handled, IMHO. With jeko, we are> pair-programming to work on it... but we are really slow. ;-)
Yes, this is very much just a stopgap. In your words (from #45984):
Toggle quote (3 lines)> Well, this patch set are trivial changes that quickly fix the current> broken situation without a deep revamp.
I will follow up with an updated patch.
Regards,Sarah
S
S
Sarah Morgensen wrote on 25 Jun 06:48 +0200
Download
[PATCH v2] import: utils: 'recursive-import' skips unfound packages
(address . 49196@debbugs.gnu.org)
770a40d86bb1b67d628b23ef3e4ae7668b7e522f.1624595313.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by repo->guix-package is false.* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse in 'repo->guix-package' when packages are not found.* tests/import-utils.scm ("recursive-import: skip false packages"): Newtest.--- guix/import/go.scm | 3 ++- guix/import/utils.scm | 26 ++++++++++++++------------ tests/import-utils.scm | 28 ++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 13 deletions(-)
Toggle diff (112 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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 versiondiff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..7c6c782917 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,33 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages (toplevel)"+ '()+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))++(test-equal "recursive-import: skip false packages (dependency)"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")
base-commit: c7804cd97b28ef012acc20c1d861904e9592382b-- 2.31.1
Z
Z
zimoun wrote on 25 Jun 08:53 +0200
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
CAJ3okZ0EtJByrvZ8efBa_fmY8aNquStwS9X6xDVMGNoHs84s3A@mail.gmail.com
Download
Hi,
On Fri, 25 Jun 2021 at 06:22, Sarah Morgensen <iskarian@mgsn.dev> wrote:
Toggle quote (5 lines)> The goal of this patch is just to allow recursive import to provide a> usable result despite some failures, when the importer supports it. I'd> rather hunt down one package than 20+ :) This may make reporting errors> more difficult, but I think the use-case is worth it.
I agree.
Toggle quote (9 lines)> Good catch. I did not think to handle the toplevel package not being> found! This actually leads to making this a much simpler patch...>> --8<---------------cut here---------------start------------->8---> - (map node-package> + (filter-map node-package> (topological-sort (list (lookup-node package-name version))> --8<---------------cut here---------------end--------------->8---
Cool!
Toggle quote (5 lines)> Yes, this is very much just a stopgap. In your words (from #45984):>> > Well, this patch set are trivial changes that quickly fix the current> > broken situation without a deep revamp.
I agree. Heh! I am consistent with my words. ;-)
Toggle quote (2 lines)> I will follow up with an updated patch.
Cool, thanks!
Cheers,simon
Z
Z
zimoun wrote on 25 Jun 18:37 +0200
Download
[PATCH v3 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210625163749.65196-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by 'repo->guix-package' is false.* tests/import-utils.scm: New tests.--- guix/import/utils.scm | 26 ++++++++++++++------------ tests/import-utils.scm | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-)
Toggle diff (89 lines)diff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..7c6c782917 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,33 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages (toplevel)"+ '()+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))++(test-equal "recursive-import: skip false packages (dependency)"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")-- 2.32.0
Z
Z
zimoun wrote on 25 Jun 18:37 +0200
Download
[PATCH v3 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210625163749.65196-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors.(go-module-recursive-import): Remove 'guard'.* guix/scripts/import/go.scm (guix-import-go): Adjust.* tests/go.scm: Adjust.--- guix/import/go.scm | 43 ++++++++++++++++++++++---------------- guix/scripts/import/go.scm | 6 +++--- tests/go.scm | 2 +- 3 files changed, 29 insertions(+), 22 deletions(-)
Toggle diff (110 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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.scmindex 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.scmindex b088ab50d2..929a8b39d1 100644--- a/tests/go.scm+++ b/tests/go.scm@@ -286,6 +286,6 @@ package.") (nix-base32-string->bytevector "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5") #f)))- (go-module->guix-package "github.com/go-check/check")))))))+ (go-module->guix-package* "github.com/go-check/check"))))))) (test-end "go")-- 2.32.0
Z
Z
zimoun wrote on 25 Jun 18:37 +0200
Download
[PATCH v3 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210625163749.65196-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse when packages are not found.--- guix/import/go.scm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Toggle diff (25 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex d110954664..c859098492 100644--- a/guix/import/go.scm+++ b/guix/import/go.scm@@ -5,6 +5,7 @@ ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -651,7 +652,7 @@ This package and its dependencies won't be imported.~%") (uri->string (http-get-error-uri c)) (http-get-error-code c) (http-get-error-reason c))- (values '() '())))+ (values #f '()))) (receive (package-sexp dependencies) (go-module->guix-package* name #:goproxy goproxy #:version version
base-commit: 88c7c739740b56cab132cf1a3f16392c434408f7-- 2.32.0
Z
Z
zimoun wrote on 25 Jun 18:47 +0200
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196@debbugs.gnu.org)
85v96199ic.fsf_-_@gmail.com
Download
Hi Sarah,
I sent a v3 where your v2 is split into 2 parts. Then I move theguard. Now, your initial example returns:
Toggle snippet (26 lines)$ ./pre-inst-env guix import go git.sr.ht/~sircmpwn/aerc -rfollowing redirection to `https://pkg.go.dev/github.com/zenhack/go.notmuch'...following redirection to `https://pkg.go.dev/github.com/zenhack/go.notmuch'...following redirection to `https://github.com/ProtonMail/go-crypto?go-get=1'...following redirection to `https://github.com/jtolio/gls?go-get=1'...guix import: warning: Something went wrong with ("github.com/neelance/sourcemap" #:goproxy "https://proxy.golang.org" #:version #f #:pin-versions? #f).guix import: warning: Something went wrong with ("github.com/neelance/astrewrite" #:goproxy "https://proxy.golang.org" #:version #f #:pin-versions? #f).(define-public go-git-sr-ht-~sircmpwn-getopt (package (name "go-git-sr-ht-~sircmpwn-getopt") (version "0.0.0-20201218204720-9961a9c6298f") (source
[...]
("go-github-com-creack-pty" ,go-github-com-creack-pty) ("go-git-sr-ht-~sircmpwn-getopt" ,go-git-sr-ht-~sircmpwn-getopt))) (home-page "https://git.sr.ht/~sircmpwn/aerc") (synopsis "aerc") (description "aerc is an email client for your terminal.") (license license:expat)))
WDYT?
Well, I do not know if it is good idea to catch the 'else' case becauseit hiddes what is wrong by the importer. On the other hand, it seems abit friendler for the end-user. :-)

Cheers,simon
S
S
Sarah Morgensen wrote on 27 Jun 06:46 +0200
Download
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 49196@debbugs.gnu.org)
868s2v7w4t.fsf_-_@mgsn.dev
Download
Hi,
Apologies for the delay. Thanks for the work.
zimoun <zimon.toutoune@gmail.com> writes:
Toggle quote (3 lines)> - #:export (go-module->guix-package> + #:export (go-module->guix-package*
Would it be better to export both, in case a callee wants access to theactual errors?
Toggle quote (4 lines)> +(define go-module->guix-package*> + (memoize> + (lambda args
I would remove the memoize from this method (to put it back in lateron), because multiple invocations with errors would only yield one errorreported. I do not think this makes sense if another tool is using this.
Toggle quote (3 lines)> + (else> + (warning (G_ "Something went wrong with ~s.~%") args)
A catch-all is fine, but we should at least report the actual error,even if it's not pretty:
(warning (G_ "Failed to import package ~s.~% reason: ~s") package-name (exception-args c))
However, if we want to move in the direction of proper error handlinglike guix/packages.scm and guix/ui.scm, we can do something like...
Toggle snippet (33 lines)(define (report-import-error package-name error) (cond ((http-get-error? error) [...] (else [...]))))
(define* (go-module->guix-package* module-path #:key (on-error report-import-error) #:allow-other-keys #:rest args) (with-exception-handler (lambda (error) (on-error module-path error) (values #f '())) (lambda () (apply go-module->guix-package module-path args)) #:unwind? #t))
(define* (go-module-recursive-import package-name #:key (goproxy "https://proxy.golang.org") version pin-versions?) (recursive-import package-name #:repo->guix-package (memoize (lambda* (name #:key version repo) (go-module->guix-package* name #:goproxy goproxy #:version version #:pin-versions? pin-versions?))) #:guix-name go-module->guix-package-name #:version version))
Looks like I got a little carried away :) But breaking out the errorreporting gives us the future option of "plugging in" other errorreporting strategies, such as collating them and returning themalongside a programmatic recursive import, or being able to provide alist of packages which failed to import at the end. This will be muchmore useful once we have a proper set of import error conditions.
Too much, perhaps?
Sarah
Z
Z
zimoun wrote on 28 Jun 13:42 +0200
Download
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
CAJ3okZ10Ocf8Wedv1Ep4OiGBxrbFpNDi-_-037iy=XeezOHDjw@mail.gmail.com
Download
Hi,
On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian@mgsn.dev> wrote:
Toggle quote (6 lines)> A catch-all is fine, but we should at least report the actual error,> even if it's not pretty:>> (warning (G_ "Failed to import package ~s.~% reason: ~s")> package-name (exception-args c))
Well, why not, even if I am not convinced the reason is meaningfulbecause 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 ofextract the URL (package-name) from 'args'.
Toggle quote (3 lines)> However, if we want to move in the direction of proper error handling> like guix/packages.scm and guix/ui.scm, we can do something like...
Thanks for the idea. As explained patch#45984 [1], all the UImessages must be in guix/scripts/import and not in guix/import andtherefore, indeed, error reporting needs to be unified between all theimporters and raised accordingly; that's what we are working on withjeko (Jérémy Korwin-Zmijowski) as pair-programming exercise. :-)
1: http://issues.guix.gnu.org/issue/45984
Toggle quote (41 lines)> --8<---------------cut here---------------start------------->8---> (define (report-import-error package-name error)> (cond> ((http-get-error? error)> [...]> (else> [...]))))>> (define* (go-module->guix-package* module-path> #:key (on-error report-import-error)> #:allow-other-keys #:rest args)> (with-exception-handler> (lambda (error)> (on-error module-path error)> (values #f '()))> (lambda () (apply go-module->guix-package module-path args))> #:unwind? #t))>> (define* (go-module-recursive-import package-name> #:key (goproxy "https://proxy.golang.org")> version> pin-versions?)> (recursive-import> package-name> #:repo->guix-package> (memoize> (lambda* (name #:key version repo)> (go-module->guix-package* name #:goproxy goproxy> #:version version> #:pin-versions? pin-versions?)))> #:guix-name go-module->guix-package-name> #:version version))> --8<---------------cut here---------------end--------------->8--->> Looks like I got a little carried away :) But breaking out the error> reporting gives us the future option of "plugging in" other error> reporting strategies, such as collating them and returning them> alongside a programmatic recursive import, or being able to provide a> list of packages which failed to import at the end. This will be much> more useful once we have a proper set of import error conditions.
Back to the initial patch, I think it is better to simply fix with theminor improvements of v3 your proposed and let this last proposal for#45984; feel free to comment there. ;-)
Cheers,simon
Z
Z
zimoun wrote on 28 Jun 18:20 +0200
Download
[PATCH v4 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210628162014.15319-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse when packages are not found.--- guix/import/go.scm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Toggle diff (17 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 5e23d6a2b3..a2863c79ad 100644--- a/guix/import/go.scm+++ b/guix/import/go.scm@@ -653,7 +653,7 @@ This package and its dependencies won't be imported.~%") (uri->string (http-get-error-uri c)) (http-get-error-code c) (http-get-error-reason c))- (values '() '())))+ (values #f '()))) (receive (package-sexp dependencies) (go-module->guix-package* name #:goproxy goproxy #:version version
base-commit: 77c9c5c103ed2d09a43709bb78bc0b13a399e797-- 2.32.0
Z
Z
zimoun wrote on 28 Jun 18:20 +0200
Download
[PATCH v4 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210628162014.15319-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by 'repo->guix-package' is false.* tests/import-utils.scm: New tests.--- guix/import/utils.scm | 26 ++++++++++++++------------ tests/import-utils.scm | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-)
Toggle diff (89 lines)diff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..7c6c782917 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,33 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages (toplevel)"+ '()+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))++(test-equal "recursive-import: skip false packages (dependency)"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")-- 2.32.0
Z
Z
zimoun wrote on 28 Jun 18:20 +0200
Download
[PATCH v4 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210628162014.15319-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors.(go-module-recursive-import): Remove 'guard'.* guix/scripts/import/go.scm (guix-import-go): Adjust.* tests/go.scm: Adjust.--- guix/import/go.scm | 49 +++++++++++++++++++++++--------------- guix/scripts/import/go.scm | 6 ++--- tests/go.scm | 2 +- 3 files changed, 34 insertions(+), 23 deletions(-)
Toggle diff (112 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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.scmindex 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.scmindex b088ab50d2..929a8b39d1 100644--- a/tests/go.scm+++ b/tests/go.scm@@ -286,6 +286,6 @@ package.") (nix-base32-string->bytevector "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5") #f)))- (go-module->guix-package "github.com/go-check/check")))))))+ (go-module->guix-package* "github.com/go-check/check"))))))) (test-end "go")-- 2.32.0
S
S
Sarah Morgensen wrote on 28 Jun 19:13 +0200
Download
Re: bug#49196: [PATCH] import: utils: 'recursive-import' skips unfound packages
(name . zimoun)(address . zimon.toutoune@gmail.com)
86lf6t7w0g.fsf_-_@mgsn.dev
Download
zimoun <zimon.toutoune@gmail.com> writes:
Hello,
Thanks for the v4.
Toggle quote (16 lines)> Hi,>> On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen <iskarian@mgsn.dev> wrote:>>> A catch-all is fine, but we should at least report the actual error,>> even if it's not pretty:>>>> (warning (G_ "Failed to import package ~s.~% reason: ~s")>> package-name (exception-args c))>> Well, why not, even if I am not convinced the reason is meaningful> because it is mostly an incorrect parsing which returns:>> reason: ("match" "no matching pattern" #f).>
Yes, it is a less than ideal compromise... I could not quickly figureout how to properly format it without a lot of complexity (likeguix/ui.scm does in 'call-with-error-handling'). I found it hard to readthe full exception object, but I would not object strongly to printingthe full exception object either. As you say, your patch will fix itanyway ;)
Toggle quote (3 lines)> and I think it is better to display the complete 'args' instead of> extract the URL (package-name) from 'args'.
You're not wrong; I was just trying to keep it somewhat consistent withthe other error message.
Toggle quote (6 lines)>> However, if we want to move in the direction of proper error handling>> like guix/packages.scm and guix/ui.scm, we can do something like...>> Thanks for the idea. As explained patch#45984 [1], all the UI> messages must be in guix/scripts/import and not in guix/import and
Yes, this was my secret trick: having separated out the error reporting,it could be easily be moved to scripts/import later.
Toggle quote (4 lines)> therefore, indeed, error reporting needs to be unified between all the> importers and raised accordingly; that's what we are working on with> jeko (Jérémy Korwin-Zmijowski) as pair-programming exercise. :-)
I look forward to the results!
Toggle quote (4 lines)> Back to the initial patch, I think it is better to simply fix with the> minor improvements of v3 your proposed and let this last proposal for> #45984; feel free to comment there. ;-)
I agree. Your v4 looks good to me, except...
Toggle quote (3 lines)> #:repo->guix-package> (lambda* (name #:key version repo)
I apologize for not being clear earlier; by "put [memoize] back in lateron" I meant "later on in the call chain," e.g.
#:repo->guix-package+ (memoize (lambda* (name #:key version repo)
That's my only nit this time! ;) Thanks for bearing with me.
Regards,Sarah
Z
Z
zimoun wrote on 29 Jun 12:52 +0200
Download
[PATCH v5 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210629105237.45517-2-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by 'repo->guix-package' is false.* tests/import-utils.scm: New tests.--- guix/import/utils.scm | 26 ++++++++++++++------------ tests/import-utils.scm | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-)
Toggle diff (89 lines)diff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..7c6c782917 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,33 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages (toplevel)"+ '()+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))++(test-equal "recursive-import: skip false packages (dependency)"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")-- 2.32.0
Z
Z
zimoun wrote on 29 Jun 12:52 +0200
Download
[PATCH v5 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)(address . iskarian@mgsn.dev)
20210629105237.45517-1-zimon.toutoune@gmail.com
Download
From: Sarah Morgensen <iskarian@mgsn.dev>
* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse when packages are not found.--- guix/import/go.scm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Toggle diff (17 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 5e23d6a2b3..a2863c79ad 100644--- a/guix/import/go.scm+++ b/guix/import/go.scm@@ -653,7 +653,7 @@ This package and its dependencies won't be imported.~%") (uri->string (http-get-error-uri c)) (http-get-error-code c) (http-get-error-reason c))- (values '() '())))+ (values #f '()))) (receive (package-sexp dependencies) (go-module->guix-package* name #:goproxy goproxy #:version version
base-commit: 5ed105a8bb1a812975496dc3a091596355a0234c-- 2.32.0
Z
Z
zimoun wrote on 29 Jun 12:52 +0200
Download
[PATCH v5 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)
20210629105237.45517-3-zimon.toutoune@gmail.com
Download
* guix/import/go.scm (go-module->guix-package*): Handle errors, removememoize.(go-module-recursive-import): Remove 'guard', add memoize.* guix/scripts/import/go.scm (guix-import-go): Adjust.* tests/go.scm: Adjust.--- guix/import/go.scm | 52 +++++++++++++++++++++++--------------- guix/scripts/import/go.scm | 6 ++--- tests/go.scm | 2 +- 3 files changed, 36 insertions(+), 24 deletions(-)
Toggle diff (115 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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.scmindex 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.scmindex b088ab50d2..929a8b39d1 100644--- a/tests/go.scm+++ b/tests/go.scm@@ -286,6 +286,6 @@ package.") (nix-base32-string->bytevector "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5") #f)))- (go-module->guix-package "github.com/go-check/check")))))))+ (go-module->guix-package* "github.com/go-check/check"))))))) (test-end "go")-- 2.32.0
Z
Z
zimoun wrote on 2 Jul 18:26 +0200
Download
control message for bug #49196
(address . control@debbugs.gnu.org)
87lf6on0l9.fsf@gmail.com
Download
retitle 49196 [PATCH] "guix import go" Improve error handlingquit
S
S
Sarah Morgensen wrote on 6 Aug 20:04 +0200
Download
[PATCH v6 1/3] import: go: Return false for package not found.
(address . 49196@debbugs.gnu.org)
5d11fc04400d26de06efba709cf2e1b14a1d5949.1628272957.git.iskarian@mgsn.dev
Download
* guix/import/go.scm (go-module-recursive-import): Explicitly returnfalse when packages are not found.---Hello Guix,
I took the liberty of rebasing this patchset since it no longer applies onmaster.
Is there any one else who would like to comment? If not, I believe this hasreached a final state.
--Sarah
guix/import/go.scm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Toggle diff (17 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 617a0d0e23..a4775f973f 100644--- a/guix/import/go.scm+++ b/guix/import/go.scm@@ -668,7 +668,7 @@ This package and its dependencies won't be imported.~%") (uri->string (http-get-error-uri c)) (http-get-error-code c) (http-get-error-reason c))- (values '() '())))+ (values #f '()))) (receive (package-sexp dependencies) (go-module->guix-package* name #:goproxy goproxy #:version version
base-commit: c8e2be3b32fe784a9db52d8a1a12902ab12ae7cb-- 2.31.1
S
S
Sarah Morgensen wrote on 6 Aug 20:05 +0200
Download
[PATCH v6 2/3] import: utils: Skip not found packages.
(address . 49196@debbugs.gnu.org)
ae55285e772acc7fa3ec5b95d6de8f1433860a4c.1628272957.git.iskarian@mgsn.dev
Download
* guix/import/utils.scm (recursive-import): Skip packages when thepackage returned by 'repo->guix-package' is false.* tests/import-utils.scm: New tests.--- guix/import/utils.scm | 26 ++++++++++++++------------ tests/import-utils.scm | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-)
Toggle diff (89 lines)diff --git a/guix/import/utils.scm b/guix/import/utils.scmindex 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.scmindex 874816442e..7c6c782917 100644--- a/tests/import-utils.scm+++ b/tests/import-utils.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net> ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;;@@ -64,6 +65,33 @@ '()))) #:guix-name identity)) +(test-equal "recursive-import: skip false packages (toplevel)"+ '()+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))++(test-equal "recursive-import: skip false packages (dependency)"+ '((package+ (name "foo")+ (inputs `(("bar" ,bar)))))+ (recursive-import "foo"+ #:repo 'repo+ #:repo->guix-package+ (match-lambda*+ (("foo" #:version #f #:repo 'repo)+ (values '(package+ (name "foo")+ (inputs `(("bar" ,bar))))+ '("bar")))+ (("bar" #:version #f #:repo 'repo)+ (values #f '())))+ #:guix-name identity))+ (test-assert "alist->package with simple source" (let* ((meta '(("name" . "hello") ("version" . "2.10")-- 2.31.1
S
S
Sarah Morgensen wrote on 6 Aug 20:05 +0200
Download
[PATCH v6 3/3] import: go: Improve error handling.
(address . 49196@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
a4cee8f58e2187619e7a7fc3fc9b73ca04ae696f.1628272957.git.iskarian@mgsn.dev
Download
From: zimoun <zimon.toutoune@gmail.com>
* guix/import/go.scm (go-module->guix-package*): Handle errors, removememoize.(go-module-recursive-import): Remove 'guard', add memoize.* guix/scripts/import/go.scm (guix-import-go): Adjust.* tests/go.scm: Adjust.--- guix/import/go.scm | 50 +++++++++++++++++++++++--------------- guix/scripts/import/go.scm | 6 ++--- tests/go.scm | 2 +- 3 files changed, 35 insertions(+), 23 deletions(-)
Toggle diff (113 lines)diff --git a/guix/import/go.scm b/guix/import/go.scmindex 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.scmindex 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.scmindex 6749f4585f..9e7223ff7c 100644--- a/tests/go.scm+++ b/tests/go.scm@@ -410,6 +410,6 @@ package.") (nix-base32-string->bytevector "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5") #f)))- (go-module->guix-package "github.com/go-check/check")))))))+ (go-module->guix-package* "github.com/go-check/check"))))))) (test-end "go")-- 2.31.1
L
L
Ludovic Courtès wrote on 1 Sep 23:39 +0200
Download
Re: bug#49196: [PATCH] "guix import go" Improve error handling
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)(address . 49196-done@debbugs.gnu.org)
87k0k0561n.fsf_-_@gnu.org
Download
Hi,
Sarah Morgensen <iskarian@mgsn.dev> skribis:
Toggle quote (11 lines)> * guix/import/go.scm (go-module-recursive-import): Explicitly return> false when packages are not found.> ---> Hello Guix,>> I took the liberty of rebasing this patchset since it no longer applies on> master.>> Is there any one else who would like to comment? If not, I believe this has> reached a final state.
I think so too. :-)
Applied; thank you and thanks zimoun, and apologies for the delay!
Ludo’.
Closed
?
Your comment

Commenting via the web interface is currently disabled.

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