GNU bug report logs - #76143
[PATCH] gnu: Canonicalize paths before comparing.

Previous Next

To reply to this bug, email your comments to 76143 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: guix-patches <at> gnu.org
Cc: Noé Lopez <noe <at> xn--no-cja.eu>
Subject: [PATCH] gnu: Canonicalize paths before comparing.
Date: Sat,  8 Feb 2025 14:36:08 +0100

From: Noé Lopez <noelopez <at> free.fr>

The comparison would fail if the load path for guix was not already
canonicalized, since it is doing a string comparison.

* gnu/packages.scm (%patch-path): Canonicalize paths before
comparing.
* guix/ui.scm (try-canonicalize-path): Move to (guix utils).
* guix/utils.scm (try-canonicalize-path): New function.

Change-Id: Id5d51ce483af74ac4e122563d84cc3e8d78c3246
---
 gnu/packages.scm | 11 ++++++-----
 guix/ui.scm      | 14 --------------
 guix/utils.scm   | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index bdd5d21940..d043d0616d 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -167,11 +167,12 @@ (define %patch-path
   ;; Define it after '%package-module-path' so that '%load-path' contains user
   ;; directories, allowing patches in $GUIX_PACKAGE_PATH to be found.
   (make-parameter
-   (map (lambda (directory)
-          (if (string=? directory %distro-root-directory)
-              (string-append directory "/gnu/packages/patches")
-              directory))
-        %load-path)))
+   (let ((root (try-canonicalize-path %distro-root-directory)))
+     (map (lambda (directory)
+            (if (string=? (try-canonicalize-path directory) root)
+                (string-append directory "/gnu/packages/patches")
+                directory))
+          %load-path))))
 
 ;; This procedure is used by Emacs-Guix up to 0.5.1.1, so keep it for now.
 ;; See <https://github.com/alezost/guix.el/issues/30>.
diff --git a/guix/ui.scm b/guix/ui.scm
index 87a448bf72..a3a9bf4e42 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -206,20 +206,6 @@ (define-syntax-rule (without-compiler-optimizations exp)
   (parameterize (((@ (system base compile) default-optimization-level) 1))
     exp))
 
-(define (try-canonicalize-path file)
-  "Like 'canonicalize-path', but return FILE as-is if 'canonicalize-path'
-throws.
-
-This is necessary for corner cases where 'canonicalize-path' fails.  One
-example is on Linux when a /dev/fd/N file denotes a pipe, represented as a
-symlink to a non-existent file like 'pipe:[1234]', as in this example:
-
-  sh -c 'stat $(readlink -f /dev/fd/1)' | cat"
-  (catch 'system-error
-    (lambda ()
-      (canonicalize-path file))
-    (const file)))
-
 (define* (load* file user-module
                 #:key (on-error 'nothing-special))
   "Load the user provided Scheme source code FILE."
diff --git a/guix/utils.scm b/guix/utils.scm
index b6cf5aea4f..6e5b6b6caf 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -162,6 +162,7 @@ (define-module (guix utils)
             compressed-output-port
             call-with-compressed-output-port
             canonical-newline-port
+            try-canonicalize-path
 
             string-distance
             string-closest
@@ -1150,6 +1151,20 @@ (define (canonical-newline-port port)
                                  get-position
                                  set-position!
                                  close))
+
+(define (try-canonicalize-path file)
+  "Like 'canonicalize-path', but return FILE as-is if 'canonicalize-path'
+throws.
+
+This is necessary for corner cases where 'canonicalize-path' fails.  One
+example is on Linux when a /dev/fd/N file denotes a pipe, represented as a
+symlink to a non-existent file like 'pipe:[1234]', as in this example:
+
+  sh -c 'stat $(readlink -f /dev/fd/1)' | cat"
+  (catch 'system-error
+    (lambda ()
+      (canonicalize-path file))
+    (const file)))
 
 ;;;
 ;;; Source location.

base-commit: 9c36d38614079611aebe4721b9e087f98e57b1b3
-- 
2.48.1

Message #8 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Noé Lopez <noe <at> xn--no-cja.eu>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 76143 <at> debbugs.gnu.org,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76143] [PATCH] gnu: Canonicalize paths before comparing.
Date: Sun, 09 Feb 2025 12:28:55 +0100

Hi,

Noé Lopez <noe <at> noé.eu> skribis:

> --- a/gnu/packages.scm
> +++ b/gnu/packages.scm
> @@ -167,11 +167,12 @@ (define %patch-path
>    ;; Define it after '%package-module-path' so that '%load-path' contains user
>    ;; directories, allowing patches in $GUIX_PACKAGE_PATH to be found.
>    (make-parameter
> -   (map (lambda (directory)
> -          (if (string=? directory %distro-root-directory)
> -              (string-append directory "/gnu/packages/patches")
> -              directory))
> -        %load-path)))
> +   (let ((root (try-canonicalize-path %distro-root-directory)))
> +     (map (lambda (directory)
> +            (if (string=? (try-canonicalize-path directory) root)
> +                (string-append directory "/gnu/packages/patches")
> +                directory))
> +          %load-path))))

I’m not sure what the goal is but please keep in mind that
‘canonicalize-path’ is expensive in terms of system calls (especially if
‘%load-path’ is long, and we’d pay it for all program startup times),
and that the comparison here remains brittle (checking the dev/ino
fields of ‘stat’ would be more accurate).

Thanks,
Ludo’.

Message #11 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: 76143 <at> debbugs.gnu.org
Cc: Noé Lopez <noe <at> xn--no-cja.eu>
Subject: [PATCH v2] gnu: Find patches directory through symlinks.
Date: Sun, 16 Feb 2025 15:08:56 +0100

From: Noé Lopez <noelopez <at> free.fr>

This fixes a bug where patches would not be found in %patch-path when the
Guile load path would contain a different path (via symlink or trailing slash)
to the %distro-root-directory than what was previously found.  We use stat to
make sure that two different paths to the same directory are still matched.

For example: if the Guile path was /guix/ and %distro-root-directory was
/guix, patches would not be found even though the two directories are the
same.

* gnu/packages.scm (%patch-path): Compare directories with directory=?.
* guix/utils.scm (directory=?): New procedure.
* tests/utils.scm: Add tests for directory=?.

Change-Id: I73f65b6c050cdeff85637e13ffd0319dcc1d4958
---
 gnu/packages.scm |  2 +-
 guix/utils.scm   | 15 +++++++++++++++
 tests/utils.scm  | 13 +++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index bdd5d21940..5cad0d50ff 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -168,7 +168,7 @@ (define %patch-path
   ;; directories, allowing patches in $GUIX_PACKAGE_PATH to be found.
   (make-parameter
    (map (lambda (directory)
-          (if (string=? directory %distro-root-directory)
+          (if (directory=? directory %distro-root-directory)
               (string-append directory "/gnu/packages/patches")
               directory))
         %load-path)))
diff --git a/guix/utils.scm b/guix/utils.scm
index b6cf5aea4f..a2537b4285 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -21,6 +21,7 @@
 ;;; Copyright © 2023 Zheng Junjie <873216071 <at> qq.com>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello <at> foundationdevices.com>
 ;;; Copyright © 2024 Herman Rimm <herman <at> rimm.ee>
+;;; Copyright © 2025 Noé Lopez <noelopez <at> free.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -162,6 +163,7 @@ (define-module (guix utils)
             compressed-output-port
             call-with-compressed-output-port
             canonical-newline-port
+            directory=?
 
             string-distance
             string-closest
@@ -1150,6 +1152,19 @@ (define (canonical-newline-port port)
                                  get-position
                                  set-position!
                                  close))
+
+(define* (directory=? directory #:rest directories)
+  (define (dev+ino directory)
+    (and-let* ((stats (stat directory #f))
+               (dev (stat:dev stats))
+               (ino (stat:ino stats)))
+      (cons dev ino)))
+  (define check (dev+ino directory))
+  (and check
+       (fold (lambda (element acc)
+               (and acc (equal? (dev+ino element) check)))
+             #t
+             directories)))
 
 ;;;
 ;;; Source location.
diff --git a/tests/utils.scm b/tests/utils.scm
index 462e43e2b1..88a88eba1d 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello <at> foundationdevices.com>
 ;;; Copyright © 2024 Herman Rimm <herman <at> rimm.ee>
+;;; Copyright © 2025 Noé Lopez <noelopez <at> free.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -370,6 +371,18 @@ (define-public package-2\n  'package)\n"
          ;; way.
          "avr32" "avr32-unknown-none")))
 
+;; Try to not depend on the user’s filesystem.
+(test-equal "directory=?"
+  '(#t #t #t #t #t #f #f)
+  (list
+   (directory=? "/" "/")
+   (directory=? "/../" "//")
+   (directory=? "//../" "/")
+   (directory=? "/")
+   (directory=? "/" "/../" "//" "//..//../")
+   (directory=? "/proc/99999999" "/proc/99999999") ;nonexistent directories
+   (directory=? "/proc/99999999/../../" "/")))
+
 (test-end)
 
 (false-if-exception (delete-file temp-file))

base-commit: 73d74032d580212e7b59644d3324677926e4339b
-- 
2.48.1

Message #14 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 76143 <at> debbugs.gnu.org,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76143] [PATCH] gnu: Canonicalize paths before comparing.
Date: Sun, 16 Feb 2025 15:15:16 +0100

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> Noé Lopez <noe <at> noé.eu> skribis:
>
>> --- a/gnu/packages.scm
>> +++ b/gnu/packages.scm
>> @@ -167,11 +167,12 @@ (define %patch-path
>>    ;; Define it after '%package-module-path' so that '%load-path' contains user
>>    ;; directories, allowing patches in $GUIX_PACKAGE_PATH to be found.
>>    (make-parameter
>> -   (map (lambda (directory)
>> -          (if (string=? directory %distro-root-directory)
>> -              (string-append directory "/gnu/packages/patches")
>> -              directory))
>> -        %load-path)))
>> +   (let ((root (try-canonicalize-path %distro-root-directory)))
>> +     (map (lambda (directory)
>> +            (if (string=? (try-canonicalize-path directory) root)
>> +                (string-append directory "/gnu/packages/patches")
>> +                directory))
>> +          %load-path))))
>
> I’m not sure what the goal is but please keep in mind that
> ‘canonicalize-path’ is expensive in terms of system calls (especially if
> ‘%load-path’ is long, and we’d pay it for all program startup times),
> and that the comparison here remains brittle (checking the dev/ino
> fields of ‘stat’ would be more accurate).
>
> Thanks,
> Ludo’.

I was not aware that you could do that with stat, I’ve sent a v2 that
uses stat for comparison which works much better.

The problem I faced was that I needed to create environment variables by
hand so that I could load guile code (including guix’s) in a C program
with no external environment (the packagekit daemon).  It would fail
because of not finding patches since I had a trailing slash in my path.

I’ve resolved the issue by removing the slash, but I wanted to fix it
for everyone else trying to use Guix as a library.

Message #17 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: 76143 <at> debbugs.gnu.org
Cc: Noé Lopez <noelopez <at> free.fr>
Subject: [PATCH v3 0/1] gnu: Find patches directory through symlinks.
Date: Sun, 16 Feb 2025 15:43:59 +0100

From: Noé Lopez <noelopez <at> free.fr>

I sent the patch too early, sorry for the spam.

If you want to test this patch, you can try the following (changing the paths
to your checkout):

noe <at> lignux ~$ export GUILE_LOAD_PATH=/home/noe/src///guix-patch-review///
noe <at> lignux ~$ export GUILE_LOAD_COMPILED_PATH=/home/noe/src/guix-patch-review////
noe <at> lignux ~$ guile
[...]

Enter `,help' for help.
scheme@(guile-user)> ,use (gnu packages)
scheme@(guile-user)> (%patch-path)
$2 = ("/home/noe/src///guix-patch-review////gnu/packages/patches" "/gnu/store/ylwk2vn18dkzkj0nxq2h4vjzhz17bm7c-guile-3.0.9/share/guile/3.0" "/gnu/store/ylwk2vn18dkzkj0nxq2h4vjzhz17bm7c-guile-3.0.9/share/guile/site/3.0" "/gnu/store/ylwk2vn18dkzkj0nxq2h4vjzhz17bm7c-guile-3.0.9/share/guile/site" "/gnu/store/ylwk2vn18dkzkj0nxq2h4vjzhz17bm7c-guile-3.0.9/share/guile")

Check that /gnu/packages/patches was correctly added.

Noé Lopez (1):
  gnu: Find patches directory through symlinks.

 gnu/packages.scm |  2 +-
 guix/utils.scm   | 16 ++++++++++++++++
 tests/utils.scm  | 13 +++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)


base-commit: 73d74032d580212e7b59644d3324677926e4339b
-- 
2.48.1

Message #20 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: 76143 <at> debbugs.gnu.org
Cc: Noé Lopez <noe <at> xn--no-cja.eu>
Subject: [PATCH v3 1/1] gnu: Find patches directory through symlinks.
Date: Sun, 16 Feb 2025 15:44:00 +0100

From: Noé Lopez <noelopez <at> free.fr>

This fixes a bug where patches would not be found in %patch-path when the
Guile load path would contain a different path (via symlink or trailing slash)
to the %distro-root-directory than what was previously found.  We use stat to
make sure that two different paths to the same directory are still matched.

For example: if the Guile path was /guix/ and %distro-root-directory was
/guix, patches would not be found even though the two directories are the
same.

* gnu/packages.scm (%patch-path): Compare directories with directory=?.
* guix/utils.scm (directory=?): New procedure.
* tests/utils.scm: Add tests for directory=?.

Change-Id: I73f65b6c050cdeff85637e13ffd0319dcc1d4958
---
 gnu/packages.scm |  2 +-
 guix/utils.scm   | 16 ++++++++++++++++
 tests/utils.scm  | 13 +++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/gnu/packages.scm b/gnu/packages.scm
index bdd5d21940..5cad0d50ff 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -168,7 +168,7 @@ (define %patch-path
   ;; directories, allowing patches in $GUIX_PACKAGE_PATH to be found.
   (make-parameter
    (map (lambda (directory)
-          (if (string=? directory %distro-root-directory)
+          (if (directory=? directory %distro-root-directory)
               (string-append directory "/gnu/packages/patches")
               directory))
         %load-path)))
diff --git a/guix/utils.scm b/guix/utils.scm
index b6cf5aea4f..7979eba040 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -21,6 +21,7 @@
 ;;; Copyright © 2023 Zheng Junjie <873216071 <at> qq.com>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello <at> foundationdevices.com>
 ;;; Copyright © 2024 Herman Rimm <herman <at> rimm.ee>
+;;; Copyright © 2025 Noé Lopez <noelopez <at> free.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -41,6 +42,7 @@ (define-module (guix utils)
   #:use-module (guix config)
   #:autoload   (guix read-print) (object->string*)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-71)
@@ -162,6 +164,7 @@ (define-module (guix utils)
             compressed-output-port
             call-with-compressed-output-port
             canonical-newline-port
+            directory=?
 
             string-distance
             string-closest
@@ -1150,6 +1153,19 @@ (define (canonical-newline-port port)
                                  get-position
                                  set-position!
                                  close))
+
+(define* (directory=? directory #:rest directories)
+  (define (dev+ino directory)
+    (and-let* ((stats (stat directory #f))
+               (dev (stat:dev stats))
+               (ino (stat:ino stats)))
+      (cons dev ino)))
+  (define check (dev+ino directory))
+  (and check
+       (fold (lambda (element acc)
+               (and acc (equal? (dev+ino element) check)))
+             #t
+             directories)))
 
 ;;;
 ;;; Source location.
diff --git a/tests/utils.scm b/tests/utils.scm
index 462e43e2b1..88a88eba1d 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello <at> foundationdevices.com>
 ;;; Copyright © 2024 Herman Rimm <herman <at> rimm.ee>
+;;; Copyright © 2025 Noé Lopez <noelopez <at> free.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -370,6 +371,18 @@ (define-public package-2\n  'package)\n"
          ;; way.
          "avr32" "avr32-unknown-none")))
 
+;; Try to not depend on the user’s filesystem.
+(test-equal "directory=?"
+  '(#t #t #t #t #t #f #f)
+  (list
+   (directory=? "/" "/")
+   (directory=? "/../" "//")
+   (directory=? "//../" "/")
+   (directory=? "/")
+   (directory=? "/" "/../" "//" "//..//../")
+   (directory=? "/proc/99999999" "/proc/99999999") ;nonexistent directories
+   (directory=? "/proc/99999999/../../" "/")))
+
 (test-end)
 
 (false-if-exception (delete-file temp-file))
-- 
2.48.1

Message #23 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Noé Lopez <noe <at> xn--no-cja.eu>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 76143 <at> debbugs.gnu.org,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76143] [PATCH] gnu: Canonicalize paths before comparing.
Date: Fri, 21 Feb 2025 11:21:25 +0100

Hi,

Noé Lopez <noe <at> noé.eu> skribis:

> The problem I faced was that I needed to create environment variables by
> hand so that I could load guile code (including guix’s) in a C program
> with no external environment (the packagekit daemon).  It would fail
> because of not finding patches since I had a trailing slash in my path.

I don’t fully understand the situation.  My suggestion (but perhaps
you’re already doing that) would be for PackageKit to invoke ‘guix
repl’, not ‘guile’.  The raison d’être of ‘guix repl’ was precisely to
have the load path for Guix and all its dependencies properly set up.

Ludo’.

Message #26 received at 76143 <at> debbugs.gnu.org (full text, mbox):

From: Noé Lopez <noe <at> xn--no-cja.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 76143 <at> debbugs.gnu.org,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76143] [PATCH] gnu: Canonicalize paths before comparing.
Date: Fri, 21 Feb 2025 16:42:07 +0100

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> Noé Lopez <noe <at> noé.eu> skribis:
>
>> The problem I faced was that I needed to create environment variables by
>> hand so that I could load guile code (including guix’s) in a C program
>> with no external environment (the packagekit daemon).  It would fail
>> because of not finding patches since I had a trailing slash in my path.
>
> I don’t fully understand the situation.  My suggestion (but perhaps
> you’re already doing that) would be for PackageKit to invoke ‘guix
> repl’, not ‘guile’.  The raison d’être of ‘guix repl’ was precisely to
> have the load path for Guix and all its dependencies properly set up.
>

Let me try to explain the situation better:

The Guix channel has a special case for where it stores its patches, so
guix needs to detect whether a directory is the Guix source to append
"/gnu/packages/patches" to it:

gnu/packages.scm(166)
(define %patch-path
  (make-parameter
   (map (lambda (directory)
          (if (string=? directory %distro-root-directory)
              (string-append directory "/gnu/packages/patches")
              directory))
        %load-path)))

The comparison is done with string=?, which is a very weak way of
comparing directories, as you know.  But the directory list is supplied
by the environment, so its up to the user to set the path exactly equal
to %distro-root-directory.

Obviously, this can fail in many ways if the user is not using guix
through pre-inst-env or guix repl.  My patch fixes that by comparing
with stat, making sure all paths to the guix source work.

For PackageKit specifically, I am using guix through guile’s C api and
needed to set the environment by hand, I fixed it by removing the
trailing slash.

Is that clearer?

All the best,
Noé

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.