diff mbox series

[bug#44344] guix: describe: Improve package provenance tracking.

Message ID 20201101142919.5bcafefb@tachikoma.lepiller.eu
State Accepted
Headers show
Series [bug#44344] guix: describe: Improve package provenance tracking. | expand

Checks

Context Check Description
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Julien Lepiller Nov. 1, 2020, 1:29 p.m. UTC 
Le Sat, 31 Oct 2020 18:30:07 +0100,
Ludovic Courtès <ludo@gnu.org> a écrit :

> Hi Julien,
> 
> Julien Lepiller <julien@lepiller.eu> skribis:
> 
> > %load-path lists ~/.config/guix/current before individual channels.
> >  We use canonicalize-path to get the store path for channel
> > packages.
> >
> > * guix/describe.scm (package-provenance): Use canonicalize-path.
> 
> [...]
> 
> >       (let ((file (if (string-prefix? "/" file)
> >                       file
> > -                     (search-path %load-path file))))
> > +                     (canonicalize-path (search-path %load-path
> > file)))))
> 
> Could you explain what problem it solves, perhaps with a simple
> reproducer?

Thanks for the answer!

Maybe this is only an issue in guix repl, but when looking for the
provenance of a package that's defined in a channel, I always get `#f`:

guix repl
,use (guix describe)
,use (gnu packages gettext)
,use (my channel packages)
(package-provenance po4a) -> I get the guix channel
(package-provenance my-package) -> I get #f

To me, this is caused by the search-path returning a path in
~/.config/guix/current instead of /gnu/store as it does for po4a:

(search-path %load-path "gnu/packages/gettext.scm") -> /gnu/store...
(search-path %load-path "my/channel/packages.scm") -> /home/...

The reason why I use canonicalize-path here is because doing so, I get
a path in the store, that is the content of the channel, so the
procedure can return that channel as the provenance for the package.
Unfortunately, %load-path seems to be weirdly set when using
pre-inst-env, so I'm not sure how to test (channel modules are not even
found).

> 
> ‘search-path’ can return #f (there’s a test right below), so this
> should probably be: (and=> (search-path …) canonicalize-path).
> 
> As a rule of thumb, I think twice before calling ‘canonicalize-path’
> because (1) it’s expensive (lots of stat(2) calls), and (2) it can
> have undesirable effects on the UI (messages mention a file name
> other than the one the user typed) and elsewhere (on logic that looks
> at what the file name looks like).

I think the result of canonicalize-path is only used internally in that
function to find the channel a package comes from, but is never used
outside of this function (it's not returned).

> 
> Maybe it’s OK here, but I mention this for completeness.
> 
> Thanks,
> Ludo’.

Attached is an updated patch.

Comments

Ludovic Courtès Nov. 2, 2020, 4:15 p.m. UTC | #1 
Hi Julien,

Julien Lepiller <julien@lepiller.eu> skribis:

> Maybe this is only an issue in guix repl, but when looking for the
> provenance of a package that's defined in a channel, I always get `#f`:

I can’t reproduce the bug:

--8<---------------cut here---------------start------------->8---
$ cat /tmp/channels.scm
(cons (channel
       (name 'guix-hpc)
       (url "https://gitlab.inria.fr/guix-hpc/guix-hpc.git"))
      %default-channels)
$ guix time-machine -C /tmp/channels.scm -- repl
Updating channel 'guix-hpc' from Git repository at 'https://gitlab.inria.fr/guix-hpc/guix-hpc.git'...
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...

[...]

scheme@(guix-user)> ,use(guix)
scheme@(guix-user)> ,use(gnu)
scheme@(guix-user)> ,use(guix describe)
scheme@(guix-user)> ,use(inria storm)
scheme@(guix-user)> (package-provenance starpu)
$1 = ((repository (version 0) (url "https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit "794928a9062529cb75c019454d7bd31b8cf83cb7") (introduction (channel-introduction (version 0) (commit "9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))) (repository (version 0) (url "https://gitlab.inria.fr/guix-hpc/ guix-hpc.git") (branch "master") (commit "bf3afdd85c68ee022b863da72b90e0c304b11bee")))
scheme@(guix-user)> ,use(gnu packages base)
scheme@(guix-user)> (package-provenance coreutils)
$2 = ((repository (version 0) (url "https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit "794928a9062529cb75c019454d7bd31b8cf83cb7") (introduction (channel-introduction (version 0) (commit "9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))))
--8<---------------cut here---------------end--------------->8---

Do you have a reproducer?

Thanks,
Ludo’.
Julien Lepiller Nov. 2, 2020, 4:42 p.m. UTC | #2 
Le 2 novembre 2020 11:15:14 GMT-05:00, "Ludovic Courtès" <ludo@gnu.org> a écrit :
>Hi Julien,
>
>Julien Lepiller <julien@lepiller.eu> skribis:
>
>> Maybe this is only an issue in guix repl, but when looking for the
>> provenance of a package that's defined in a channel, I always get
>`#f`:
>
>I can’t reproduce the bug:
>
>--8<---------------cut here---------------start------------->8---
>$ cat /tmp/channels.scm
>(cons (channel
>       (name 'guix-hpc)
>       (url "https://gitlab.inria.fr/guix-hpc/guix-hpc.git"))
>      %default-channels)
>$ guix time-machine -C /tmp/channels.scm -- repl
>Updating channel 'guix-hpc' from Git repository at
>'https://gitlab.inria.fr/guix-hpc/guix-hpc.git'...
>Updating channel 'guix' from Git repository at
>'https://git.savannah.gnu.org/git/guix.git'...
>
>[...]
>
>scheme@(guix-user)> ,use(guix)
>scheme@(guix-user)> ,use(gnu)
>scheme@(guix-user)> ,use(guix describe)
>scheme@(guix-user)> ,use(inria storm)
>scheme@(guix-user)> (package-provenance starpu)
>$1 = ((repository (version 0) (url
>"https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit
>"794928a9062529cb75c019454d7bd31b8cf83cb7") (introduction
>(channel-introduction (version 0) (commit
>"9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA
>F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))) (repository (version 0) (url
>"https://gitlab.inria.fr/guix-hpc/ guix-hpc.git") (branch "master")
>(commit "bf3afdd85c68ee022b863da72b90e0c304b11bee")))
>scheme@(guix-user)> ,use(gnu packages base)
>scheme@(guix-user)> (package-provenance coreutils)
>$2 = ((repository (version 0) (url
>"https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit
>"794928a9062529cb75c019454d7bd31b8cf83cb7") (introduction
>(channel-introduction (version 0) (commit
>"9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA
>F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))))
>--8<---------------cut here---------------end--------------->8---
>
>Do you have a reproducer?

So it seems that I cannot reproduce che bug either when using guix timekmachine and a channel that has not been pulled by guix. I suppose in the case of timekmachine, it doesn't add ~/.config/guix to the search path, so the information is found.

Can you try pulling this channel with guix itself? You should be able to reproduce then.

>
>Thanks,
>Ludo’.
Ludovic Courtès Nov. 3, 2020, 9:17 a.m. UTC | #3 
Hi,

Julien Lepiller <julien@lepiller.eu> skribis:

> So it seems that I cannot reproduce che bug either when using guix timekmachine and a channel that has not been pulled by guix. I suppose in the case of timekmachine, it doesn't add ~/.config/guix to the search path, so the information is found.
>
> Can you try pulling this channel with guix itself? You should be able to reproduce then.

I tried this and got the same result:

--8<---------------cut here---------------start------------->8---
$ guix pull -C /tmp/channels.scm -p /tmp/test
Updating channel 'guix-hpc' from Git repository at 'https://gitlab.inria.fr/guix-hpc/guix-hpc.git'...
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...

[...]

$ /tmp/test/bin/guix repl
GNU Guile 3.0.4
Copyright (C) 1995-2020 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guix-user)> ,use(guix)
scheme@(guix-user)> ,use(gnu)
scheme@(guix-user)> ,use(guix describe)
scheme@(guix-user)> ,use(inria storm)
scheme@(guix-user)> (package-provenance starpu)
$1 = ((repository (version 0) (url "https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit "7a9e68cc8750a9b378ae54a42d3e882a2e548c95") (introduction (channel-introduction (version 0) (commit "9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))) (repository (version 0) (url "https://gitlab.inria.fr/guix-hpc/guix-hpc.git") (branch "master") (commit "bf3afdd85c68ee022b863da72b90e0c304b11bee")))
scheme@(guix-user)> ,use(gnu packages base)
scheme@(guix-user)> (package-provenance grep)
$2 = ((repository (version 0) (url "https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit "7a9e68cc8750a9b378ae54a42d3e882a2e548c95") (introduction (channel-introduction (version 0) (commit "9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))))
--8<---------------cut here---------------end--------------->8---

Let me know if you can reproduce it somehow, otherwise we can close this
issue.

Ludo’.
Julien Lepiller Nov. 3, 2020, 11:25 a.m. UTC | #4 
Le 3 novembre 2020 04:17:53 GMT-05:00, "Ludovic Courtès" <ludo@gnu.org> a écrit :
>Hi,
>
>Julien Lepiller <julien@lepiller.eu> skribis:
>
>> So it seems that I cannot reproduce che bug either when using guix
>timekmachine and a channel that has not been pulled by guix. I suppose
>in the case of timekmachine, it doesn't add ~/.config/guix to the
>search path, so the information is found.
>>
>> Can you try pulling this channel with guix itself? You should be able
>to reproduce then.
>
>I tried this and got the same result:
>
>--8<---------------cut here---------------start------------->8---
>$ guix pull -C /tmp/channels.scm -p /tmp/test
>Updating channel 'guix-hpc' from Git repository at
>'https://gitlab.inria.fr/guix-hpc/guix-hpc.git'...
>Updating channel 'guix' from Git repository at
>'https://git.savannah.gnu.org/git/guix.git'...
>
>[...]
>
>$ /tmp/test/bin/guix repl
>GNU Guile 3.0.4
>Copyright (C) 1995-2020 Free Software Foundation, Inc.
>
>Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
>This program is free software, and you are welcome to redistribute it
>under certain conditions; type `,show c' for details.
>
>Enter `,help' for help.
>scheme@(guix-user)> ,use(guix)
>scheme@(guix-user)> ,use(gnu)
>scheme@(guix-user)> ,use(guix describe)
>scheme@(guix-user)> ,use(inria storm)
>scheme@(guix-user)> (package-provenance starpu)
>$1 = ((repository (version 0) (url
>"https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit
>"7a9e68cc8750a9b378ae54a42d3e882a2e548c95") (introduction
>(channel-introduction (version 0) (commit
>"9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA
>F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))) (repository (version 0) (url
>"https://gitlab.inria.fr/guix-hpc/guix-hpc.git") (branch "master")
>(commit "bf3afdd85c68ee022b863da72b90e0c304b11bee")))
>scheme@(guix-user)> ,use(gnu packages base)
>scheme@(guix-user)> (package-provenance grep)
>$2 = ((repository (version 0) (url
>"https://git.savannah.gnu.org/git/guix.git") (branch "master") (commit
>"7a9e68cc8750a9b378ae54a42d3e882a2e548c95") (introduction
>(channel-introduction (version 0) (commit
>"9edb3f66fd807b096b48283debdcddccfea34bad") (signer "BBB0 2DDF 2CEA
>F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA")))))
>--8<---------------cut here---------------end--------------->8---
>
>Let me know if you can reproduce it somehow, otherwise we can close
>this
>issue.

Ah! I found where the issue was. Since I'm using the guix home manager, I added ~/.config/guix to my GUILE_LOAD_PATH, which is added to %load-path very early, so for anything that's added afterwards, I get #f. The reason I had to extend GUILE_LOAD_PATH was to be able to use the guix home subcommand. Without it the command was not looked up with the correct search path and was not found.

>
>Ludo’.
diff mbox series

Patch

From 5998d636ea17a192fa1cf1720c38c9b4676d0189 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sat, 31 Oct 2020 15:35:54 +0100
Subject: [PATCH] guix: describe: Improve package provenance tracking.

%load-path lists ~/.config/guix/current before individual channels.  We
use canonicalize-path to get the store path for channel packages.

* guix/describe.scm (package-provenance): Use canonicalize-path.
---
 guix/describe.scm | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/guix/describe.scm b/guix/describe.scm
index 05bf99eb58..ecda90897a 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -132,25 +132,25 @@  property of manifest entries, or #f if it could not be determined."
     (file
      (let ((file (if (string-prefix? "/" file)
                      file
-                     (search-path %load-path file))))
+                     (and=> (search-path %load-path file) canonicalize-path))))
        (and file
             (string-prefix? (%store-prefix) file)
 
             ;; Always store information about the 'guix' channel and
             ;; optionally about the specific channel FILE comes from.
-            (or (let ((main  (and=> (find (lambda (entry)
-                                            (string=? "guix"
-                                                      (manifest-entry-name entry)))
-                                          (current-profile-entries))
-                                    entry-source))
-                      (extra (any (lambda (entry)
-                                    (let ((item (manifest-entry-item entry)))
-                                      (and (string-prefix? item file)
-                                           (entry-source entry))))
-                                  (current-profile-entries))))
-                  (and main
-                       `(,main
-                         ,@(if extra (list extra) '()))))))))))
+            (let ((main  (and=> (find (lambda (entry)
+                                        (string=? "guix"
+                                                  (manifest-entry-name entry)))
+                                      (current-profile-entries))
+                                entry-source))
+                  (extra (any (lambda (entry)
+                                (let ((item (manifest-entry-item entry)))
+                                  (and (string-prefix? item file)
+                                       (entry-source entry))))
+                              (current-profile-entries))))
+              (and main
+                   `(,main
+                     ,@(if extra (list extra) '())))))))))
 
 (define (manifest-entry-with-provenance entry)
   "Return ENTRY with an additional 'provenance' property if it's not already
-- 
2.28.0