GNU bug report logs - #75688
Replace wrapper scripts with search path value files in search-paths.d

Previous Next

Full log

View this message in rfc822 format

From: 宋文武 <iyzsong <at> envs.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 75688 <at> debbugs.gnu.org, 宋文武 <iyzsong <at> member.fsf.org>, Vivien Kraus <vivien <at> planete-kraus.eu>, Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Subject: [bug#75688] GUIX_ify harmful environment variables and replace wrapper scripts with search path value files
Date: Mon, 27 Jan 2025 13:17:14 +0800

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

>> +++ b/gnu/packages/patches/glib-guix-search-paths.patch
>> @@ -0,0 +1,158 @@
>
> I think it'd be nice to forward a subset of this patch that implements
> just the loading environment variable from a file, as that mechanism
> seems like it could be generally useful (and upstreaming it would lower
> the maintenance burden for us).

Okay, I could try.


>> ++  gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);

While, I just find that this "/proc/self/exe" alone only works for ELF
executables, for interpreted scripts, we also need patch interpreters to
set 2 environment variables, eg: GUIX_INTERPRETER_PATH and GUIX_MAIN_SCRIPT_PATH.

And use GUIX_MAIN_SCRIPT_PATH when "/proc/self/exe" match
GUIX_INTERPRETER_PATH...

I'll send updated patches later.


>> ++  gchar *out_path = NULL;
>> ++  gchar *search_paths_d = NULL;
>> ++
>> ++  /* We install executables under "bin" or "libexec", can also be a subdirectory of "libexec". */
>> ++  if (exe_path && (g_str_match_string("/bin/", exe_path, FALSE) ||
>> ++                   g_str_match_string("/libexec/", exe_path, FALSE))) {
>
> Perhaps these 'bin' and 'libexec' hard-coded names should come from the
> build system of gdk-pixbuf, in case a distro uses different names across
> its package collection (to make it more general).

Okay, I could do this when upstream.

>> ++    /* Find output directory, which is the parent directory of "bin" or "libexec". */
>> ++    out_path = g_path_get_dirname (exe_path);
>> ++    while (g_str_match_string("/bin/", out_path, FALSE) ||
>> ++           g_str_match_string("/libexec/", out_path, FALSE)) {
>> ++      gchar *dir_path = out_path;
>
> Is the intent above to *copy* out_path into dir_path?  Currently that's
> not done; we just point another pointer to it.
>
>> ++      out_path = g_path_get_dirname (dir_path);
>
> If g_path_get_dirname mutates dir_path, than dir_path should be a string
> copy.  Otherwise if it doesn't get mutated by the call, we should be
> able to use just:
>
> out_path = g_path_get_dirname (out_path);
>
>> ++      g_free (dir_path);

That 'dir_path' is only maded to be freed here, since
'g_path_get_dirname' allocate a new array instead of modify existed one,
so we need free the old out_path once we get a new one.


> [...]
>
> Apart from my above comments, this looks good to me.  I think I'd stress
> once more the value of upstreaming as much of this to ease maintenance
> in the future.
>
> Thanks for this novel idea/implementation!

Sure, with additional interpreter patches which I haven't sent, to be
honest I feel this more hacky than novel, hope upstream or other folks
can give better ideas...

Thank you!

Previous Next

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