guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#72867] [PATCH v5] gexp: Make 'local-file' follow symlinks.


From: pelzflorian (Florian Pelz)
Subject: [bug#72867] [PATCH v5] gexp: Make 'local-file' follow symlinks.
Date: Sat, 07 Sep 2024 09:35:54 +0200
User-agent: Gnus/5.13 (Gnus v5.13)

Hello Nigko, Tobias, Ludo, Attila (putting them in Cc again).

Nigko Yerden <nigko.yerden@gmail.com> writes:
> Using of 'eval' in test from v4 is wrong. It does not play any role there.
> Most importantly it does not prevent spoiling of '%load-path' for the
> rest of 'tests/gexp.scm' module. Here is the better version of the test
> that uses 'dynamic-wind'.

The test is beautiful.  It makes clear why each
canonicalize-path is needed.  I had not understood the issue entirely
before I read it.

When we follow symlinks, both calling the real "../test-local-file.scm"
and the symlink to it behaves the same.

When not following symlinks, we get different results depending on what
we run.  I see no reason to ever want that except displaying info or
debugging.

And except that not following symlinks is faster (fewer stat syscalls).
But Ludovic wrapped absolute-dirname in a memoizing mlambda in commit
87b711d200ad13eaef284bdd1ab77f85618b0498, which reduces the difference.

Regarding the code, if we kept the old code when `follow-symlinks?' is
false (we should not, but if we did), it remains surprising that we do
follow some symlinks.

> +       (if follow-symlinks?
> +        (dirname (canonicalize-path file))
> +        ;; If there are relative names in %LOAD-PATH, FILE can be relative
> +        ;; and needs to be canonicalized.
> +        (if (string-prefix? "/" file)
> +               (dirname file)
> +               (canonicalize-path (dirname file))))))))

In the new use-modules part of the test, `file-name'
in `current-source-directory' is "./test-local-file.scm" code, which
means if we look at what happens if we not follow-symlinks, we took the
latter (canonicalize-path (dirname file)) path in
> (if (string-prefix? "/" file)
>        (dirname file)
>        (canonicalize-path (dirname file))))))))
which does not follow the symlink in the basename and fails.
But it would follow symlinks in the directory part.

Regards,
Florian





reply via email to

[Prev in Thread] Current Thread [Next in Thread]