[PATCH] guix: describe: Improve package provenance tracking.

  • Done
  • quality assurance status badge
Details
2 participants
  • Julien Lepiller
  • Ludovic Courtès
Owner
unassigned
Submitted by
Julien Lepiller
Severity
normal
J
J
Julien Lepiller wrote on 31 Oct 2020 15:40
(address . guix-patches@gnu.org)
20201031144007.25531-1-julien@lepiller.eu
%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(-)

Toggle diff (46 lines)
diff --git a/guix/describe.scm b/guix/describe.scm
index 05bf99eb58..fe5921a3b3 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))))
+ (canonicalize-path (search-path %load-path file)))))
(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
L
L
Ludovic Courtès wrote on 31 Oct 2020 18:30
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 44344@debbugs.gnu.org)
87pn4ytiao.fsf@gnu.org
Hi Julien,

Julien Lepiller <julien@lepiller.eu> skribis:

Toggle quote (5 lines)
> %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.

[...]

Toggle quote (5 lines)
> (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?

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

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

Thanks,
Ludo’.
J
J
Julien Lepiller wrote on 1 Nov 2020 14:29
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44344@debbugs.gnu.org)
20201101142919.5bcafefb@tachikoma.lepiller.eu
Le Sat, 31 Oct 2020 18:30:07 +0100,
Ludovic Courtès <ludo@gnu.org> a écrit :

Toggle quote (21 lines)
> 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).

Toggle quote (10 lines)
>
> ‘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).

Toggle quote (6 lines)
>
> Maybe it’s OK here, but I mention this for completeness.
>
> Thanks,
> Ludo’.

Attached is an updated 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(-)

Toggle diff (46 lines)
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
L
L
Ludovic Courtès wrote on 2 Nov 2020 17:15
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 44344@debbugs.gnu.org)
87r1pbivl9.fsf@gnu.org
Hi Julien,

Julien Lepiller <julien@lepiller.eu> skribis:

Toggle quote (3 lines)
> 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:

Toggle snippet (22 lines)
$ 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")))))

Do you have a reproducer?

Thanks,
Ludo’.
J
J
Julien Lepiller wrote on 2 Nov 2020 17:42
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44344@debbugs.gnu.org)
CFACD0A8-BAFC-43B7-AB3D-AA496935CF85@lepiller.eu
Le 2 novembre 2020 11:15:14 GMT-05:00, "Ludovic Courtès" <ludo@gnu.org> a écrit :
Toggle quote (49 lines)
>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.

Toggle quote (3 lines)
>
>Thanks,
>Ludo’.
L
L
Ludovic Courtès wrote on 3 Nov 2020 10:17
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 44344@debbugs.gnu.org)
87d00uhk8u.fsf@gnu.org
Hi,

Julien Lepiller <julien@lepiller.eu> skribis:

Toggle quote (4 lines)
> 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:

Toggle snippet (26 lines)
$ 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")))))

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

Ludo’.
J
J
Julien Lepiller wrote on 3 Nov 2020 12:25
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44344-close@debbugs.gnu.org)
AD3B00C2-9689-41BA-8AA4-7410631DAAD0@lepiller.eu
Le 3 novembre 2020 04:17:53 GMT-05:00, "Ludovic Courtès" <ludo@gnu.org> a écrit :
Toggle quote (59 lines)
>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.

Toggle quote (2 lines)
>
>Ludo’.
?