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

Previous Next

Full log

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 宋文武 <iyzsong <at> envs.net>
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: Re: bug#75688: GUIX_ify harmful environment variables and replace
 wrapper scripts with search path value files
Date: Tue, 28 Jan 2025 22:15:07 +0900

Hello,

宋文武 <iyzsong <at> envs.net> writes:

> 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.

Seems upstream prefer we open an issue to discuss the use case/feature
request first.  I've done so for the proposed GDK_PIXBUF_MODULE_FILES
environment variable here [0].

[0]  https://gitlab.gnome.org/GNOME/gdk-pixbuf/-/issues/247

[...]

>>> ++    /* 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.

Thanks for the explanation.

>> [...]
>>
>> 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...

It's indeed a bit hacky, but still an improvement over wrapping
everything with leaky shell scripts in my opinion.

--
Thanks,
Maxim

Previous Next

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