diff mbox series

[bug#76376] guix: gexp: canonicalize file paths for import

Message ID 0dafbea136e328cd214e7e1fb05ab91ab04b17da.1739829485.git.ryan@arctype.co
State New
Headers
Series [bug#76376] guix: gexp: canonicalize file paths for import |

Commit Message

Ryan Sundberg Feb. 17, 2025, 9:58 p.m. UTC 
  When we intern a file from the store during `imported-modules`, if the
file is a symlink (e.g., from a Guix profile), a dangling symlink can be
created in the module-import builder.

Follow any symlinks before interning the files to the store, so that the
file itself is imported and not the dangling link.

See also: https://issues.guix.gnu.org/73275

* guix/gexp.scm (imported-files/derivation): canonicalize-path

Change-Id: Ic0af90cda7c5c5819526e455cf62300e18408dbd
---
 guix/gexp.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 91b18baa4274a025d28f06133682a9269217730d

Comments

Ryan Sundberg Feb. 17, 2025, 10:18 p.m. UTC | #1 
Hello team,

This is a patch for a "deep" bug in Guix gexp processing which evokes in 
circumstances when using g-expressions to build things that try to 
create module closures with code that is referenced in the current 
environment via symlink.

It can manifest in difficult to comprehend errors, such as "no code for 
module: (guix utils)` when guix/utils.scm is correctly defined in the 
load path of the program and exists (but it is a symlink, such as by 
using `guix shell` to load another guix environment, e.g. where the 
shell imports a different guix itself).

In my use case, I was using `guix` to build raw os disk images with my 
own set of customized packages and services when this bug blocked me at 
a dead stop.

The root cause of this after much complex debugging, tracing, and 
reading helped me to identify the bug report from Ludo at 
https://issues.guix.gnu.org/73275 and understand the dangling symlink 
issue. What happens here, and what this patch fixes, is that the 
`interned-file` procedure will not follow symlinks, and will intern a 
symlink if it is told to. In most scenarios this is harmless as the 
symlinks intersect to something (e.g. guix/utils.scm) which is already 
in the profile anyways, so the bug is dormant.
However, in other cases, it is possible to create a dangling symlink 
here when `imported-modules` references a file which is a symlink on the 
Guile %load-path, and `interned-file` in this line of gexp.scm can 
intern a dangling symlink.

This patch closes that possibility by canonicalizing the path of the 
interned file before loading it into the module closure path, so that 
`imported-modules` will never import a dangling symlink to a guile file 
used by a module-closure.

--Ryan
Ludovic Courtès Feb. 21, 2025, 10:56 a.m. UTC | #2 
Hi,

Ryan Sundberg <ryan@arctype.co> skribis:

> When we intern a file from the store during `imported-modules`, if the
> file is a symlink (e.g., from a Guix profile), a dangling symlink can be
> created in the module-import builder.
>
> Follow any symlinks before interning the files to the store, so that the
> file itself is imported and not the dangling link.
>
> See also: https://issues.guix.gnu.org/73275
>
> * guix/gexp.scm (imported-files/derivation): canonicalize-path
>
> Change-Id: Ic0af90cda7c5c5819526e455cf62300e18408dbd

[...]

>       ((final-path . (? string? file-name))
> -      (mlet %store-monad ((file (interned-file file-name
> +      (mlet %store-monad ((file (interned-file (canonicalize-path file-name)
>                                                 (basename final-path))))

Instead of calling ‘canonicalize-path’, which leads to many syscalls,
I’d suggest:

   (interned-file file-name (basename final-path)
                  #:recursive? #f)

I believe that would have the desired effect.

Could you also add a test that reproduces the problem?

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/guix/gexp.scm b/guix/gexp.scm
index e44aea6420..85351b0322 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1576,7 +1576,7 @@  (define* (imported-files/derivation files
   (define file-pair
     (match-lambda
      ((final-path . (? string? file-name))
-      (mlet %store-monad ((file (interned-file file-name
+      (mlet %store-monad ((file (interned-file (canonicalize-path file-name)
                                                (basename final-path))))
         (return (list final-path file))))
      ((final-path . file-like)