Message ID | 20201031144007.25531-1-julien@lepiller.eu |
---|---|
State | Accepted |
Headers | show |
Series | [bug#44344] guix: describe: Improve package provenance tracking. | expand |
Context | Check | Description |
---|---|---|
cbaines/submitting builds | success | |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
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? ‘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’.
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