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

[zimoun]

Hi,

Thanks for the patch.

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

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

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

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


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

Yes, there is an inconsistency...

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

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

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

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

with:

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

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


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

--8<---------------cut here---------------start------------->8---
r@bioinfomeary01-Precision-7820-Tower$ guix import gem do-not-exist -r
#f

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

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

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

> diff --git a/tests/import-utils.scm b/tests/import-utils.scm
> index 874816442e..1b728104e0 100644
> --- a/tests/import-utils.scm
> +++ b/tests/import-utils.scm

Thanks for adding a test. :-)


Cheers,
simon