| Message ID | 20210515095227.3245343-1-slyfox@gentoo.org |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#48434] guile: allow pre-inst-env inject local paths | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
On Sat, 15 May 2021 10:52:27 +0100 Sergei Trofimovich <slyfox@gentoo.org> wrote: > I observed the problem when tried to run 'guix refresh' from local git checkout: > > $ strace -f ./pre-inst-env guix refresh -u re2c |& fgrep re2c.scm > ... > [pid 3014757] openat(AT_FDCWD, "/usr/share/guile/site/3.0/gnu/packages/re2c.scm.qilB0R", > O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EACCES (Permission denied) > > Attempt to /usr/share happens because local directory override is ignored: > > $ ./pre-inst-env guile -c '(display (search-path %load-path "gnu/packages/re2c.scm")) (newline) (display %load-path) (newline)' > /usr/share/guile/site/3.0/gnu/packages/re2c.scm > (/usr/share/guile/3.0 \ > /usr/share/guile/site/3.0 \ > /usr/share/guile/site \ > /usr/share/guile \ > /home/slyfox/dev/git/guix \ > /home/slyfox/dev/git/guix) > > It happens because ./guile ignores GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH > unconditionally. > > The change keeps GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH for ./pre-inst-env. > > * gnu/packages/aux-files/guile-launcher.c (main): don't ignore > GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH in GUIX_UNINSTALLED=1 mode. > > Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> > --- > gnu/packages/aux-files/guile-launcher.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/gnu/packages/aux-files/guile-launcher.c b/gnu/packages/aux-files/guile-launcher.c > index 47ba069de1..bed63353a9 100644 > --- a/gnu/packages/aux-files/guile-launcher.c > +++ b/gnu/packages/aux-files/guile-launcher.c > @@ -73,14 +73,19 @@ main (int argc, char **argv) > which is always preferable over the C locale. */ > setlocale (LC_ALL, "en_US.utf8"); > > - const char *str; > - str = getenv ("GUILE_LOAD_PATH"); > - load_path = str != NULL ? strdup (str) : NULL; > - str = getenv ("GUILE_LOAD_COMPILED_PATH"); > - load_compiled_path = str ? strdup (str) : NULL; > + /* Allow ./pre-inst-env to inject local paths. That way local sources > + are preferred for most operations. */ > + if (getenv ("GUIX_UNINSTALLED") == NULL) > + { > + const char *str; > + str = getenv ("GUILE_LOAD_PATH"); > + load_path = str != NULL ? strdup (str) : NULL; > + str = getenv ("GUILE_LOAD_COMPILED_PATH"); > + load_compiled_path = str ? strdup (str) : NULL; > > - unsetenv ("GUILE_LOAD_PATH"); > - unsetenv ("GUILE_LOAD_COMPILED_PATH"); > + unsetenv ("GUILE_LOAD_PATH"); > + unsetenv ("GUILE_LOAD_COMPILED_PATH"); > + } > > /* XXX: Do not let GMP allocate via libgc as this can lead to memory > corruption in GnuTLS/Nettle since Nettle also uses GMP: Stumbled on it again today when cloned fresh guix repo and forgot to apply this patch. Is it a reasonable approach? Or something else is at fault here? Thanks!
Sergei Trofimovich schreef op ma 16-08-2021 om 18:28 [+0100]: > On Sat, 15 May 2021 10:52:27 +0100 > Sergei Trofimovich <slyfox@gentoo.org> wrote: > > > I observed the problem when tried to run 'guix refresh' from local git checkout: > > > > $ strace -f ./pre-inst-env guix refresh -u re2c |& fgrep re2c.scm > > ... > > [pid 3014757] openat(AT_FDCWD, "/usr/share/guile/site/3.0/gnu/packages/re2c.scm.qilB0R", > > O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EACCES (Permission denied) > > > > Attempt to /usr/share happens because local directory override is ignored: > > > > $ ./pre-inst-env guile -c '(display (search-path %load-path "gnu/packages/re2c.scm")) (newline) (display %load-path) (newline)' > > /usr/share/guile/site/3.0/gnu/packages/re2c.scm > > (/usr/share/guile/3.0 \ > > /usr/share/guile/site/3.0 \ > > /usr/share/guile/site \ > > /usr/share/guile \ > > /home/slyfox/dev/git/guix \ > > /home/slyfox/dev/git/guix) ^ here's the checkout in the load path > > > > It happens because ./guile ignores GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH > > unconditionally. The local directory isn't ignored, it's at the end (the effect is about the same though). > > The change keeps GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH for ./pre-inst-env. > > > > * gnu/packages/aux-files/guile-launcher.c (main): don't ignore > > GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH in GUIX_UNINSTALLED=1 mode. Could you do something like #define GUIX_UNINSTALLED 1 #if GUIX_UNINSTALLED new behaviour #else OLD BEHAVIOUR #endif and change "Makefile.am" to compile two variants of "guile", one with "DGUIX_UNINSTALLED=1" which goes into libexec, and another with "DGUIX_UNINSTALLED=0" named "$CHECKOUT/guile" which isn't installed anywhere but will be added to PATH by "pre-inst-env", or something like that? I've a bit of an aversion towards using $..._UNINSTALLED environment variables, as it leads to difficult situations like ‘what should happen if I /gnu/store/.../bin/guix is run with GUIX_UNINSTALLED set to 1’: (a): Reset "GUILE_LOAD{,_COMPILED}_PATH" because we're running directly from the store (b): Don't reset "GUILE_LOAD_{,_COMPILED}_PATH" because GUIX_UNINSTALLED is set to 1. As a comparison, GUIX_UNINSTALLED, the preprocessor variable, is like a ‘lexical variable’, and GUIX_UNINSTALLED, the environment variable, is like a ‘parameter object’/‘dynamically bound variable’ (see, e.g., 6.11.12 Parameters in the Guile manual). I tend to prefer the former, except for dynamic things like LC_ALL, $[...]_PATH (when used by "guile", "gcc" -- applications typically shouldn't depend on $GUILE_LOAD_PATH, if they do, they need 'wrap-program' or the like’). Also, "guile-launcher.c" is used by 'quiet-quile" in (guix self). It should probably be verified that "guix pull" still works well. Greetings, Maxime.
diff --git a/gnu/packages/aux-files/guile-launcher.c b/gnu/packages/aux-files/guile-launcher.c index 47ba069de1..bed63353a9 100644 --- a/gnu/packages/aux-files/guile-launcher.c +++ b/gnu/packages/aux-files/guile-launcher.c @@ -73,14 +73,19 @@ main (int argc, char **argv) which is always preferable over the C locale. */ setlocale (LC_ALL, "en_US.utf8"); - const char *str; - str = getenv ("GUILE_LOAD_PATH"); - load_path = str != NULL ? strdup (str) : NULL; - str = getenv ("GUILE_LOAD_COMPILED_PATH"); - load_compiled_path = str ? strdup (str) : NULL; + /* Allow ./pre-inst-env to inject local paths. That way local sources + are preferred for most operations. */ + if (getenv ("GUIX_UNINSTALLED") == NULL) + { + const char *str; + str = getenv ("GUILE_LOAD_PATH"); + load_path = str != NULL ? strdup (str) : NULL; + str = getenv ("GUILE_LOAD_COMPILED_PATH"); + load_compiled_path = str ? strdup (str) : NULL; - unsetenv ("GUILE_LOAD_PATH"); - unsetenv ("GUILE_LOAD_COMPILED_PATH"); + unsetenv ("GUILE_LOAD_PATH"); + unsetenv ("GUILE_LOAD_COMPILED_PATH"); + } /* XXX: Do not let GMP allocate via libgc as this can lead to memory corruption in GnuTLS/Nettle since Nettle also uses GMP:
I observed the problem when tried to run 'guix refresh' from local git checkout: $ strace -f ./pre-inst-env guix refresh -u re2c |& fgrep re2c.scm ... [pid 3014757] openat(AT_FDCWD, "/usr/share/guile/site/3.0/gnu/packages/re2c.scm.qilB0R", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EACCES (Permission denied) Attempt to /usr/share happens because local directory override is ignored: $ ./pre-inst-env guile -c '(display (search-path %load-path "gnu/packages/re2c.scm")) (newline) (display %load-path) (newline)' /usr/share/guile/site/3.0/gnu/packages/re2c.scm (/usr/share/guile/3.0 \ /usr/share/guile/site/3.0 \ /usr/share/guile/site \ /usr/share/guile \ /home/slyfox/dev/git/guix \ /home/slyfox/dev/git/guix) It happens because ./guile ignores GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH unconditionally. The change keeps GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH for ./pre-inst-env. * gnu/packages/aux-files/guile-launcher.c (main): don't ignore GUILE_LOAD_PATH / GUILE_LOAD_COMPILED_PATH in GUIX_UNINSTALLED=1 mode. Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> --- gnu/packages/aux-files/guile-launcher.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)