Replace wrapper scripts with search path value files in search-paths.d

  • Open
  • quality assurance status badge
Details
3 participants
  • iyzsong
  • Liliana Marie Prikler
  • Maxim Cournoyer
Owner
unassigned
Submitted by
iyzsong
Severity
normal
I
I
iyzsong wrote on 20 Jan 13:04 +0100
[PATCH 0/2] Introduce GUIX_LIBRARY_PATH to replace harmful environment variables
(address . guix-patches@gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
cover.1737374057.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

Hello, we currently set many environment variables from search-paths, some can be harmful for
foreign system, which leading crash, eg: GDK_PIXBUF_MODULE_FILE, QT_PLUGIN_PATH, as reported in:


Instead of patch each software/library with its own environment variables (GUIX_GTK3_PATH, GUIX_QT_PLUGIN_PATH),
we can try patch all to calculate their from one GUIX_LIBRARY_PATH (default to ~/.guix-profile/lib), since the
problems mostly come from incompatible libraries ABI, so from "lib" construct its subdirectories should be enough.
eg:
GTK3_PATH use: lib/gtk-3.0
FCITX_ADDONS_DIRS use: lib/fcitx5

First patch add GUIX_LIBRARY_PATH to the default search paths, like PATH.
Second patch replace GDK_PIXBUF_MODULE_FILE with GUIX_LIBRARY_PATH.

If this is fine, I could work on replace QT_PLUGIN_PATH, etc. later.
What do you think? Thank you!


Sou Bunnbu (???) (2):
profiles: Add $GUIX_LIBRARY_PATH to default search paths.
gnu: gdk-pixbuf: Respect GUIX_LIBRARY_PATH.

gnu/local.mk | 1 +
gnu/packages/gtk.scm | 12 ++----
...gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch | 42 +++++++++++++++++++
guix/build/profiles.scm | 1 +
guix/profiles.scm | 8 ++--
guix/search-paths.scm | 10 +++++
6 files changed, 61 insertions(+), 13 deletions(-)
create mode 100644 gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch


base-commit: c985075db3e6682d8a5a231c01c770aa5a147f72
--
2.47.1
I
I
iyzsong wrote on 20 Jan 13:06 +0100
[PATCH 1/2] profiles: Add $GUIX_LIBRARY_PATH to default search paths.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
26c794d4f4f2d2d8a118e35b0509c190df21373d.1737374057.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

* guix/search-paths.scm ($GUIX_LIBRARY_PATH): New search path.
* guix/profiles.scm (manifest-search-paths): Add $GUIX_LIBRARY_PATH.
* guix/build/profiles.scm (manifest-sexp->inputs+search-paths): Add
$GUIX_LIBRARY_PATH.

Change-Id: I9ff090552bb40df7b42aaec71d587d3db07b20ed
---
guix/build/profiles.scm | 1 +
guix/profiles.scm | 8 ++++----
guix/search-paths.scm | 10 ++++++++++
3 files changed, 15 insertions(+), 4 deletions(-)

Toggle diff (62 lines)
diff --git a/guix/build/profiles.scm b/guix/build/profiles.scm
index b19d93f971..760b519bfa 100644
--- a/guix/build/profiles.scm
+++ b/guix/build/profiles.scm
@@ -180,6 +180,7 @@ (define (manifest-sexp->inputs+search-paths manifest)
(values (reverse inputs)
(delete-duplicates
(cons* $PATH
+ $GUIX_LIBRARY_PATH
$GUIX_EXTENSIONS_PATH
(map sexp->search-path-specification
(reverse search-paths)))))))))))
diff --git a/guix/profiles.scm b/guix/profiles.scm
index 87b9543ac0..f6ec51fe0b 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -742,11 +742,11 @@ (define (manifest-matching-entries manifest patterns)
(define (manifest-search-paths manifest)
"Return the list of search path specifications that apply to MANIFEST,
-including the search path specification for $PATH."
+including the search path specification for $PATH and $GUIX_LIBRARY_PATH."
(delete-duplicates
- (cons $PATH
- (append-map manifest-entry-search-paths
- (manifest-entries manifest)))))
+ (cons* $PATH $GUIX_LIBRARY_PATH
+ (append-map manifest-entry-search-paths
+ (manifest-entries manifest)))))
(define* (manifest->code manifest
#:key (entry-package-version (const "")))
diff --git a/guix/search-paths.scm b/guix/search-paths.scm
index 27fcb78054..359ddc1750 100644
--- a/guix/search-paths.scm
+++ b/guix/search-paths.scm
@@ -40,6 +40,7 @@ (define-module (guix search-paths)
$LIBRARY_PATH
$GUIX_EXTENSIONS_PATH
$PATH
+ $GUIX_LIBRARY_PATH
$PKG_CONFIG_PATH
$SSL_CERT_DIR
$SSL_CERT_FILE
@@ -128,6 +129,15 @@ (define $PATH
(variable "PATH")
(files '("bin" "sbin"))))
+(define $GUIX_LIBRARY_PATH
+ ;; Set some environment variables can make a foreign system crash, eg:
+ ;; GDK_PIXBUF_MODULE_FILE <https://issues.guix.gnu.org/75523>
+ ;; QT_PLUGIN_PATH <https://issues.guix.gnu.org/73897>
+ ;; For those cases, we could patch softwares to use this special variable.
+ (search-path-specification
+ (variable "GUIX_LIBRARY_PATH")
+ (files '("lib"))))
+
(define $GUIX_EXTENSIONS_PATH
;; 'GUIX_EXTENSIONS_PATH' is used by Guix to locate extension commands.
;; Unlike 'PATH', it is attached to a package, Guix; however, it is
--
2.47.1
I
I
iyzsong wrote on 20 Jan 13:06 +0100
[PATCH 2/2] gnu: gdk-pixbuf: Respect GUIX_LIBRARY_PATH.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
9596193713fcfc45725e0b1f0008bf0453d352f1.1737374057.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>


* gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/gtk.scm (gdk-pixbuf)[source]: Apply patch.
[native-search-paths]: Remove GDK_PIXBUF_MODULE_FILE.

Change-Id: I109565e5f506b9335856143f68abe164aff3cd26
---
gnu/local.mk | 1 +
gnu/packages/gtk.scm | 12 ++----
...gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch | 42 +++++++++++++++++++
3 files changed, 46 insertions(+), 9 deletions(-)
create mode 100644 gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch

Toggle diff (92 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index e06a605712..133512d613 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1400,6 +1400,7 @@ dist_patch_DATA = \
%D%/packages/patches/gd-fix-tests-on-i686.patch \
%D%/packages/patches/gd-brect-bounds.patch \
%D%/packages/patches/gdb-hurd64.patch \
+ %D%/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch \
%D%/packages/patches/gdm-default-session.patch \
%D%/packages/patches/gdm-elogind-support.patch \
%D%/packages/patches/gdm-remove-hardcoded-xwayland-path.patch \
diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 7ed7d7b7df..4a4069d495 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -726,7 +726,9 @@ (define-public gdk-pixbuf
name "-" version ".tar.xz"))
(sha256
(base32
- "0jz4kziz5lirnjjvbspbqzsigk8vnqknng1fga89d81vs5snr6zf"))))
+ "0jz4kziz5lirnjjvbspbqzsigk8vnqknng1fga89d81vs5snr6zf"))
+ (patches
+ (search-patches "gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch"))))
(build-system meson-build-system)
(outputs '("out" "debug"))
(arguments
@@ -773,14 +775,6 @@ (define-public gdk-pixbuf
;; For the documentation.
gi-docgen
python-docutils))
- (native-search-paths
- ;; This file is produced by the gdk-pixbuf-loaders-cache-file
- ;; profile hook.
- (list (search-path-specification
- (variable "GDK_PIXBUF_MODULE_FILE")
- (files (list %gdk-pixbuf-loaders-cache-file))
- (separator #f) ;single valued
- (file-type 'regular))))
(synopsis "Image loading library")
(description "GdkPixbuf is a library that loads image data in various
formats and stores it as linear buffers in memory. The buffers can then be
diff --git a/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch
new file mode 100644
index 0000000000..3ec7bc28ab
--- /dev/null
+++ b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch
@@ -0,0 +1,42 @@
+diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c
+index e1df590..e553eba 100644
+--- a/gdk-pixbuf/gdk-pixbuf-io.c
++++ b/gdk-pixbuf/gdk-pixbuf-io.c
+@@ -663,6 +663,19 @@ gdk_pixbuf_io_init_builtin (void)
+ #undef load_one_builtin_module
+ }
+
++
++static gchar **
++build_guix_library_path (void)
++{
++ gchar **dirs = NULL;
++ gchar *library_path = g_strdup (g_getenv ("GUIX_LIBRARY_PATH"));
++ if (!library_path || !library_path[0])
++ library_path = g_build_filename (g_get_home_dir (), ".guix-profile", "lib", NULL);
++ dirs = g_strsplit (library_path, G_SEARCHPATH_SEPARATOR_S, 0);
++ g_free (library_path);
++ return dirs;
++}
++
+ static gboolean
+ gdk_pixbuf_io_init (void)
+ {
+@@ -670,6 +683,17 @@ gdk_pixbuf_io_init (void)
+ gboolean ret;
+
+ gdk_pixbuf_io_init_builtin ();
++
++ /* Load loaders from GUIX_LIBRARY_PATH */
++ gchar **guix_libdirs = build_guix_library_path ();
++ for (gsize i = 0; guix_libdirs[i] != NULL; i++) {
++ module_file = g_build_filename (guix_libdirs[i], "gdk-pixbuf-2.0",
++ GDK_PIXBUF_BINARY_VERSION, "loaders.cache", NULL);
++ gdk_pixbuf_io_init_modules (module_file, NULL);
++ g_free (module_file);
++ }
++ g_strfreev (guix_libdirs);
++
+ #ifdef USE_GMODULE
+ module_file = gdk_pixbuf_get_module_file ();
+ #endif
--
2.47.1
M
M
Maxim Cournoyer wrote on 20 Jan 15:00 +0100
Re: [bug#75688] [PATCH 0/2] Introduce GUIX_LIBRARY_PATH to replace harmful environment variables
(address . iyzsong@envs.net)
87msfld1pi.fsf@gmail.com
Hello,

iyzsong@envs.net writes:

Toggle quote (16 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> Hello, we currently set many environment variables from search-paths, some can be harmful for
> foreign system, which leading crash, eg: GDK_PIXBUF_MODULE_FILE, QT_PLUGIN_PATH, as reported in:
>
> <https://issues.guix.gnu.org/53514>
> <https://issues.guix.gnu.org/75523>
> <https://issues.guix.gnu.org/73897>
>
> Instead of patch each software/library with its own environment variables (GUIX_GTK3_PATH, GUIX_QT_PLUGIN_PATH),
> we can try patch all to calculate their from one GUIX_LIBRARY_PATH (default to ~/.guix-profile/lib), since the
> problems mostly come from incompatible libraries ABI, so from "lib" construct its subdirectories should be enough.
> eg:
> GTK3_PATH use: lib/gtk-3.0
> FCITX_ADDONS_DIRS use: lib/fcitx5

Instead of reconstructing the paths from something like the suggested
new GUIX_LIBRARY_PATH, I think it'd be nice if additionally to the stock
environment variables supported by each software, we introduced GUIX_
prefixed variants such as GUIX_GTK3_PATH and GUIX_QT_PLUGIN_PATH which
would be used by the Guix search paths specifications defined on these
packages.

Toggle quote (3 lines)
> First patch add GUIX_LIBRARY_PATH to the default search paths, like PATH.
> Second patch replace GDK_PIXBUF_MODULE_FILE with GUIX_LIBRARY_PATH.

For the GDK_PIXBUF_MODULE_FILE special case, the problem is foremost
that it's a single entry value; as mentioned previously it'd be nice if
we could contribute a true multi-items variant named
GDK_PIXBUF_MODULE_FILES that could point to more than one file.
Alternatively we could use GUIX_GDK_PIXBUF_MODULE_FILE. But I don't see
why we should use an intermetate GUIX_LIBRARY_PATH to compute all the
other correct paths; this should be left to the search paths, in my
opinion (as it's simpler, cleaner).

Thanks for the initiative! Let me know if I misunderstood something.

--
Maxim
M
M
Maxim Cournoyer wrote on 20 Jan 15:03 +0100
Re: [bug#75688] [PATCH 2/2] gnu: gdk-pixbuf: Respect GUIX_LIBRARY_PATH.
(address . iyzsong@envs.net)
87ikq9d1kk.fsf@gmail.com
Hi,

iyzsong@envs.net writes:

Toggle quote (9 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> This fixes <https://issues.guix.gnu.org/75523>.
>
> * gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/gtk.scm (gdk-pixbuf)[source]: Apply patch.
> [native-search-paths]: Remove GDK_PIXBUF_MODULE_FILE.

[...]

Toggle quote (49 lines)
> diff --git a/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch
> new file mode 100644
> index 0000000000..3ec7bc28ab
> --- /dev/null
> +++ b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_LIBRARY_PATH.patch
> @@ -0,0 +1,42 @@
> +diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c
> +index e1df590..e553eba 100644
> +--- a/gdk-pixbuf/gdk-pixbuf-io.c
> ++++ b/gdk-pixbuf/gdk-pixbuf-io.c
> +@@ -663,6 +663,19 @@ gdk_pixbuf_io_init_builtin (void)
> + #undef load_one_builtin_module
> + }
> +
> ++
> ++static gchar **
> ++build_guix_library_path (void)
> ++{
> ++ gchar **dirs = NULL;
> ++ gchar *library_path = g_strdup (g_getenv ("GUIX_LIBRARY_PATH"));
> ++ if (!library_path || !library_path[0])
> ++ library_path = g_build_filename (g_get_home_dir (), ".guix-profile", "lib", NULL);
> ++ dirs = g_strsplit (library_path, G_SEARCHPATH_SEPARATOR_S, 0);
> ++ g_free (library_path);
> ++ return dirs;
> ++}
> ++
> + static gboolean
> + gdk_pixbuf_io_init (void)
> + {
> +@@ -670,6 +683,17 @@ gdk_pixbuf_io_init (void)
> + gboolean ret;
> +
> + gdk_pixbuf_io_init_builtin ();
> ++
> ++ /* Load loaders from GUIX_LIBRARY_PATH */
> ++ gchar **guix_libdirs = build_guix_library_path ();
> ++ for (gsize i = 0; guix_libdirs[i] != NULL; i++) {
> ++ module_file = g_build_filename (guix_libdirs[i], "gdk-pixbuf-2.0",
> ++ GDK_PIXBUF_BINARY_VERSION, "loaders.cache", NULL);
> ++ gdk_pixbuf_io_init_modules (module_file, NULL);
> ++ g_free (module_file);
> ++ }
> ++ g_strfreev (guix_libdirs);
> ++
> + #ifdef USE_GMODULE
> + module_file = gdk_pixbuf_get_module_file ();
> + #endif

As I mentioned in my previous reply, I think having a
GDK_PIXBUF_MODULE_FILES environment variable using similar code as above
would be a nicer solution, since it could probably be upstreamed and
thus no custom patches to maintain on our side indefinitely (and useful
functionality available to others -- a win/win situation)

Do you agree that'd be nicer?

--
Thanks,
Maxim
?
Re: [bug#75688] [PATCH 0/2] Introduce GUIX_LIBRARY_PATH to replace harmful environment variables
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
877c6ord5p.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (27 lines)
> Hello,
>
> iyzsong@envs.net writes:
>
>> From: ??? <iyzsong@member.fsf.org>
>>
>> Hello, we currently set many environment variables from search-paths, some can be harmful for
>> foreign system, which leading crash, eg: GDK_PIXBUF_MODULE_FILE, QT_PLUGIN_PATH, as reported in:
>>
>> <https://issues.guix.gnu.org/53514>
>> <https://issues.guix.gnu.org/75523>
>> <https://issues.guix.gnu.org/73897>
>>
>> Instead of patch each software/library with its own environment variables (GUIX_GTK3_PATH, GUIX_QT_PLUGIN_PATH),
>> we can try patch all to calculate their from one GUIX_LIBRARY_PATH (default to ~/.guix-profile/lib), since the
>> problems mostly come from incompatible libraries ABI, so from "lib" construct its subdirectories should be enough.
>> eg:
>> GTK3_PATH use: lib/gtk-3.0
>> FCITX_ADDONS_DIRS use: lib/fcitx5
>
> Instead of reconstructing the paths from something like the suggested
> new GUIX_LIBRARY_PATH, I think it'd be nice if additionally to the stock
> environment variables supported by each software, we introduced GUIX_
> prefixed variants such as GUIX_GTK3_PATH and GUIX_QT_PLUGIN_PATH which
> would be used by the Guix search paths specifications defined on these
> packages.

Yes, that will also works.

Toggle quote (9 lines)
>
>> First patch add GUIX_LIBRARY_PATH to the default search paths, like PATH.
>> Second patch replace GDK_PIXBUF_MODULE_FILE with GUIX_LIBRARY_PATH.
>
> For the GDK_PIXBUF_MODULE_FILE special case, the problem is foremost
> that it's a single entry value; as mentioned previously it'd be nice if
> we could contribute a true multi-items variant named
> GDK_PIXBUF_MODULE_FILES that could point to more than one file.

Sure, but using GDK_PIXBUF_MODULE_FILES after upstreamed would also
cause problems for foreign system, since host programs will try to load
plugins from guix packages, which may or may not work, even crash.


Toggle quote (2 lines)
> Alternatively we could use GUIX_GDK_PIXBUF_MODULE_FILE.

Another alternative is to make softwares "relocatable" to profile
directory, make them discovery plugins relative to their executables,
gdk-pixbuf does have a "relocatable" build option, but it use
"/proc/self/exe" which resolved to the final store path instead of
profile level directory. Improve that likely could go upstreams, eg: by
first add 'g_get_executable_path' for glib:

It also open the chance to get rid of wrapper scripts (eg: by using a
virtualenv'd python as interpreter instead of wrap executables with
GUIX_PYTHONPATH), which I plan to play with later. But this
"relocatable" way doesn't allow mix paths from differenet directories
(system and user profiles), it's good as it reduce the risk of
incompability, but not convenient.

So using specified GUIX_* variables are a must to avoid influence host
programs and use plugins from multiple profiles.


Toggle quote (4 lines)
> But I don't see why we should use an intermetate GUIX_LIBRARY_PATH to
> compute all the other correct paths; this should be left to the search
> paths, in my opinion (as it's simpler, cleaner).

Well, lesser variables give a "clean" feeling, it's mostly aesthetic,
especially in GNOME and KDE, mixed with wrappers, there are a lot
variables "noise" in the environment.

Also think a litte more, as we need patching software anyway, I hope to
modify the logic `guix_build_library_path` to also include paths from
what wrappers currently provides, to drop wrappers. That logic should
be simpler to implement in one place than for every specified variable.

Note that even with GUIX_ specified variables, they're still problems
since we have different profiles and problems built with different
versions of libraries. Search "undefined symbol: __libc_pthread_init"
in the list could find some reports.

I think the final goal is:

- profile only provides PATH, GUIX_LIBRARY_PATH (replace most if not all
other "libs/.*" paths), XDG_DATA_DIRS, INFOPATH, and other shareable
(usually for data files, under the "share" directory) environment
variables.

- no wrapper scripts for hardcoded plugins, so GUIX_LIBRARY_PATH only
contains user, home, system profiles.

- if incompability problems occurs, we can just unset one
GUIX_LIBRARY_PATH to launch the influenced program without
incompatible plugins from profiles. ofc this is temporary, you should
not mix incompatible libraries (by update all packages in profile(s)
at once), but handy for hurry adventurers.

If this looks reasonable, then I'd work on patch gtk or qt to get rid of
wrappers to justify this, how's this sound?


Toggle quote (2 lines)
> Thanks for the initiative! Let me know if I misunderstood something.

There is no misunderstood, thank you to let me think and clarify more.
?
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87wmeophsk.fsf@envs.net
??? <iyzsong@envs.net> writes:

Toggle quote (14 lines)
>> But I don't see why we should use an intermetate GUIX_LIBRARY_PATH to
>> compute all the other correct paths; this should be left to the search
>> paths, in my opinion (as it's simpler, cleaner).
>
> Well, lesser variables give a "clean" feeling, it's mostly aesthetic,
> especially in GNOME and KDE, mixed with wrappers, there are a lot
> variables "noise" in the environment.
>
> Also think a litte more, as we need patching software anyway, I hope to
> modify the logic `guix_build_library_path` to also include paths from
> what wrappers currently provides, to drop wrappers. That logic should
> be simpler to implement in one place than for every specified
> variable.

Okay, I changed the plan, since use only one env like GUIX_LIBRARY_PATH
would lead to some form of 'stats storm', and the logic is not harder
for patching different variables. If we get rid of wrapper scripts, the
"noise" will only contains profile paths, so are acceptable..

Now I get this patch for glib:

1. add a `gchar **g_build_guix_search_path_dirs (const gchar *variable)`
function to GLIB, which handle .VARIABLE file in addition to the env
value. Those .VARIABLE files sit at the same directory of
executable, would be used to replace wrapper script. As said early,
wrapper scripts leaks environment variables could cause incompatable
problems.

2. Use `g_build_guix_search_path_dirs` for GUIX_GIO_EXTRA_MODULES,
GUIX_GSETTINGS_SCHEMA_DIR, and etc. (later).
3. (later) Modify wrap-program usages to use .VARIABLE files to get rid
of environment variables leaks.
Toggle diff (101 lines)
diff --git a/gio/giomodule.c b/gio/giomodule.c
index 76c2028..49b02bb 100644
--- a/gio/giomodule.c
+++ b/gio/giomodule.c
@@ -1330,6 +1330,13 @@ _g_io_modules_ensure_loaded (void)
g_io_modules_scan_all_in_directory_with_scope (module_dir, scope);
g_free (module_dir);
+ /* GUIX: Load gio modules from GUIX_GIO_EXTRA_MODULES */
+ gchar **guix_giomodule_dirs = g_build_guix_search_path_dirs ("GUIX_GIO_EXTRA_MODULES");
+ for (int i = 0; guix_giomodule_dirs[i] != NULL; i++) {
+ g_io_modules_scan_all_in_directory_with_scope (guix_giomodule_dirs[i], scope);
+ }
+ g_strfreev (guix_giomodule_dirs);
+
g_io_module_scope_free (scope);
/* Initialize types from built-in "modules" */
diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c
index e8ccc8c..d7ff8f4 100644
--- a/gio/gsettingsschema.c
+++ b/gio/gsettingsschema.c
@@ -369,6 +369,13 @@ initialise_schema_sources (void)
g_strfreev (extra_schema_dirs);
}
+ /* GUIX: Load schemas from GUIX_GSETTINGS_SCHEMA_DIR. */
+ char **guix_schema_dirs = g_build_guix_search_path_dirs ("GUIX_GSETTINGS_SCHEMA_DIR");
+ i = g_strv_length(guix_schema_dirs);
+ while (i--)
+ try_prepend_dir (guix_schema_dirs[i]);
+ g_strfreev (guix_schema_dirs);
+
g_once_init_leave (&initialised, TRUE);
}
}
diff --git a/glib/gutils.c b/glib/gutils.c
index 8628a56..0139d42 100644
--- a/glib/gutils.c
+++ b/glib/gutils.c
@@ -2849,6 +2849,46 @@ g_get_system_config_dirs (void)
return system_config_dirs;
}
+gchar **
+g_build_guix_search_path_dirs (const gchar *variable)
+{
+ gchar **dirs = NULL;
+ char *value = NULL;
+ GStrvBuilder *builder = g_strv_builder_new ();
+
+#if defined(__linux__) || defined(__gnu_hurd__)
+ /* First add paths from the .VARIABLE file, which can be used to replace wrapper script. */
+ gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
+ gchar *var_path = g_strjoin(NULL, exe_path, ".", variable, NULL);
+ if (g_file_get_contents (var_path, &value, NULL, NULL)) {
+ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
+ g_strv_builder_addv (builder, (const gchar **) dirs);
+ g_strfreev (dirs);
+ g_free (value);
+ }
+ g_free (exe_path);
+ g_free (var_path);
+#endif
+
+ /* Then add paths from the environment variable. */
+ gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
+ if (is_setuid) /* we don't want to access arbitrary files when running as setuid. */
+ value = NULL;
+ else
+ value = g_strdup (g_getenv (variable));
+
+ if (value && value[0]) {
+ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
+ g_strv_builder_addv (builder, (const gchar **) dirs);
+ g_strfreev (dirs);
+ }
+ g_free (value);
+
+ dirs = g_strv_builder_end (builder);
+ g_strv_builder_unref (builder);
+ return dirs;
+}
+
/**
* g_nullify_pointer:
* @nullify_location: (not nullable): the memory address of the pointer.
diff --git a/glib/gutils.h b/glib/gutils.h
index efc6914..710cf27 100644
--- a/glib/gutils.h
+++ b/glib/gutils.h
@@ -36,6 +36,9 @@
G_BEGIN_DECLS
+GLIB_AVAILABLE_IN_ALL
+gchar **g_build_guix_search_path_dirs (const gchar *variable);
+
GLIB_AVAILABLE_IN_ALL
const gchar * g_get_user_name (void);
GLIB_AVAILABLE_IN_ALL
A similiar patch would needed for qtbase, or maybe I can use glib API
directly from any Qt application? (haven't tried) Does this level of
patching seems okay?

Thanks!
M
M
Maxim Cournoyer wrote on 21 Jan 14:15 +0100
(name . ???)(address . iyzsong@envs.net)
87sepcs3z9.fsf@gmail.com
Hi,

??? <iyzsong@envs.net> writes:

[...]

Toggle quote (4 lines)
> Sure, but using GDK_PIXBUF_MODULE_FILES after upstreamed would also
> cause problems for foreign system, since host programs will try to load
> plugins from guix packages, which may or may not work, even crash.

Ah, I wasn't thinking about these being plugins depending on a specific
ABI, likely incompatible. I see.

Toggle quote (17 lines)
>> Alternatively we could use GUIX_GDK_PIXBUF_MODULE_FILE.
>
> Another alternative is to make softwares "relocatable" to profile
> directory, make them discovery plugins relative to their executables,
> gdk-pixbuf does have a "relocatable" build option, but it use
> "/proc/self/exe" which resolved to the final store path instead of
> profile level directory. Improve that likely could go upstreams, eg: by
> first add 'g_get_executable_path' for glib:
> https://gitlab.gnome.org/GNOME/glib/-/issues/31
>
> It also open the chance to get rid of wrapper scripts (eg: by using a
> virtualenv'd python as interpreter instead of wrap executables with
> GUIX_PYTHONPATH), which I plan to play with later. But this
> "relocatable" way doesn't allow mix paths from differenet directories
> (system and user profiles), it's good as it reduce the risk of
> incompability, but not convenient.

It sounds interesting, worth exploring. But being unable to compose
multiple profile together easily defeats its purpose a bit (though the
same would be true with single valued variable such as
GDK_PIXBUF_MODULE_FILE: you can't honor the two profiles here, one has
to win).

Toggle quote (12 lines)
> So using specified GUIX_* variables are a must to avoid influence host
> programs and use plugins from multiple profiles.
>
>
>> But I don't see why we should use an intermetate GUIX_LIBRARY_PATH to
>> compute all the other correct paths; this should be left to the search
>> paths, in my opinion (as it's simpler, cleaner).
>
> Well, lesser variables give a "clean" feeling, it's mostly aesthetic,
> especially in GNOME and KDE, mixed with wrappers, there are a lot
> variables "noise" in the environment.

While aesthetic is nice, I'd rather to keep the design stupid simple and
rely on the search path mechanism; we can refine later. Having 10 or 20
environment variables in my profile doesn't appear to be that big of a
deal to me.

Toggle quote (5 lines)
> Also think a litte more, as we need patching software anyway, I hope to
> modify the logic `guix_build_library_path` to also include paths from
> what wrappers currently provides, to drop wrappers. That logic should
> be simpler to implement in one place than for every specified variable.

Sorry, I've lost you; what is this `guix_build_library_path` variable?
I'm not sure we'd be able to get rid of wrappers fully, as not
everything would be relative to lib. I guess that's in the space to be
explored :-).

Toggle quote (5 lines)
> Note that even with GUIX_ specified variables, they're still problems
> since we have different profiles and problems built with different
> versions of libraries. Search "undefined symbol: __libc_pthread_init"
> in the list could find some reports.

Right. It seems the responsibility of the user to avoid combining
incompatible software from different profiles.

Toggle quote (19 lines)
> I think the final goal is:
>
> - profile only provides PATH, GUIX_LIBRARY_PATH (replace most if not all
> other "libs/.*" paths), XDG_DATA_DIRS, INFOPATH, and other shareable
> (usually for data files, under the "share" directory) environment
> variables.
>
> - no wrapper scripts for hardcoded plugins, so GUIX_LIBRARY_PATH only
> contains user, home, system profiles.
>
> - if incompability problems occurs, we can just unset one
> GUIX_LIBRARY_PATH to launch the influenced program without
> incompatible plugins from profiles. ofc this is temporary, you should
> not mix incompatible libraries (by update all packages in profile(s)
> at once), but handy for hurry adventurers.
>
> If this looks reasonable, then I'd work on patch gtk or qt to get rid of
> wrappers to justify this, how's this sound?

It's too early for me to say if it could really work, but I'll repeat
that a priori I'm not too fond of GUIX_LIBRARY_PATH (just the name would
clash with a GCC env var too) and custom logic that'd need to be
maintained in the software (we'd need to patch the software to have
GUIX_* variables anyway but the resulting patch would be trivial); I
think I'd rather have multiple, explicit environment variables computed
by search paths.

Just to make sure, GUIX_* variables would be honored *on top* of their
non GUIX_ prefixed (stock) variants, right? E.g. we wouldn't want GCC
to stop honoring LIBRARY_PATH even if we add GUIX_LIBRARY_PATH, as that
would confuse users.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 21 Jan 14:19 +0100
(name . ???)(address . iyzsong@envs.net)
87o700s3re.fsf@gmail.com
Hello,

??? <iyzsong@envs.net> writes:

[...]

Toggle quote (20 lines)
> Okay, I changed the plan, since use only one env like GUIX_LIBRARY_PATH
> would lead to some form of 'stats storm', and the logic is not harder
> for patching different variables. If we get rid of wrapper scripts, the
> "noise" will only contains profile paths, so are acceptable..
>
> Now I get this patch for glib:
>
> 1. add a `gchar **g_build_guix_search_path_dirs (const gchar *variable)`
> function to GLIB, which handle .VARIABLE file in addition to the env
> value. Those .VARIABLE files sit at the same directory of
> executable, would be used to replace wrapper script. As said early,
> wrapper scripts leaks environment variables could cause incompatable
> problems.
>
> 2. Use `g_build_guix_search_path_dirs` for GUIX_GIO_EXTRA_MODULES,
> GUIX_GSETTINGS_SCHEMA_DIR, and etc. (later).
>
> 3. (later) Modify wrap-program usages to use .VARIABLE files to get rid
> of environment variables leaks.

This reminds me of how Python virtualenv works, IIRC. What do you mean
by "wrapper scripts leak environment variables and could cause
incompatibility problems" ? Sure, if your term binary is wrapped, your
shell will have these environment variables as its a child process
inheriting its parent environment, but otherwise I'm not sure I
understand the issue, especially how processing a .VARIABLE file instead
would handle this better? Could yo give an example?

--
Thanks,
Maxim
?
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87o700p6xl.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (28 lines)
>> Okay, I changed the plan, since use only one env like GUIX_LIBRARY_PATH
>> would lead to some form of 'stats storm', and the logic is not harder
>> for patching different variables. If we get rid of wrapper scripts, the
>> "noise" will only contains profile paths, so are acceptable..
>>
>> Now I get this patch for glib:
>>
>> 1. add a `gchar **g_build_guix_search_path_dirs (const gchar *variable)`
>> function to GLIB, which handle .VARIABLE file in addition to the env
>> value. Those .VARIABLE files sit at the same directory of
>> executable, would be used to replace wrapper script. As said early,
>> wrapper scripts leaks environment variables could cause incompatable
>> problems.
>>
>> 2. Use `g_build_guix_search_path_dirs` for GUIX_GIO_EXTRA_MODULES,
>> GUIX_GSETTINGS_SCHEMA_DIR, and etc. (later).
>>
>> 3. (later) Modify wrap-program usages to use .VARIABLE files to get rid
>> of environment variables leaks.
>
> This reminds me of how Python virtualenv works, IIRC. What do you mean
> by "wrapper scripts leak environment variables and could cause
> incompatibility problems" ? Sure, if your term binary is wrapped, your
> shell will have these environment variables as its a child process
> inheriting its parent environment, but otherwise I'm not sure I
> understand the issue, especially how processing a .VARIABLE file instead
> would handle this better? Could yo give an example?

A program try to load different versions of plugins via
GIO_EXTRA_MODULES, QT_PLUGIN_PATHS could crash due to ABI incompatible, see:

mix GIO modules with different glibc: https://yhetil.org/guix/87r0rvasph.fsf@jpoiret.xyz/

In a desktop environment, with wrapper scripts you would get
GIO_EXTRA_MODULES, GTK_PATH contains store paths:


GIO_EXTRA_MODULES=/gnu/store/8k9s3z2315p494fj937jyvc9v7gpbjr8-dconf-0.40.0/lib/gio/modules:/gnu/store/knm6b1dxg2j3vji4wrgngv99pvb6f5ff-glib-networking-2.70.0/lib/gio/modules::/run/current-system/profile/lib/gio/modules
GTK_PATH=/gnu/store/kq72g9hjl1sj4c1qhw98m8rdw2ymmk7m-gtk+-3.24.30/lib/gtk-3.0:/gnu/store/fkl4fg06f538ryhiw4bs2iwwfs56g2k3-libcanberra-0.30/lib/gtk-3.0

Then when you try to run a incompatible program (eg: with a newer glibc
or gtk), those variables leaks from wrapper scripts will make the
program crash.

By replace those variables via .VARIABLE file, those starts with
'/gnu/store/' won't get exported to the shell and desktop sessions,
since they're not in the environ, but read from file (any executable
will read its unique set of .VARIABLE files) and used by executables
directly. So it should reduce the chance of incompatible, also if we
wrap programs need dconf/glib-networking with .VARIABLE file, there will
be no need to install dconf/glib-networking in the system profile,
reduce the risk more.

'/run/current-system/' still could cause problems though, which user can
update all packages sync or choose to not mix profiles.
M
M
Maxim Cournoyer wrote on 22 Jan 02:03 +0100
(name . ???)(address . iyzsong@envs.net)
87tt9rr776.fsf@gmail.com
Hello,

??? <iyzsong@envs.net> writes:

[...]

Toggle quote (18 lines)
>> inheriting its parent environment, but otherwise I'm not sure I
>> understand the issue, especially how processing a .VARIABLE file instead
>> would handle this better? Could yo give an example?
>
> A program try to load different versions of plugins via
> GIO_EXTRA_MODULES, QT_PLUGIN_PATHS could crash due to ABI incompatible, see:
>
> mix Qt 5.9.3 and 5.9.4: https://yhetil.org/guix/874jzl2n16.fsf@gmail.com/
> mix GIO modules with different glibc: https://yhetil.org/guix/87r0rvasph.fsf@jpoiret.xyz/
>
> In a desktop environment, with wrapper scripts you would get
> GIO_EXTRA_MODULES, GTK_PATH contains store paths:
>
> https://yhetil.org/guix/2c93c29e-032b-2b5e-6139-b28de456b47b@telenet.be/
>
> GIO_EXTRA_MODULES=/gnu/store/8k9s3z2315p494fj937jyvc9v7gpbjr8-dconf-0.40.0/lib/gio/modules:/gnu/store/knm6b1dxg2j3vji4wrgngv99pvb6f5ff-glib-networking-2.70.0/lib/gio/modules::/run/current-system/profile/lib/gio/modules
> GTK_PATH=/gnu/store/kq72g9hjl1sj4c1qhw98m8rdw2ymmk7m-gtk+-3.24.30/lib/gtk-3.0:/gnu/store/fkl4fg06f538ryhiw4bs2iwwfs56g2k3-libcanberra-0.30/lib/gtk-3.0

I see; I agree that if the environment variables or not set by a search
path and instead picked up by the binary from some file, then it solves
this class of problem, but that seems equivalent to wrapping the
binaries, no? You'd still have some glue code reading .VARIABLE and
then doing a bunch of setenv before launching the actual program, no?
That glue code is currently the shell or Guile script used to wrap the
binaries.

Unless we can fully eliminate search paths and propagation, we'll still
have the issue of having environment variables being set, even with the
use of .VARIABLE, as far s I understand.

Toggle quote (6 lines)
> Then when you try to run a incompatible program (eg: with a newer glibc
> or gtk), those variables leaks from wrapper scripts will make the
> program crash.
>
> By replace those variables via .VARIABLE file, those starts with

nitpick: Perhaps it could be called .environment or
.environment-variables, similarly named as in our failed Guix builds.

[...]

As you can see, perhaps I'm still missing pieces of your big picture. a
RFC/GCD documenting the idea, or a demo implementation if that's
easier/faster would help (code is often more succinct than text).

--
Thanks,
Maxim
?
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87r04vyyjv.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (4 lines)
> I see; I agree that if the environment variables or not set by a search
> path and instead picked up by the binary from some file, then it solves
> this class of problem, but that seems equivalent to wrapping the
> binaries, no?
Well, wrapping solve it in one way, since we use 'prefix' in wrapper,
eg: konsole has:

export QT_PLUGIN_PATH="/gnu/store/.../lib/qt6/plugins${QT_PLUGIN_PATH:+:}$QT_PLUGIN_PATH"

which let konsole work in a environment which has an incompatible
QT_PLUGIN_PATH, since the incompatible one load last and applications
only load a named plugin once.

It dosen't work other way, that in konsole when you run an another
unwrapped application, can be influenced by QT_PLUGIN_PATH from
konsole's wrapper script.

Toggle quote (5 lines)
> You'd still have some glue code reading .VARIABLE and
> then doing a bunch of setenv before launching the actual program, no?
> That glue code is currently the shell or Guile script used to wrap the
> binaries.

The point for those .VARIABLE files is without wrapper scripts there're
no setenv call anymore, ensure that getenv only get profiles paths, in
another words: reading from file and apply the values replaced setenv
then getenv and apply the values.

Toggle quote (5 lines)
>
> Unless we can fully eliminate search paths and propagation, we'll still
> have the issue of having environment variables being set, even with the
> use of .VARIABLE, as far s I understand.

Well, we must have environment variables due to we want to mix profiles.
Search paths can be applied without environment variables, by using
.VARIABLE files for hardcoded dependencies; and by finding current
profile path from executable (lookup argv[0] in PATH), from current
profile path we can construct and apply search paths values.

Use GUIX_* would solve problems for foreign programs, then reduce
wrapper scripts would solve some problems for guix programs.

Disallow load plugins from different profiles will solve more remaining
problems, but this can be considered later if there are actually reports.


Toggle quote (9 lines)
>> Then when you try to run a incompatible program (eg: with a newer glibc
>> or gtk), those variables leaks from wrapper scripts will make the
>> program crash.
>>
>> By replace those variables via .VARIABLE file, those starts with
>
> nitpick: Perhaps it could be called .environment or
> .environment-variables, similarly named as in our failed Guix builds.

Those are not variables in the "environment", but search-path values
hardcoded for the specified binary. Maybe I should use one set of files
for all binaries in one output like 'etc/ld.so.cache' does tough.
Compare with use one ".environment-variables" file, use different files
for each variable value simplify the logic and be more efficiency.

How about /etc/search-paths.d/VAR1, /etc/search-paths.d/VAR2?
Need more logic to get "etc", and add it to %harmless-collisions for
union-build though.

Toggle quote (4 lines)
> As you can see, perhaps I'm still missing pieces of your big picture. a
> RFC/GCD documenting the idea, or a demo implementation if that's
> easier/faster would help (code is often more succinct than text).

Sure, I'll work on implementation this. Thank you!
?
control message for bug #75688
(address . control@debbugs.gnu.org)
6345ed72449589e2@localhost
retitle 75688 GUIX_ify harmful environment variables and replace wrapper scripts with search path value files
quit
I
I
iyzsong wrote on 23 Jan 07:28 +0100
[PATCH v2 1/4] gnu: glib: Support load search paths from etc/search-paths.d files.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
916f2d3087c47d6212682656f47a2899ba795df1.1737613671.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

Add a new function "g_build_guix_search_path_dirs" to GLIB, which in addition
to environment variable, read search path values from etc/search-paths.d
directory of the current executable. This can be used to replace wrapper
scripts.

Use it for GUIX_GSETTINGS_SCHEMA_DIR, GUIX_GIO_EXTRA_MODULES,
GUIX_XDG_DATA_DIRS and GUIX_XDG_CONFIG_DIRS.

* gnu/packages/patches/glib-guix-search-paths.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/glib.scm (glib)[source]: Add patch.
[native-search-paths]: Add GUIX_GSETTINGS_SCHEMA_DIR, Replace
GIO_EXTRA_MODULES with GUIX_GIO_EXTRA_MODULES.

Change-Id: I1d6d113fc38b20ebd4dce195f6d9c58ce85967e4
---
gnu/local.mk | 1 +
gnu/packages/glib.scm | 9 +-
.../patches/glib-guix-search-paths.patch | 158 ++++++++++++++++++
3 files changed, 166 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/glib-guix-search-paths.patch

Toggle diff (209 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 5091f93eb8..6bb3a35770 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1436,6 +1436,7 @@ dist_patch_DATA = \
%D%/packages/patches/git-filter-repo-generate-doc.patch \
%D%/packages/patches/gklib-suitesparse.patch \
%D%/packages/patches/glib-appinfo-watch.patch \
+ %D%/packages/patches/glib-guix-search-paths.patch \
%D%/packages/patches/glib-skip-failing-test.patch \
%D%/packages/patches/glibc-2.33-riscv64-miscompilation.patch \
%D%/packages/patches/glibc-2.39-git-updates.patch \
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index e04eedb7ba..0704ba2c53 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -258,7 +258,8 @@ (define glib
(base32 "0c3vagxl77wma85qinbj974jvw96n5bvch2m7hqcwxq8fa5spsj4"))
(patches
(search-patches "glib-appinfo-watch.patch"
- "glib-skip-failing-test.patch"))
+ "glib-skip-failing-test.patch"
+ "glib-guix-search-paths.patch"))
(modules '((guix build utils)))
(snippet
'(begin
@@ -516,9 +517,13 @@ (define glib
(search-path-specification
(variable "XDG_DATA_DIRS")
(files '("share")))
+ ;; To load gsettings schemas from GTK, etc.
+ (search-path-specification
+ (variable "GUIX_GSETTINGS_SCHEMA_DIR")
+ (files '("share/glib-2.0/schemas")))
;; To load extra gio modules from glib-networking, etc.
(search-path-specification
- (variable "GIO_EXTRA_MODULES")
+ (variable "GUIX_GIO_EXTRA_MODULES")
(files '("lib/gio/modules")))))
(search-paths native-search-paths)
(synopsis "Low-level core library for GNOME projects")
diff --git a/gnu/packages/patches/glib-guix-search-paths.patch b/gnu/packages/patches/glib-guix-search-paths.patch
new file mode 100644
index 0000000000..ebd57024dd
--- /dev/null
+++ b/gnu/packages/patches/glib-guix-search-paths.patch
@@ -0,0 +1,158 @@
+diff --git a/gio/giomodule.c b/gio/giomodule.c
+index 76c2028..49b02bb 100644
+--- a/gio/giomodule.c
++++ b/gio/giomodule.c
+@@ -1330,6 +1330,13 @@ _g_io_modules_ensure_loaded (void)
+ g_io_modules_scan_all_in_directory_with_scope (module_dir, scope);
+ g_free (module_dir);
+
++ /* GUIX: Load gio modules from GUIX_GIO_EXTRA_MODULES */
++ gchar **guix_giomodule_dirs = g_build_guix_search_path_dirs ("GUIX_GIO_EXTRA_MODULES");
++ for (int i = 0; guix_giomodule_dirs[i] != NULL; i++) {
++ g_io_modules_scan_all_in_directory_with_scope (guix_giomodule_dirs[i], scope);
++ }
++ g_strfreev (guix_giomodule_dirs);
++
+ g_io_module_scope_free (scope);
+
+ /* Initialize types from built-in "modules" */
+diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c
+index e8ccc8c..b8bae14 100644
+--- a/gio/gsettingsschema.c
++++ b/gio/gsettingsschema.c
+@@ -354,6 +354,13 @@ initialise_schema_sources (void)
+ while (i--)
+ try_prepend_data_dir (dirs[i]);
+
++ /* GUIX: Load schemas from GUIX_GSETTINGS_SCHEMA_DIR. */
++ char **guix_schema_dirs = g_build_guix_search_path_dirs ("GUIX_GSETTINGS_SCHEMA_DIR");
++ i = g_strv_length(guix_schema_dirs);
++ while (i--)
++ try_prepend_dir (guix_schema_dirs[i]);
++ g_strfreev (guix_schema_dirs);
++
+ try_prepend_data_dir (g_get_user_data_dir ());
+
+ /* Disallow loading extra schemas if running as setuid, as that could
+diff --git a/glib/gutils.c b/glib/gutils.c
+index 8628a56..bc21efc 100644
+--- a/glib/gutils.c
++++ b/glib/gutils.c
+@@ -2708,6 +2708,16 @@ g_build_system_data_dirs (void)
+ data_dir_vector = g_strsplit (data_dirs, G_SEARCHPATH_SEPARATOR_S, 0);
+ #endif
+
++ /* GUIX: Use data files from GUIX_XDG_DATA_DIRS. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) data_dir_vector);
++ g_strfreev (data_dir_vector);
++ data_dir_vector = g_build_guix_search_path_dirs ("GUIX_XDG_DATA_DIRS");
++ g_strv_builder_addv (builder, (const gchar **) data_dir_vector);
++ g_strfreev (data_dir_vector);
++ data_dir_vector = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return g_steal_pointer (&data_dir_vector);
+ }
+
+@@ -2800,6 +2810,16 @@ g_build_system_config_dirs (void)
+ conf_dir_vector = g_strsplit (conf_dirs, G_SEARCHPATH_SEPARATOR_S, 0);
+ #endif
+
++ /* GUIX: Use config files from GUIX_XDG_CONFIG_DIRS. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) conf_dir_vector);
++ g_strfreev (conf_dir_vector);
++ conf_dir_vector = g_build_guix_search_path_dirs ("GUIX_XDG_CONFIG_DIRS");
++ g_strv_builder_addv (builder, (const gchar **) conf_dir_vector);
++ g_strfreev (conf_dir_vector);
++ conf_dir_vector = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return g_steal_pointer (&conf_dir_vector);
+ }
+
+@@ -2849,6 +2869,69 @@ g_get_system_config_dirs (void)
+ return system_config_dirs;
+ }
+
++gchar **
++g_build_guix_search_path_dirs (const gchar *variable)
++{
++ gchar **dirs = NULL;
++ char *value = NULL;
++ GStrvBuilder *builder = g_strv_builder_new ();
++
++#if defined(__linux__) || defined(__gnu_hurd__)
++ /* First add paths from the etc/search-paths.d, which can be used to replace wrapper script. */
++ gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
++ 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))) {
++ /* 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;
++ out_path = g_path_get_dirname (dir_path);
++ g_free (dir_path);
++ }
++
++ /* Now add paths from etc/search-paths.d/VARIABLE file. */
++ search_paths_d = g_build_filename (out_path, "etc", "search-paths.d", NULL);
++ if (g_file_test (search_paths_d, G_FILE_TEST_EXISTS)) {
++ gchar *var_path = g_build_filename (search_paths_d, variable, NULL);
++ if (g_file_get_contents (var_path, &value, NULL, NULL)) {
++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
++ g_strv_builder_addv (builder, (const gchar **) dirs);
++ g_strfreev (dirs);
++ g_free (value);
++ }
++ g_free (var_path);
++ }
++ }
++
++ g_free (exe_path);
++ g_free (out_path);
++ g_free (search_paths_d);
++#endif
++
++ /* Then add paths from the environment variable. */
++ gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
++ if (is_setuid) /* we don't want to access arbitrary files when running as setuid. */
++ value = NULL;
++ else
++ value = g_strdup (g_getenv (variable));
++
++ if (value && value[0]) {
++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
++ g_strv_builder_addv (builder, (const gchar **) dirs);
++ g_strfreev (dirs);
++ }
++ g_free (value);
++
++ dirs = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++ return dirs;
++}
++
+ /**
+ * g_nullify_pointer:
+ * @nullify_location: (not nullable): the memory address of the pointer.
+diff --git a/glib/gutils.h b/glib/gutils.h
+index efc6914..710cf27 100644
+--- a/glib/gutils.h
++++ b/glib/gutils.h
+@@ -36,6 +36,9 @@
+
+ G_BEGIN_DECLS
+
++GLIB_AVAILABLE_IN_ALL
++gchar **g_build_guix_search_path_dirs (const gchar *variable);
++
+ GLIB_AVAILABLE_IN_ALL
+ const gchar * g_get_user_name (void);
+ GLIB_AVAILABLE_IN_ALL

base-commit: 7080aaf08102ec4c9c976582d6adfa0c14e6c640
--
2.47.1
I
I
iyzsong wrote on 23 Jan 07:28 +0100
[PATCH v2 2/4] gnu: gdk-pixbuf: Respect GUIX_GDK_PIXBUF_MODULE_FILES search path.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
96be080084ef425fb5d5a8ab920d30667dc68c99.1737613671.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>


* gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch:
New file.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/gtk.scm (gdk-pixbuf)[source]: Add patch.
[native-search-paths]: Replace GDK_PIXBUF_MODULE_FILE with GUIX_GDK_PIXBUF_MODULE_FILES.

Change-Id: I63ce8fa14799e04551522e6d27e89bf47b08043e
---
gnu/local.mk | 1 +
gnu/packages/gtk.scm | 8 +++++---
...-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch | 18 ++++++++++++++++++
3 files changed, 24 insertions(+), 3 deletions(-)
create mode 100644 gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch

Toggle diff (65 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 6bb3a35770..4249476dfa 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1400,6 +1400,7 @@ dist_patch_DATA = \
%D%/packages/patches/gd-fix-tests-on-i686.patch \
%D%/packages/patches/gd-brect-bounds.patch \
%D%/packages/patches/gdb-hurd64.patch \
+ %D%/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch \
%D%/packages/patches/gdm-default-session.patch \
%D%/packages/patches/gdm-elogind-support.patch \
%D%/packages/patches/gdm-remove-hardcoded-xwayland-path.patch \
diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 7ed7d7b7df..392734a33c 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -726,7 +726,10 @@ (define-public gdk-pixbuf
name "-" version ".tar.xz"))
(sha256
(base32
- "0jz4kziz5lirnjjvbspbqzsigk8vnqknng1fga89d81vs5snr6zf"))))
+ "0jz4kziz5lirnjjvbspbqzsigk8vnqknng1fga89d81vs5snr6zf"))
+ (patches
+ (search-patches
+ "gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch"))))
(build-system meson-build-system)
(outputs '("out" "debug"))
(arguments
@@ -777,9 +780,8 @@ (define-public gdk-pixbuf
;; This file is produced by the gdk-pixbuf-loaders-cache-file
;; profile hook.
(list (search-path-specification
- (variable "GDK_PIXBUF_MODULE_FILE")
+ (variable "GUIX_GDK_PIXBUF_MODULE_FILES")
(files (list %gdk-pixbuf-loaders-cache-file))
- (separator #f) ;single valued
(file-type 'regular))))
(synopsis "Image loading library")
(description "GdkPixbuf is a library that loads image data in various
diff --git a/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch
new file mode 100644
index 0000000000..eb22761403
--- /dev/null
+++ b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch
@@ -0,0 +1,18 @@
+diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c
+index e1df590..ba24cf6 100644
+--- a/gdk-pixbuf/gdk-pixbuf-io.c
++++ b/gdk-pixbuf/gdk-pixbuf-io.c
+@@ -670,6 +670,13 @@ gdk_pixbuf_io_init (void)
+ gboolean ret;
+
+ gdk_pixbuf_io_init_builtin ();
++
++ /* Load loaders from GUIX_GDK_PIXBUF_MODULE_FILES. */
++ gchar **guix_module_files = g_build_guix_search_path_dirs ("GUIX_GDK_PIXBUF_MODULE_FILES");
++ for (int i = 0; guix_module_files[i] != NULL; i++)
++ gdk_pixbuf_io_init_modules (guix_module_files[i], NULL);
++ g_strfreev (guix_module_files);
++
+ #ifdef USE_GMODULE
+ module_file = gdk_pixbuf_get_module_file ();
+ #endif
--
2.47.1
I
I
iyzsong wrote on 23 Jan 07:28 +0100
[PATCH v2 3/4] gnu: gtk: Add search-paths.d support for GUIX_GTK{2, 3, 4}_PATH.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
a22dda0eb30660d90f25a3ec0862d9332b9ab542.1737613671.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

* gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch,
gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch,
gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch: Rewrite in terms
of 'g_build_guix_search_path_dirs'.

Change-Id: Ib0748c39e56fd598f30f40b9ac3bb0f012f14c31
---
.../patches/gtk2-respect-GUIX_GTK2_PATH.patch | 64 ++++++-------------
.../patches/gtk3-respect-GUIX_GTK3_PATH.patch | 55 ++++++----------
.../patches/gtk4-respect-GUIX_GTK4_PATH.patch | 62 +++++-------------
3 files changed, 54 insertions(+), 127 deletions(-)

Toggle diff (206 lines)
diff --git a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
index 93a8ddc242..fb6c7809f9 100644
--- a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
+++ b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
@@ -1,46 +1,20 @@
-This patch makes GTK+ look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK2_PATH". This can be used
-instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
-other major versions of GTK+.
+diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
+index 50729b61a5..2d4c2c2a85 100644
+--- a/gtk/gtkmodules.c
++++ b/gtk/gtkmodules.c
+@@ -96,5 +96,15 @@ get_module_path (void)
+ result = pango_split_file_list (module_path);
+ g_free (module_path);
---- a/gtk/gtkmodules.c 2014-09-29 22:02:17.000000000 +0200
-+++ b/gtk/gtkmodules.c 2015-12-02 18:41:53.306396938 +0100
-@@ -55,6 +55,7 @@
- get_module_path (void)
- {
- const gchar *module_path_env;
-+ const gchar *module_guix_gtk2_path_env;
- const gchar *exe_prefix;
- const gchar *home_dir;
- gchar *home_gtk_dir = NULL;
-@@ -70,6 +71,7 @@
- home_gtk_dir = g_build_filename (home_dir, ".gtk-2.0", NULL);
-
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk2_path_env = g_getenv ("GUIX_GTK2_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
-
- if (exe_prefix)
-@@ -77,9 +79,21 @@
- else
- default_dir = g_build_filename (GTK_LIBDIR, "gtk-2.0", NULL);
-
-- if (module_path_env && home_gtk_dir)
-+ if (module_guix_gtk2_path_env && module_path_env && home_gtk_dir)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, module_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env && home_gtk_dir)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, module_path_env, default_dir, NULL);
-+ else if (module_path_env && home_gtk_dir)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, default_dir, NULL);
- else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
++ /* GUIX: Load additional modules from GUIX_GTK2_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK2_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
diff --git a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
index 66fd2fd1c4..28e232a812 100644
--- a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
+++ b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
@@ -1,38 +1,21 @@
-This patch makes GTK+ look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK3_PATH". This can be used
-instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
-other major versions of GTK+.
-
---- a/gtk/gtkmodules.c 2015-09-20 20:09:05.060590217 +0200
-+++ b/gtk/gtkmodules.c 2015-09-20 20:10:33.423124833 +0200
-@@ -52,6 +52,7 @@
- get_module_path (void)
- {
- const gchar *module_path_env;
-+ const gchar *module_guix_gtk3_path_env;
- const gchar *exe_prefix;
- gchar *module_path;
- gchar *default_dir;
-@@ -61,6 +62,7 @@
- return result;
+diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
+index f93101c272..b57e1da802 100644
+--- a/gtk/gtkmodules.c
++++ b/gtk/gtkmodules.c
+@@ -81,6 +81,16 @@ get_module_path (void)
+ result = gtk_split_file_list (module_path);
+ g_free (module_path);
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk3_path_env = g_getenv ("GUIX_GTK3_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
++ /* GUIX: Load additional modules from GUIX_GTK3_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK3_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
- if (exe_prefix)
-@@ -68,7 +70,13 @@
- else
- default_dir = g_build_filename (_gtk_get_libdir (), "gtk-3.0", NULL);
-
-- if (module_path_env)
-+ if (module_guix_gtk3_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk3_path_env, module_path_env, default_dir, NULL);
-+ else if (module_guix_gtk3_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk3_path_env, default_dir, NULL);
-+ else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
- else
diff --git a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
index 4a60023bf7..56c202ecf4 100644
--- a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
+++ b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
@@ -1,51 +1,21 @@
-From 889294a93fc6464c2c2919bc47f6fd85ec823363 Mon Sep 17 00:00:00 2001
-From: Raghav Gururajan <rg@raghavgururajan.name>
-Date: Tue, 18 May 2021 19:57:00 -0400
-Subject: [PATCH] [PATCH]: Honor GUIX_GTK4_PATH.
-
-This patch makes GTK look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK4_PATH". This can be used
-instead of "GTK_PATH" to make GTK find modules that are incompatible with
-other major versions of GTK.
----
- gtk/gtkmodules.c | 10 +++++++++-
- 1 file changed, 9 insertions(+), 1 deletion(-)
-
diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
-index aace5dcbc9..193b6a02e9 100644
+index 51b0916624..0cd6ee7e30 100644
--- a/gtk/gtkmodules.c
+++ b/gtk/gtkmodules.c
-@@ -105,6 +105,7 @@ static char **
- get_module_path (void)
- {
- const char *module_path_env;
-+ const gchar *module_guix_gtk4_path_env;
- const char *exe_prefix;
- char *module_path;
- char *default_dir;
-@@ -114,6 +115,7 @@ get_module_path (void)
- return result;
+@@ -132,6 +132,16 @@ get_module_path (void)
+ result = split_file_list (module_path);
+ g_free (module_path);
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk4_path_env = g_getenv ("GUIX_GTK4_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
++ /* GUIX: Load additional modules from GUIX_GTK4_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK4_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
- if (exe_prefix)
-@@ -121,7 +123,13 @@ get_module_path (void)
- else
- default_dir = g_build_filename (_gtk_get_libdir (), "gtk-4.0", NULL);
-
-- if (module_path_env)
-+ if (module_guix_gtk4_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk4_path_env, module_path_env, default_dir, NULL);
-+ else if (module_guix_gtk4_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk4_path_env, default_dir, NULL);
-+ else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
- else
---
-2.31.1
-
--
2.47.1
I
I
iyzsong wrote on 23 Jan 07:28 +0100
[PATCH v2 4/4] build: glib-or-gtk-build-system: Replace wrapper scripts with 'search-paths.d'.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
96b1c701576df682137650386544821196e2152d.1737613671.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

* guix/build/glib-or-gtk-build-system.scm (write-search-path-file): New procedure.
(gtk-module-directories): Add version to arguments.
(gsettings-schema-directories): New procedure.
(data-directories): Don't check for "/glib-2.0/schemas".
(conf-directories): New procedure.
(wrap-all-programs): Rewrite in terms of 'write-search-path-file'.

Change-Id: I1c9e8d491b96e298d1568a5e29b04c762c26e4d1
---
guix/build/glib-or-gtk-build-system.scm | 165 ++++++++++++++----------
1 file changed, 94 insertions(+), 71 deletions(-)

Toggle diff (242 lines)
diff --git a/guix/build/glib-or-gtk-build-system.scm b/guix/build/glib-or-gtk-build-system.scm
index 67a52ddad3..a04c1b0616 100644
--- a/guix/build/glib-or-gtk-build-system.scm
+++ b/guix/build/glib-or-gtk-build-system.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2025 ??? <iyzsong@envs.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -39,6 +40,13 @@ (define-module (guix build glib-or-gtk-build-system)
;;
;; Code:
+(define* (write-search-path-file output variable value)
+ "Write VALUE to @file{etc/search-paths.d/VARIABLE} under OUTPUT."
+ (let ((search-paths.d (string-append output "/etc/search-paths.d")))
+ (mkdir-p search-paths.d)
+ (with-output-to-file (string-append search-paths.d "/" variable)
+ (lambda () (display variable)))))
+
(define (subdirectory-exists? parent sub-directory)
(directory-exists? (string-append parent sub-directory)))
@@ -47,32 +55,12 @@ (define (directory-included? directory directories-list)
(fold (lambda (s p) (or (string-ci=? s directory) p))
#f directories-list))
-;; We do not include $HOME/.guix-profile/gtk-v.0 (v=2 or 3) because we do not
-;; want to mix gtk+-2 and gtk+-3 modules. See
-;; https://developer.gnome.org/gtk3/stable/gtk-running.html
-(define (gtk-module-directories inputs)
- "Check for the existence of \"libdir/gtk-v.0\" in INPUTS. Return a list
+;; We load GTK modules via the GUIX_GTK2_PATH, GUIX_GTK3_PATH and GUIX_GTK4_PATH
+;; search paths.
+(define (gtk-module-directories inputs version)
+ "Check for the existence of \"libdir/gtk-VERSION\" in INPUTS. Return a list
with all found directories."
- (let* ((version
- (cond
- ((string-match "gtk-4"
- (or (assoc-ref inputs "gtk")
- (assoc-ref inputs "source")
- ""))
- "4.0")
- ((string-match "gtk\\+-3"
- (or (assoc-ref inputs "gtk+")
- (assoc-ref inputs "source")
- ""))
- "3.0")
- ((string-match "gtk\\+-2"
- (or (assoc-ref inputs "gtk+")
- (assoc-ref inputs "source")
- ""))
- "2.0")
- (else
- "4.0"))) ; We default to version 4.0.
- (gtk-module
+ (let ((gtk-module
(lambda (input prev)
(let* ((in (match input
((_ . dir) dir)
@@ -85,27 +73,22 @@ (define (gtk-module-directories inputs)
prev)))))
(fold gtk-module '() inputs)))
-;; See
+;; XDG data files include themes, sounds, icons, etc. See:
;; http://www.freedesktop.org/wiki/DesktopThemeSpec
;; http://freedesktop.org/wiki/Specifications/sound-theme-spec
;; http://freedesktop.org/wiki/Specifications/icon-theme-spec
;;
-;; Currently desktop themes are not well supported and do not honor
-;; XDG_DATA_DIRS. One example is evince which only looks for desktop themes
-;; in $HOME/.themes (for backward compatibility) and in XDG_DATA_HOME (which
-;; defaults to $HOME/.local/share). One way to handle these applications
-;; appears to be by making $HOME/.themes a symlink to
-;; $HOME/.guix-profile/share/themes.
+;; We load them via XDG_DATA_DIRS (from profile, has higher priority) and
+;; GUIX_XDG_DATA_DIRS (application specified) search paths.
(define (data-directories inputs)
- "Check for the existence of \"$datadir/glib-2.0/schemas\" or XDG themes data
-in INPUTS. Return a list with all found directories."
+ "Check for the existence of XDG data files in INPUTS. Return a list with all found
+directories."
(define (data-directory input previous)
(let* ((in (match input
((_ . dir) dir)
(_ "")))
(datadir (string-append in "/share")))
- (if (and (or (subdirectory-exists? datadir "/glib-2.0/schemas")
- (subdirectory-exists? datadir "/sounds")
+ (if (and (or (subdirectory-exists? datadir "/sounds")
(subdirectory-exists? datadir "/themes")
(subdirectory-exists? datadir "/cursors")
(subdirectory-exists? datadir "/wallpapers")
@@ -117,15 +100,45 @@ (define (data-directories inputs)
(fold data-directory '() inputs))
+;;; XDG configuration files are expected to be installed in etc/xdg directory.
+;;; We load them via XDG_CONFIG_DIRS (from profile, has higher priority) and
+;;; GUIX_XDG_CONFIG_DIRS (application specified) search paths.
+(define (conf-directories inputs)
+ "Check for the existence of XDG configuration files in INPUTS. Return a list with
+all found directories."
+ (define (conf-directory input previous)
+ (let* ((in (match input
+ ((_ . dir) dir)
+ (_ "")))
+ (conf-dir (string-append in "etc/xdg")))
+ (if (and (directory-exists? conf-dir)
+ (not (directory-included? conf-dir previous)))
+ (cons conf-dir previous)
+ previous)))
+
+ (fold conf-directory '() inputs))
+
+;;; GIO GSettings schemas are expected to be installed in $datadir/glib-2.0/schemas
+;;; directory. We load them via the GUIX_GSETTINGS_SCHEMA_DIR search path.
+(define (gsettings-schema-directories inputs)
+ "Check for the existence of \"$datadir/glib-2.0/schemas\" in INPUTS.
+Return a list with all found directories."
+ (define (gsettings-schema-directory input previous)
+ (let* ((in (match input
+ ((_ . dir) dir)
+ (_ "")))
+ (schema-dir (string-append in "/share/glib-2.0/schemas")))
+ (if (and (directory-exists? schema-dir)
+ (not (directory-included? schema-dir previous)))
+ (cons schema-dir previous)
+ previous)))
+
+ (fold gsettings-schema-directory '() inputs))
+
;; All GIO modules are expected to be installed in GLib's $libdir/gio/modules
;; directory. That directory has to include a file called giomodule.cache
-;; listing all available modules. GIO can be made aware of modules in other
-;; directories with the help of the environment variable GIO_EXTRA_MODULES.
-;; The official GIO documentation states that this environment variable should
-;; only be used for testing and not in a production environment. However, it
-;; appears that there is no other way of specifying multiple modules
-;; directories (NIXOS also does use this variable). See
-;; https://developer.gnome.org/gio/stable/running-gio-apps.html
+;; listing all available modules. We load them via the GUIX_GIO_EXTRA_MODULES
+;; search path.
(define (gio-module-directories inputs)
"Check for the existence of \"$libdir/gio/modules\" in the INPUTS and
returns a list with all found directories."
@@ -141,50 +154,60 @@ (define (gio-module-directories inputs)
(fold gio-module-directory '() inputs))
+
(define* (wrap-all-programs #:key inputs outputs
(glib-or-gtk-wrap-excluded-outputs '())
#:allow-other-keys)
"Implement phase \"glib-or-gtk-wrap\": look for GSettings schemas and
-gtk+-v.0 libraries and create wrappers with suitably set environment variables
+GTK libraries and create etc/search-paths.d with suitably set of files
if found.
Wrapping is not applied to outputs whose name is listed in
GLIB-OR-GTK-WRAP-EXCLUDED-OUTPUTS. This is useful when an output is known not
to contain any GLib or GTK+ binaries, and where wrapping would gratuitously
-add a dependency of that output on GLib and GTK+."
- ;; Do not require bash to be present in the package inputs
- ;; even when there is nothing to wrap.
- ;; Also, calculate (sh) only once to prevent some I/O.
- (define %sh (delay (search-input-file inputs "bin/bash")))
- (define (sh) (force %sh))
+add a dependency of that output on GLib and GTK."
(define handle-output
(match-lambda
((output . directory)
(unless (member output glib-or-gtk-wrap-excluded-outputs)
- (let* ((bindir (string-append directory "/bin"))
- (libexecdir (string-append directory "/libexec"))
- (bin-list (filter (negate wrapped-program?)
- (append (find-files bindir ".*")
- (find-files libexecdir ".*"))))
- (datadirs (data-directories
+ (let* ((datadirs (data-directories
(alist-cons output directory inputs)))
- (gtk-mod-dirs (gtk-module-directories
+ (confdirs (conf-directories
(alist-cons output directory inputs)))
- (gio-mod-dirs (gio-module-directories
+ (schemadirs (gsettings-schema-directories
(alist-cons output directory inputs)))
- (env-vars `(,@(if (not (null? datadirs))
- (list `("XDG_DATA_DIRS" ":" prefix ,datadirs))
- '())
- ,@(if (not (null? gtk-mod-dirs))
- (list `("GTK_PATH" ":" prefix ,gtk-mod-dirs))
- '())
- ,@(if (not (null? gio-mod-dirs))
- (list `("GIO_EXTRA_MODULES" ":"
- prefix ,gio-mod-dirs))
- '()))))
- (for-each (lambda (program)
- (apply wrap-program program #:sh (sh) env-vars))
- bin-list))))))
+ (gtk2-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "2.0"))
+ (gtk3-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "3.0"))
+ (gtk4-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "4.0"))
+ (gio-mod-dirs (gio-module-directories
+ (alist-cons output directory inputs))))
+ (when (not (null? datadirs))
+ (write-search-path-file output "GUIX_XDG_DATA_DIRS"
+ (string-join datadirs ":")))
+ (when (not (null? confdirs))
+ (write-search-path-file output "GUIX_XDG_CONFIG_DIRS"
+ (string-join confdirs ":")))
+ (when (not (null? schemadirs))
+ (write-search-path-file output "GUIX_GSETTINGS_SCHEMA_DIR"
+ (string-join schemadirs ":")))
+ (when (not (null? gtk2-mod-dirs))
+ (write-search-path-file output "GUIX_GTK2_PATH"
+ (string-join gtk2-mod-dirs ":")))
+ (when (not (null? gtk3-mod-dirs))
+ (write-search-path-file output "GUIX_GTK3_PATH"
+ (string-join gtk3-mod-dirs ":")))
+ (when (not (null? gtk4-mod-dirs))
+ (write-search-path-file output "GUIX_GTK4_PATH"
+ (string-join gtk4-mod-dirs ":")))
+ (when (not (null? gio-mod-dirs))
+ (write-search-path-file output "GUIX_GIO_EXTRA_MODULES"
+ (string-join gio-mod-dirs ":"))))))))
(for-each handle-output outputs))
--
2.47.1
?
Re: [bug#75688] [PATCH 0/2] Introduce GUIX_LIBRARY_PATH to replace harmful environment variables
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87frla81yt.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (8 lines)
> It's too early for me to say if it could really work, but I'll repeat
> that a priori I'm not too fond of GUIX_LIBRARY_PATH (just the name would
> clash with a GCC env var too) and custom logic that'd need to be
> maintained in the software (we'd need to patch the software to have
> GUIX_* variables anyway but the resulting patch would be trivial); I
> think I'd rather have multiple, explicit environment variables computed
> by search paths.

v2 patches sent to use multiple variables, replace the use of wrapper
scripts in glib-or-gtk-build-system with 'search-paths.d' files.
Patches for glib, gdk-pixbuf and gtk(s) while not trivial, seems
reasonable to me..


Toggle quote (5 lines)
> Just to make sure, GUIX_* variables would be honored *on top* of their
> non GUIX_ prefixed (stock) variants, right? E.g. we wouldn't want GCC
> to stop honoring LIBRARY_PATH even if we add GUIX_LIBRARY_PATH, as that
> would confuse users.

Yes, and non prefixed ones have higher priority than GUIX_ ones, to
allow overrides.

The patches require mass rebuilds due to glib-or-gtk-build-system ->
meson/cmake/rust, etc. I haven't test them, but show be fine for review
now, if it seems fine, I could work on qt.

Also nix have the same issue, http://github.com/nixos/nixpkgs/issues/60260
Maybe we could share the maintaince of patches with them somehow.

Thanks.
L
L
Liliana Marie Prikler wrote on 23 Jan 19:08 +0100
941dd634e317a301457a6803cc5389cebf869802.camel@gmail.com
Am Mittwoch, dem 22.01.2025 um 17:42 +0800 schrieb ???:
Toggle quote (9 lines)
> > You'd still have some glue code reading .VARIABLE and
> > then doing a bunch of setenv before launching the actual program,
> > no? That glue code is currently the shell or Guile script used to
> > wrap the binaries.
>
> The point for those .VARIABLE files is without wrapper scripts
> there're no setenv call anymore, ensure that getenv only get profiles
> paths, in another words: reading from file and apply the values
> replaced setenv then getenv and apply the values.
I think this is too big a change to be sneakily added onto a
replacement for an upstream environment variable with a GUIX_-prefixed
one. I think this should be discussed more broadly, using a Guix
Consensus Document for example.

As for adding GUIX_GDK_PIXBUF_MODULE_FILES, I think this could safely
be done on gnome-team.

What do y'all think?
?
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87r04s6i1r.fsf@envs.net
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (15 lines)
> Am Mittwoch, dem 22.01.2025 um 17:42 +0800 schrieb ???:
>> > You'd still have some glue code reading .VARIABLE and
>> > then doing a bunch of setenv before launching the actual program,
>> > no? That glue code is currently the shell or Guile script used to
>> > wrap the binaries.
>>
>> The point for those .VARIABLE files is without wrapper scripts
>> there're no setenv call anymore, ensure that getenv only get profiles
>> paths, in another words: reading from file and apply the values
>> replaced setenv then getenv and apply the values.
> I think this is too big a change to be sneakily added onto a
> replacement for an upstream environment variable with a GUIX_-prefixed
> one. I think this should be discussed more broadly, using a Guix
> Consensus Document for example.

Okay, I'll try a GCD later.


Toggle quote (5 lines)
> As for adding GUIX_GDK_PIXBUF_MODULE_FILES, I think this could safely
> be done on gnome-team.
>
> What do y'all think?

Sure, I'll send a patch for that later.

Thanks.
M
M
Maxim Cournoyer wrote on 27 Jan 02:27 +0100
Re: bug#75688: GUIX_ify harmful environment variables and replace wrapper scripts with search path value files
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87ed0p59lx.fsf_-_@gmail.com
Hi,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (15 lines)
> Am Mittwoch, dem 22.01.2025 um 17:42 +0800 schrieb ???:
>> > You'd still have some glue code reading .VARIABLE and
>> > then doing a bunch of setenv before launching the actual program,
>> > no? That glue code is currently the shell or Guile script used to
>> > wrap the binaries.
>>
>> The point for those .VARIABLE files is without wrapper scripts
>> there're no setenv call anymore, ensure that getenv only get profiles
>> paths, in another words: reading from file and apply the values
>> replaced setenv then getenv and apply the values.
> I think this is too big a change to be sneakily added onto a
> replacement for an upstream environment variable with a GUIX_-prefixed
> one. I think this should be discussed more broadly, using a Guix
> Consensus Document for example.

I was thinking about the GCD too, it's a bit more work, but this
solution appears novel and impacting enough to want to ensure all Guix
participants are aware of it and discuss any shortcoming it might have
in its current form.

Toggle quote (5 lines)
> As for adding GUIX_GDK_PIXBUF_MODULE_FILES, I think this could safely
> be done on gnome-team.
>
> What do y'all think?

I've posted some comments on the issue for the later.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 27 Jan 03:59 +0100
(address . iyzsong@envs.net)
877c6h55dy.fsf_-_@gmail.com
Hello,

iyzsong@envs.net writes:

Toggle quote (5 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> Add a new function "g_build_guix_search_path_dirs" to GLIB, which in addition
> to environment variable, read search path values from etc/search-paths.d

Some typo/grammar nitpicks:

environment variableS, readS search path values from THE
etc/search-paths.d ...

Toggle quote (11 lines)
> directory of the current executable. This can be used to replace wrapper
> scripts.
>
> Use it for GUIX_GSETTINGS_SCHEMA_DIR, GUIX_GIO_EXTRA_MODULES,
> GUIX_XDG_DATA_DIRS and GUIX_XDG_CONFIG_DIRS.
>
> * gnu/packages/patches/glib-guix-search-paths.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/glib.scm (glib)[source]: Add patch.
> [native-search-paths]: Add GUIX_GSETTINGS_SCHEMA_DIR, Replace

nitpick: I'd use a '. ' instead of ', ' above (two sentences).

[...]

Toggle quote (3 lines)
> +++ 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).

Toggle quote (10 lines)
> +diff --git a/gio/giomodule.c b/gio/giomodule.c
> +index 76c2028..49b02bb 100644
> +--- a/gio/giomodule.c
> ++++ b/gio/giomodule.c
> +@@ -1330,6 +1330,13 @@ _g_io_modules_ensure_loaded (void)
> + g_io_modules_scan_all_in_directory_with_scope (module_dir, scope);
> + g_free (module_dir);
> +
> ++ /* GUIX: Load gio modules from GUIX_GIO_EXTRA_MODULES */

Let's not add credence to the Reddit's GUIX misspelling ;-). I'd drop
'GUIX: ' from the beginning of your comment, as it's already clear from
the variable name. Please add a period to make it a complete sentence.

[...]

Toggle quote (3 lines)
> +diff --git a/glib/gutils.c b/glib/gutils.c
> +index 8628a56..bc21efc 100644

[...]

Toggle quote (11 lines)
> ++gchar **
> ++g_build_guix_search_path_dirs (const gchar *variable)

> ++{
> ++ gchar **dirs = NULL;
> ++ char *value = NULL;
> ++ GStrvBuilder *builder = g_strv_builder_new ();
> ++
> ++#if defined(__linux__) || defined(__gnu_hurd__)
> ++ /* First add paths from the etc/search-paths.d, which can be used to replace wrapper script. */

I'd ensure all lines wrapped around the 80 characters mark (here and
everywhere else).

Toggle quote (8 lines)
> ++ gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
> ++ 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).

Toggle quote (6 lines)
> ++ /* 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.

Toggle quote (2 lines)
> ++ 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);

Toggle quote (41 lines)
> ++ g_free (dir_path);
> ++ }
> ++
> ++ /* Now add paths from etc/search-paths.d/VARIABLE file. */
> ++ search_paths_d = g_build_filename (out_path, "etc", "search-paths.d", NULL);
> ++ if (g_file_test (search_paths_d, G_FILE_TEST_EXISTS)) {
> ++ gchar *var_path = g_build_filename (search_paths_d, variable, NULL);
> ++ if (g_file_get_contents (var_path, &value, NULL, NULL)) {
> ++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
> ++ g_strv_builder_addv (builder, (const gchar **) dirs);
> ++ g_strfreev (dirs);
> ++ g_free (value);
> ++ }
> ++ g_free (var_path);
> ++ }
> ++ }
> ++
> ++ g_free (exe_path);
> ++ g_free (out_path);
> ++ g_free (search_paths_d);
> ++#endif
> ++
> ++ /* Then add paths from the environment variable. */
> ++ gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
> ++ if (is_setuid) /* we don't want to access arbitrary files when running as setuid. */
> ++ value = NULL;
> ++ else
> ++ value = g_strdup (g_getenv (variable));
> ++
> ++ if (value && value[0]) {
> ++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
> ++ g_strv_builder_addv (builder, (const gchar **) dirs);
> ++ g_strfreev (dirs);
> ++ }
> ++ g_free (value);
> ++
> ++ dirs = g_strv_builder_end (builder);
> ++ g_strv_builder_unref (builder);
> ++ return dirs;
> ++}

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!

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 27 Jan 04:03 +0100
(address . iyzsong@envs.net)
8734h55567.fsf_-_@gmail.com
Hi,

iyzsong@envs.net writes:

Toggle quote (6 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> This fixes <https://issues.guix.gnu.org/75523>.
>
> * gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch:

nitpick: I'd name this patch
gdk-pixbuf-honor-GUIX_GDK_PIXBUF_MODULE_FILES.patch (s/respect/honor/).

[...]

Toggle quote (34 lines)
> + (patches
> + (search-patches
> + "gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch"))))
> (build-system meson-build-system)
> (outputs '("out" "debug"))
> (arguments
> @@ -777,9 +780,8 @@ (define-public gdk-pixbuf
> ;; This file is produced by the gdk-pixbuf-loaders-cache-file
> ;; profile hook.
> (list (search-path-specification
> - (variable "GDK_PIXBUF_MODULE_FILE")
> + (variable "GUIX_GDK_PIXBUF_MODULE_FILES")
> (files (list %gdk-pixbuf-loaders-cache-file))
> - (separator #f) ;single valued
> (file-type 'regular))))
> (synopsis "Image loading library")
> (description "GdkPixbuf is a library that loads image data in various
> diff --git a/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch
> new file mode 100644
> index 0000000000..eb22761403
> --- /dev/null
> +++ b/gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch
> @@ -0,0 +1,18 @@
> +diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c
> +index e1df590..ba24cf6 100644
> +--- a/gdk-pixbuf/gdk-pixbuf-io.c
> ++++ b/gdk-pixbuf/gdk-pixbuf-io.c
> +@@ -670,6 +670,13 @@ gdk_pixbuf_io_init (void)
> + gboolean ret;
> +
> + gdk_pixbuf_io_init_builtin ();
> ++
> ++ /* Load loaders from GUIX_GDK_PIXBUF_MODULE_FILES. */

nitpick: "Load loaders" reads a bit loaded ;-). I'd write "Load modules
[...]" instead.

Toggle quote (3 lines)
> ++ gchar **guix_module_files = g_build_guix_search_path_dirs ("GUIX_GDK_PIXBUF_MODULE_FILES");
> ++ for (int i = 0; guix_module_files[i] != NULL; i++)

nitpick: I read a long time ago that ++i achieves the same and is more
efficient, though I forgot the reasons. Probably doesn't matter here,
but could be neater (here and elsewhere).

Otherwise, for this commit alone,

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

(You'll want to give this series plenty of time for others to chime in).

--
Thanks,
Maxim
?
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87sep46g07.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (17 lines)
>> * gnu/packages/patches/gdk-pixbuf-respect-GUIX_GDK_PIXBUF_MODULE_FILES.patch:
>
> nitpick: I'd name this patch
> gdk-pixbuf-honor-GUIX_GDK_PIXBUF_MODULE_FILES.patch (s/respect/honor/).
>
> [...]
>
> nitpick: "Load loaders" reads a bit loaded ;-). I'd write "Load modules
> [...]" instead.
>
>> ++ gchar **guix_module_files = g_build_guix_search_path_dirs ("GUIX_GDK_PIXBUF_MODULE_FILES");
>> ++ for (int i = 0; guix_module_files[i] != NULL; i++)
>
> nitpick: I read a long time ago that ++i achieves the same and is more
> efficient, though I forgot the reasons. Probably doesn't matter here,
> but could be neater (here and elsewhere).

Since it's not a big/hot loop, it doesn't matter here, and existed code use
"i++", so I'd leave it.

Toggle quote (7 lines)
>
> Otherwise, for this commit alone,
>
> Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>
>
> (You'll want to give this series plenty of time for others to chime in).

I had update gdk-pixbuf patch for https://issues.guix.gnu.org/75795

The "search-paths.d" part of this series surely need more work, I'll
work on that later.

Thank you!
M
M
Maxim Cournoyer wrote on 27 Jan 06:06 +0100
(name . ???)(address . iyzsong@envs.net)
87tt9k4zha.fsf_-_@gmail.com
Hi,

??? <iyzsong@envs.net> writes:

[...]

Toggle quote (4 lines)
> The patches require mass rebuilds due to glib-or-gtk-build-system ->
> meson/cmake/rust, etc. I haven't test them, but show be fine for review
> now, if it seems fine, I could work on qt.

OK. It looks promising from what I've read thus far :-).

Toggle quote (3 lines)
> Also nix have the same issue, http://github.com/nixos/nixpkgs/issues/60260
> Maybe we could share the maintaince of patches with them somehow.

That's a good idea.

--
Thanks,
Maxim
?
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87ed0o6dk5.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (8 lines)
>> +++ 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.


Toggle quote (2 lines)
>> ++ 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.


Toggle quote (11 lines)
>> ++ 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.

Toggle quote (19 lines)
>> ++ /* 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.


Toggle quote (8 lines)
> [...]
>
> 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!
I
I
iyzsong wrote on 27 Jan 14:21 +0100
[PATCH v3 1/4] gnu: glib: Support load search paths from etc/search-paths.d files.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
1df5c051d9d5e61894a49761415f69f7503451d0.1737983975.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

Add a new function "g_build_guix_search_path_dirs" to GLIB, which in addition
to environment variables, reads search path values from the etc/search-paths.d
directory of the current executable. This can be used to replace wrapper
scripts.

Use it for GUIX_GSETTINGS_SCHEMA_DIR, GUIX_GIO_EXTRA_MODULES,
GUIX_XDG_DATA_DIRS and GUIX_XDG_CONFIG_DIRS.

* gnu/packages/patches/glib-guix-search-paths.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/glib.scm (glib)[source]: Add patch.
[native-search-paths]: Add GUIX_GSETTINGS_SCHEMA_DIR. Replace
GIO_EXTRA_MODULES with GUIX_GIO_EXTRA_MODULES.

Change-Id: I1d6d113fc38b20ebd4dce195f6d9c58ce85967e4
---
gnu/local.mk | 1 +
gnu/packages/glib.scm | 9 +-
.../patches/glib-guix-search-paths.patch | 162 ++++++++++++++++++
3 files changed, 170 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/glib-guix-search-paths.patch

Toggle diff (213 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 6961b8816c..7ae66dd57d 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1438,6 +1438,7 @@ dist_patch_DATA = \
%D%/packages/patches/git-filter-repo-generate-doc.patch \
%D%/packages/patches/gklib-suitesparse.patch \
%D%/packages/patches/glib-appinfo-watch.patch \
+ %D%/packages/patches/glib-guix-search-paths.patch \
%D%/packages/patches/glib-skip-failing-test.patch \
%D%/packages/patches/glibc-2.33-riscv64-miscompilation.patch \
%D%/packages/patches/glibc-2.39-git-updates.patch \
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index e04eedb7ba..0704ba2c53 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -258,7 +258,8 @@ (define glib
(base32 "0c3vagxl77wma85qinbj974jvw96n5bvch2m7hqcwxq8fa5spsj4"))
(patches
(search-patches "glib-appinfo-watch.patch"
- "glib-skip-failing-test.patch"))
+ "glib-skip-failing-test.patch"
+ "glib-guix-search-paths.patch"))
(modules '((guix build utils)))
(snippet
'(begin
@@ -516,9 +517,13 @@ (define glib
(search-path-specification
(variable "XDG_DATA_DIRS")
(files '("share")))
+ ;; To load gsettings schemas from GTK, etc.
+ (search-path-specification
+ (variable "GUIX_GSETTINGS_SCHEMA_DIR")
+ (files '("share/glib-2.0/schemas")))
;; To load extra gio modules from glib-networking, etc.
(search-path-specification
- (variable "GIO_EXTRA_MODULES")
+ (variable "GUIX_GIO_EXTRA_MODULES")
(files '("lib/gio/modules")))))
(search-paths native-search-paths)
(synopsis "Low-level core library for GNOME projects")
diff --git a/gnu/packages/patches/glib-guix-search-paths.patch b/gnu/packages/patches/glib-guix-search-paths.patch
new file mode 100644
index 0000000000..565c045a55
--- /dev/null
+++ b/gnu/packages/patches/glib-guix-search-paths.patch
@@ -0,0 +1,162 @@
+diff --git a/gio/giomodule.c b/gio/giomodule.c
+index 76c2028..7afa8ef 100644
+--- a/gio/giomodule.c
++++ b/gio/giomodule.c
+@@ -1330,6 +1330,13 @@ _g_io_modules_ensure_loaded (void)
+ g_io_modules_scan_all_in_directory_with_scope (module_dir, scope);
+ g_free (module_dir);
+
++ /* Load modules from GUIX_GIO_EXTRA_MODULES. */
++ gchar **guix_giomodule_dirs = g_build_guix_search_path_dirs ("GUIX_GIO_EXTRA_MODULES");
++ for (int i = 0; guix_giomodule_dirs[i] != NULL; i++) {
++ g_io_modules_scan_all_in_directory_with_scope (guix_giomodule_dirs[i], scope);
++ }
++ g_strfreev (guix_giomodule_dirs);
++
+ g_io_module_scope_free (scope);
+
+ /* Initialize types from built-in "modules" */
+diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c
+index e8ccc8c..6ac0f32 100644
+--- a/gio/gsettingsschema.c
++++ b/gio/gsettingsschema.c
+@@ -354,6 +354,13 @@ initialise_schema_sources (void)
+ while (i--)
+ try_prepend_data_dir (dirs[i]);
+
++ /* Load schemas from GUIX_GSETTINGS_SCHEMA_DIR. */
++ char **guix_schema_dirs = g_build_guix_search_path_dirs ("GUIX_GSETTINGS_SCHEMA_DIR");
++ i = g_strv_length(guix_schema_dirs);
++ while (i--)
++ try_prepend_dir (guix_schema_dirs[i]);
++ g_strfreev (guix_schema_dirs);
++
+ try_prepend_data_dir (g_get_user_data_dir ());
+
+ /* Disallow loading extra schemas if running as setuid, as that could
+diff --git a/glib/gutils.c b/glib/gutils.c
+index 8628a56..0f71890 100644
+--- a/glib/gutils.c
++++ b/glib/gutils.c
+@@ -2708,6 +2708,16 @@ g_build_system_data_dirs (void)
+ data_dir_vector = g_strsplit (data_dirs, G_SEARCHPATH_SEPARATOR_S, 0);
+ #endif
+
++ /* Use data files from GUIX_XDG_DATA_DIRS. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) data_dir_vector);
++ g_strfreev (data_dir_vector);
++ data_dir_vector = g_build_guix_search_path_dirs ("GUIX_XDG_DATA_DIRS");
++ g_strv_builder_addv (builder, (const gchar **) data_dir_vector);
++ g_strfreev (data_dir_vector);
++ data_dir_vector = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return g_steal_pointer (&data_dir_vector);
+ }
+
+@@ -2800,6 +2810,16 @@ g_build_system_config_dirs (void)
+ conf_dir_vector = g_strsplit (conf_dirs, G_SEARCHPATH_SEPARATOR_S, 0);
+ #endif
+
++ /* Use config files from GUIX_XDG_CONFIG_DIRS. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) conf_dir_vector);
++ g_strfreev (conf_dir_vector);
++ conf_dir_vector = g_build_guix_search_path_dirs ("GUIX_XDG_CONFIG_DIRS");
++ g_strv_builder_addv (builder, (const gchar **) conf_dir_vector);
++ g_strfreev (conf_dir_vector);
++ conf_dir_vector = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return g_steal_pointer (&conf_dir_vector);
+ }
+
+@@ -2849,6 +2869,73 @@ g_get_system_config_dirs (void)
+ return system_config_dirs;
+ }
+
++gchar **
++g_build_guix_search_path_dirs (const gchar *variable)
++{
++ gchar **dirs = NULL;
++ char *value = NULL;
++ GStrvBuilder *builder = g_strv_builder_new ();
++
++ /* First add paths from the etc/search-paths.d, which can be used to replace wrapper script. */
++ gchar *out_path = NULL;
++ gchar *search_paths_d = NULL;
++ gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
++
++ /* For scripts, we use GUIX_MAIN_SCRIPT_PATH to find its location. */
++ if (g_strcmp0 (exe_path, g_getenv ("GUIX_INTERPRETER_PATH")) == 0) {
++ g_free (exe_path);
++ exe_path = g_getenv ("GUIX_MAIN_SCRIPT_PATH");
++ }
++
++ /* 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))) {
++ /* 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;
++ out_path = g_path_get_dirname (dir_path);
++ g_free (dir_path);
++ }
++
++ /* Now add paths from etc/search-paths.d/VARIABLE file. */
++ search_paths_d = g_build_filename (out_path, "etc", "search-paths.d", NULL);
++ if (g_file_test (search_paths_d, G_FILE_TEST_EXISTS)) {
++ gchar *var_path = g_build_filename (search_paths_d, variable, NULL);
++ if (g_file_get_contents (var_path, &value, NULL, NULL)) {
++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
++ g_strv_builder_addv (builder, (const gchar **) dirs);
++ g_strfreev (dirs);
++ g_free (value);
++ }
++ g_free (var_path);
++ }
++ }
++
++ free (exe_path);
++ g_free (out_path);
++ g_free (search_paths_d);
++
++ /* Then add paths from the environment variable. */
++ gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
++ if (is_setuid) /* we don't want to access arbitrary files when running as setuid. */
++ value = NULL;
++ else
++ value = g_strdup (g_getenv (variable));
++
++ if (value && value[0]) {
++ dirs = g_strsplit (value, G_SEARCHPATH_SEPARATOR_S, 0);
++ g_strv_builder_addv (builder, (const gchar **) dirs);
++ g_strfreev (dirs);
++ }
++ g_free (value);
++
++ dirs = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++ return dirs;
++}
++
+ /**
+ * g_nullify_pointer:
+ * @nullify_location: (not nullable): the memory address of the pointer.
+diff --git a/glib/gutils.h b/glib/gutils.h
+index efc6914..710cf27 100644
+--- a/glib/gutils.h
++++ b/glib/gutils.h
+@@ -36,6 +36,9 @@
+
+ G_BEGIN_DECLS
+
++GLIB_AVAILABLE_IN_ALL
++gchar **g_build_guix_search_path_dirs (const gchar *variable);
++
+ GLIB_AVAILABLE_IN_ALL
+ const gchar * g_get_user_name (void);
+ GLIB_AVAILABLE_IN_ALL

base-commit: 77603927fba0edc2c4d9de122aa132b968a051e5
--
2.47.1
I
I
iyzsong wrote on 27 Jan 14:21 +0100
[PATCH v3 2/4] gnu: python: Set GUIX_INTERPRETER_PATH and GUIX_MAIN_SCRIPT_PATH.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
74079e8d389a7c278bdbbecac5075c4170446edf.1737983975.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

This is used by 'g_build_guix_search_path_dirs' in our patched GLIB.

* gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch: New patch.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/python.scm (python-3.10)[source]<patches>: Add it.

Change-Id: I4588cbd087a783da1ad8c94fccda7ebf5e9f39ad
---
gnu/local.mk | 1 +
.../python-3-set-GUIX_INTERPRETER_PATH.patch | 28 +++++++++++++++++++
gnu/packages/python.scm | 3 +-
3 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch

Toggle diff (62 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 7ae66dd57d..ba355dabf8 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -2085,6 +2085,7 @@ dist_patch_DATA = \
%D%/packages/patches/python-3-arm-alignment.patch \
%D%/packages/patches/python-3-deterministic-build-info.patch \
%D%/packages/patches/python-3-search-paths.patch \
+ %D%/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch \
%D%/packages/patches/python-3-fix-tests.patch \
%D%/packages/patches/python-3-hurd-configure.patch \
%D%/packages/patches/python-angr-check-exec-deps.patch \
diff --git a/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch b/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch
new file mode 100644
index 0000000000..2f173c68c8
--- /dev/null
+++ b/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch
@@ -0,0 +1,28 @@
+The 'g_build_guix_search_path_dirs' function in our patched GLIB requires
+2 environment variables (GUIX_INTERPRETER_PATH and GUIX_MAIN_SCRIPT_PATH) to
+check if the current executable is a script launched by an interpreter, and
+find the script path if it is.
+---
+diff --git a/Modules/main.c b/Modules/main.c
+index 5bb1de2..83ada3d 100644
+--- a/Modules/main.c
++++ b/Modules/main.c
+@@ -636,6 +636,18 @@ pymain_run_python(int *exitcode)
+ prepended to sys.path.
+
+ Otherwise, main_importer_path is left unchanged. */
++
++ /* Set environment variables to support 'search-paths.d'. */
++ char *exe_path = realpath("/proc/self/exe", NULL);
++ PyObject *filename = PyUnicode_FromWideChar(config->run_filename, -1);
++ const char *main_script_path = PyUnicode_AsUTF8(filename);
++ if (exe_path != NULL && main_script_path != NULL) {
++ setenv("GUIX_INTERPRETER_PATH", exe_path, 1);
++ setenv("GUIX_MAIN_SCRIPT_PATH", main_script_path, 1);
++ }
++ free(exe_path);
++ Py_DECREF(filename);
++
+ if (pymain_get_importer(config->run_filename, &main_importer_path,
+ exitcode)) {
+ return;
diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index c5f98c3a46..7701d111c4 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -463,7 +463,8 @@ (define-public python-3.10
"python-3-fix-tests.patch"
"python-3-hurd-configure.patch"
"python-3-reproducible-build.patch"
- "python-3-search-paths.patch"))
+ "python-3-search-paths.patch"
+ "python-3-set-GUIX_INTERPRETER_PATH.patch"))
(sha256
(base32
"0j6wvh2ad5jjq5n7sjmj1k66mh6lipabavchc3rb4vsinwaq9vbf"))
--
2.47.1
I
I
iyzsong wrote on 27 Jan 14:21 +0100
[PATCH v3 3/4] gnu: gtk: Add search-paths.d support for GUIX_GTK{2, 3, 4}_PATH.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
08a25b1c35ccc1fa3869bc1cdf3b6e8f883de4d6.1737983975.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

* gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch,
gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch,
gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch: Rewrite in terms
of 'g_build_guix_search_path_dirs'.

Change-Id: Ib0748c39e56fd598f30f40b9ac3bb0f012f14c31
---
.../patches/gtk2-respect-GUIX_GTK2_PATH.patch | 64 ++++++-------------
.../patches/gtk3-respect-GUIX_GTK3_PATH.patch | 55 ++++++----------
.../patches/gtk4-respect-GUIX_GTK4_PATH.patch | 62 +++++-------------
3 files changed, 54 insertions(+), 127 deletions(-)

Toggle diff (206 lines)
diff --git a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
index 93a8ddc242..fb6c7809f9 100644
--- a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
+++ b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
@@ -1,46 +1,20 @@
-This patch makes GTK+ look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK2_PATH". This can be used
-instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
-other major versions of GTK+.
+diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
+index 50729b61a5..2d4c2c2a85 100644
+--- a/gtk/gtkmodules.c
++++ b/gtk/gtkmodules.c
+@@ -96,5 +96,15 @@ get_module_path (void)
+ result = pango_split_file_list (module_path);
+ g_free (module_path);
---- a/gtk/gtkmodules.c 2014-09-29 22:02:17.000000000 +0200
-+++ b/gtk/gtkmodules.c 2015-12-02 18:41:53.306396938 +0100
-@@ -55,6 +55,7 @@
- get_module_path (void)
- {
- const gchar *module_path_env;
-+ const gchar *module_guix_gtk2_path_env;
- const gchar *exe_prefix;
- const gchar *home_dir;
- gchar *home_gtk_dir = NULL;
-@@ -70,6 +71,7 @@
- home_gtk_dir = g_build_filename (home_dir, ".gtk-2.0", NULL);
-
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk2_path_env = g_getenv ("GUIX_GTK2_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
-
- if (exe_prefix)
-@@ -77,9 +79,21 @@
- else
- default_dir = g_build_filename (GTK_LIBDIR, "gtk-2.0", NULL);
-
-- if (module_path_env && home_gtk_dir)
-+ if (module_guix_gtk2_path_env && module_path_env && home_gtk_dir)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, module_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env && home_gtk_dir)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, module_path_env, default_dir, NULL);
-+ else if (module_path_env && home_gtk_dir)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, home_gtk_dir, default_dir, NULL);
-+ else if (module_guix_gtk2_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk2_path_env, default_dir, NULL);
- else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
++ /* GUIX: Load additional modules from GUIX_GTK2_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK2_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
diff --git a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
index 66fd2fd1c4..28e232a812 100644
--- a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
+++ b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
@@ -1,38 +1,21 @@
-This patch makes GTK+ look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK3_PATH". This can be used
-instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
-other major versions of GTK+.
-
---- a/gtk/gtkmodules.c 2015-09-20 20:09:05.060590217 +0200
-+++ b/gtk/gtkmodules.c 2015-09-20 20:10:33.423124833 +0200
-@@ -52,6 +52,7 @@
- get_module_path (void)
- {
- const gchar *module_path_env;
-+ const gchar *module_guix_gtk3_path_env;
- const gchar *exe_prefix;
- gchar *module_path;
- gchar *default_dir;
-@@ -61,6 +62,7 @@
- return result;
+diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
+index f93101c272..b57e1da802 100644
+--- a/gtk/gtkmodules.c
++++ b/gtk/gtkmodules.c
+@@ -81,6 +81,16 @@ get_module_path (void)
+ result = gtk_split_file_list (module_path);
+ g_free (module_path);
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk3_path_env = g_getenv ("GUIX_GTK3_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
++ /* GUIX: Load additional modules from GUIX_GTK3_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK3_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
- if (exe_prefix)
-@@ -68,7 +70,13 @@
- else
- default_dir = g_build_filename (_gtk_get_libdir (), "gtk-3.0", NULL);
-
-- if (module_path_env)
-+ if (module_guix_gtk3_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk3_path_env, module_path_env, default_dir, NULL);
-+ else if (module_guix_gtk3_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk3_path_env, default_dir, NULL);
-+ else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
- else
diff --git a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
index 4a60023bf7..56c202ecf4 100644
--- a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
+++ b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
@@ -1,51 +1,21 @@
-From 889294a93fc6464c2c2919bc47f6fd85ec823363 Mon Sep 17 00:00:00 2001
-From: Raghav Gururajan <rg@raghavgururajan.name>
-Date: Tue, 18 May 2021 19:57:00 -0400
-Subject: [PATCH] [PATCH]: Honor GUIX_GTK4_PATH.
-
-This patch makes GTK look for additional modules in a list of directories
-specified by the environment variable "GUIX_GTK4_PATH". This can be used
-instead of "GTK_PATH" to make GTK find modules that are incompatible with
-other major versions of GTK.
----
- gtk/gtkmodules.c | 10 +++++++++-
- 1 file changed, 9 insertions(+), 1 deletion(-)
-
diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
-index aace5dcbc9..193b6a02e9 100644
+index 51b0916624..0cd6ee7e30 100644
--- a/gtk/gtkmodules.c
+++ b/gtk/gtkmodules.c
-@@ -105,6 +105,7 @@ static char **
- get_module_path (void)
- {
- const char *module_path_env;
-+ const gchar *module_guix_gtk4_path_env;
- const char *exe_prefix;
- char *module_path;
- char *default_dir;
-@@ -114,6 +115,7 @@ get_module_path (void)
- return result;
+@@ -132,6 +132,16 @@ get_module_path (void)
+ result = split_file_list (module_path);
+ g_free (module_path);
- module_path_env = g_getenv ("GTK_PATH");
-+ module_guix_gtk4_path_env = g_getenv ("GUIX_GTK4_PATH");
- exe_prefix = g_getenv ("GTK_EXE_PREFIX");
++ /* GUIX: Load additional modules from GUIX_GTK4_PATH. */
++ GStrvBuilder *builder = g_strv_builder_new ();
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_build_guix_search_path_dirs ("GUIX_GTK4_PATH");
++ g_strv_builder_addv (builder, (const gchar **) result);
++ g_strfreev (result);
++ result = g_strv_builder_end (builder);
++ g_strv_builder_unref (builder);
++
+ return result;
+ }
- if (exe_prefix)
-@@ -121,7 +123,13 @@ get_module_path (void)
- else
- default_dir = g_build_filename (_gtk_get_libdir (), "gtk-4.0", NULL);
-
-- if (module_path_env)
-+ if (module_guix_gtk4_path_env && module_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk4_path_env, module_path_env, default_dir, NULL);
-+ else if (module_guix_gtk4_path_env)
-+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
-+ module_guix_gtk4_path_env, default_dir, NULL);
-+ else if (module_path_env)
- module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
- module_path_env, default_dir, NULL);
- else
---
-2.31.1
-
--
2.47.1
I
I
iyzsong wrote on 27 Jan 14:21 +0100
[PATCH v3 4/4] build: glib-or-gtk-build-system: Replace wrapper scripts with 'search-paths.d'.
(address . 75688@debbugs.gnu.org)(name . ???)(address . iyzsong@member.fsf.org)
06a62fdc2853a732baf3f6396d38293f38c11860.1737983975.git.iyzsong@member.fsf.org
From: ??? <iyzsong@member.fsf.org>

* guix/build/glib-or-gtk-build-system.scm (write-search-path-file): New procedure.
(gtk-module-directories): Add version to arguments.
(gsettings-schema-directories): New procedure.
(data-directories): Don't check for "/glib-2.0/schemas".
(conf-directories): New procedure.
(wrap-all-programs): Rewrite in terms of 'write-search-path-file'.

Change-Id: I1c9e8d491b96e298d1568a5e29b04c762c26e4d1
---
guix/build/glib-or-gtk-build-system.scm | 166 ++++++++++++++----------
1 file changed, 95 insertions(+), 71 deletions(-)

Toggle diff (243 lines)
diff --git a/guix/build/glib-or-gtk-build-system.scm b/guix/build/glib-or-gtk-build-system.scm
index 67a52ddad3..335a856575 100644
--- a/guix/build/glib-or-gtk-build-system.scm
+++ b/guix/build/glib-or-gtk-build-system.scm
@@ -4,6 +4,7 @@
;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2025 ??? <iyzsong@envs.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -39,6 +40,13 @@ (define-module (guix build glib-or-gtk-build-system)
;;
;; Code:
+(define* (write-search-path-file output variable value)
+ "Write VALUE to @file{etc/search-paths.d/VARIABLE} under OUTPUT."
+ (let ((search-paths.d (string-append output "/etc/search-paths.d")))
+ (mkdir-p search-paths.d)
+ (with-output-to-file (string-append search-paths.d "/" variable)
+ (lambda () (display value)))))
+
(define (subdirectory-exists? parent sub-directory)
(directory-exists? (string-append parent sub-directory)))
@@ -47,32 +55,12 @@ (define (directory-included? directory directories-list)
(fold (lambda (s p) (or (string-ci=? s directory) p))
#f directories-list))
-;; We do not include $HOME/.guix-profile/gtk-v.0 (v=2 or 3) because we do not
-;; want to mix gtk+-2 and gtk+-3 modules. See
-;; https://developer.gnome.org/gtk3/stable/gtk-running.html
-(define (gtk-module-directories inputs)
- "Check for the existence of \"libdir/gtk-v.0\" in INPUTS. Return a list
+;; We load GTK modules via the GUIX_GTK2_PATH, GUIX_GTK3_PATH and GUIX_GTK4_PATH
+;; search paths.
+(define (gtk-module-directories inputs version)
+ "Check for the existence of \"libdir/gtk-VERSION\" in INPUTS. Return a list
with all found directories."
- (let* ((version
- (cond
- ((string-match "gtk-4"
- (or (assoc-ref inputs "gtk")
- (assoc-ref inputs "source")
- ""))
- "4.0")
- ((string-match "gtk\\+-3"
- (or (assoc-ref inputs "gtk+")
- (assoc-ref inputs "source")
- ""))
- "3.0")
- ((string-match "gtk\\+-2"
- (or (assoc-ref inputs "gtk+")
- (assoc-ref inputs "source")
- ""))
- "2.0")
- (else
- "4.0"))) ; We default to version 4.0.
- (gtk-module
+ (let ((gtk-module
(lambda (input prev)
(let* ((in (match input
((_ . dir) dir)
@@ -85,27 +73,22 @@ (define (gtk-module-directories inputs)
prev)))))
(fold gtk-module '() inputs)))
-;; See
+;; XDG data files include themes, sounds, icons, etc. See:
;; http://www.freedesktop.org/wiki/DesktopThemeSpec
;; http://freedesktop.org/wiki/Specifications/sound-theme-spec
;; http://freedesktop.org/wiki/Specifications/icon-theme-spec
;;
-;; Currently desktop themes are not well supported and do not honor
-;; XDG_DATA_DIRS. One example is evince which only looks for desktop themes
-;; in $HOME/.themes (for backward compatibility) and in XDG_DATA_HOME (which
-;; defaults to $HOME/.local/share). One way to handle these applications
-;; appears to be by making $HOME/.themes a symlink to
-;; $HOME/.guix-profile/share/themes.
+;; We load them via XDG_DATA_DIRS (from profile, has higher priority) and
+;; GUIX_XDG_DATA_DIRS (application specified) search paths.
(define (data-directories inputs)
- "Check for the existence of \"$datadir/glib-2.0/schemas\" or XDG themes data
-in INPUTS. Return a list with all found directories."
+ "Check for the existence of XDG data files in INPUTS. Return a list with all found
+directories."
(define (data-directory input previous)
(let* ((in (match input
((_ . dir) dir)
(_ "")))
(datadir (string-append in "/share")))
- (if (and (or (subdirectory-exists? datadir "/glib-2.0/schemas")
- (subdirectory-exists? datadir "/sounds")
+ (if (and (or (subdirectory-exists? datadir "/sounds")
(subdirectory-exists? datadir "/themes")
(subdirectory-exists? datadir "/cursors")
(subdirectory-exists? datadir "/wallpapers")
@@ -117,15 +100,45 @@ (define (data-directories inputs)
(fold data-directory '() inputs))
+;;; XDG configuration files are expected to be installed in etc/xdg directory.
+;;; We load them via XDG_CONFIG_DIRS (from profile, has higher priority) and
+;;; GUIX_XDG_CONFIG_DIRS (application specified) search paths.
+(define (conf-directories inputs)
+ "Check for the existence of XDG configuration files in INPUTS. Return a list with
+all found directories."
+ (define (conf-directory input previous)
+ (let* ((in (match input
+ ((_ . dir) dir)
+ (_ "")))
+ (conf-dir (string-append in "/etc/xdg")))
+ (if (and (directory-exists? conf-dir)
+ (not (directory-included? conf-dir previous)))
+ (cons conf-dir previous)
+ previous)))
+
+ (fold conf-directory '() inputs))
+
+;;; GIO GSettings schemas are expected to be installed in $datadir/glib-2.0/schemas
+;;; directory. We load them via the GUIX_GSETTINGS_SCHEMA_DIR search path.
+(define (gsettings-schema-directories inputs)
+ "Check for the existence of \"$datadir/glib-2.0/schemas\" in INPUTS.
+Return a list with all found directories."
+ (define (gsettings-schema-directory input previous)
+ (let* ((in (match input
+ ((_ . dir) dir)
+ (_ "")))
+ (schema-dir (string-append in "/share/glib-2.0/schemas")))
+ (if (and (directory-exists? schema-dir)
+ (not (directory-included? schema-dir previous)))
+ (cons schema-dir previous)
+ previous)))
+
+ (fold gsettings-schema-directory '() inputs))
+
;; All GIO modules are expected to be installed in GLib's $libdir/gio/modules
;; directory. That directory has to include a file called giomodule.cache
-;; listing all available modules. GIO can be made aware of modules in other
-;; directories with the help of the environment variable GIO_EXTRA_MODULES.
-;; The official GIO documentation states that this environment variable should
-;; only be used for testing and not in a production environment. However, it
-;; appears that there is no other way of specifying multiple modules
-;; directories (NIXOS also does use this variable). See
-;; https://developer.gnome.org/gio/stable/running-gio-apps.html
+;; listing all available modules. We load them via the GUIX_GIO_EXTRA_MODULES
+;; search path.
(define (gio-module-directories inputs)
"Check for the existence of \"$libdir/gio/modules\" in the INPUTS and
returns a list with all found directories."
@@ -141,50 +154,61 @@ (define (gio-module-directories inputs)
(fold gio-module-directory '() inputs))
+
+;;; XXX: Only works for ELF executables and python3 scripts.
(define* (wrap-all-programs #:key inputs outputs
(glib-or-gtk-wrap-excluded-outputs '())
#:allow-other-keys)
"Implement phase \"glib-or-gtk-wrap\": look for GSettings schemas and
-gtk+-v.0 libraries and create wrappers with suitably set environment variables
+GTK libraries and create @file{etc/search-paths.d} with suitably set of files
if found.
Wrapping is not applied to outputs whose name is listed in
GLIB-OR-GTK-WRAP-EXCLUDED-OUTPUTS. This is useful when an output is known not
to contain any GLib or GTK+ binaries, and where wrapping would gratuitously
-add a dependency of that output on GLib and GTK+."
- ;; Do not require bash to be present in the package inputs
- ;; even when there is nothing to wrap.
- ;; Also, calculate (sh) only once to prevent some I/O.
- (define %sh (delay (search-input-file inputs "bin/bash")))
- (define (sh) (force %sh))
+add a dependency of that output on GLib and GTK."
(define handle-output
(match-lambda
((output . directory)
(unless (member output glib-or-gtk-wrap-excluded-outputs)
- (let* ((bindir (string-append directory "/bin"))
- (libexecdir (string-append directory "/libexec"))
- (bin-list (filter (negate wrapped-program?)
- (append (find-files bindir ".*")
- (find-files libexecdir ".*"))))
- (datadirs (data-directories
+ (let* ((datadirs (data-directories
(alist-cons output directory inputs)))
- (gtk-mod-dirs (gtk-module-directories
+ (confdirs (conf-directories
(alist-cons output directory inputs)))
- (gio-mod-dirs (gio-module-directories
+ (schemadirs (gsettings-schema-directories
(alist-cons output directory inputs)))
- (env-vars `(,@(if (not (null? datadirs))
- (list `("XDG_DATA_DIRS" ":" prefix ,datadirs))
- '())
- ,@(if (not (null? gtk-mod-dirs))
- (list `("GTK_PATH" ":" prefix ,gtk-mod-dirs))
- '())
- ,@(if (not (null? gio-mod-dirs))
- (list `("GIO_EXTRA_MODULES" ":"
- prefix ,gio-mod-dirs))
- '()))))
- (for-each (lambda (program)
- (apply wrap-program program #:sh (sh) env-vars))
- bin-list))))))
+ (gtk2-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "2.0"))
+ (gtk3-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "3.0"))
+ (gtk4-mod-dirs (gtk-module-directories
+ (alist-cons output directory inputs)
+ "4.0"))
+ (gio-mod-dirs (gio-module-directories
+ (alist-cons output directory inputs))))
+ (when (not (null? datadirs))
+ (write-search-path-file directory "GUIX_XDG_DATA_DIRS"
+ (string-join datadirs ":")))
+ (when (not (null? confdirs))
+ (write-search-path-file directory "GUIX_XDG_CONFIG_DIRS"
+ (string-join confdirs ":")))
+ (when (not (null? schemadirs))
+ (write-search-path-file directory "GUIX_GSETTINGS_SCHEMA_DIR"
+ (string-join schemadirs ":")))
+ (when (not (null? gtk2-mod-dirs))
+ (write-search-path-file directory "GUIX_GTK2_PATH"
+ (string-join gtk2-mod-dirs ":")))
+ (when (not (null? gtk3-mod-dirs))
+ (write-search-path-file directory "GUIX_GTK3_PATH"
+ (string-join gtk3-mod-dirs ":")))
+ (when (not (null? gtk4-mod-dirs))
+ (write-search-path-file directory "GUIX_GTK4_PATH"
+ (string-join gtk4-mod-dirs ":")))
+ (when (not (null? gio-mod-dirs))
+ (write-search-path-file directory "GUIX_GIO_EXTRA_MODULES"
+ (string-join gio-mod-dirs ":"))))))))
(for-each handle-output outputs))
--
2.47.1
?
control message for bug #75688
(address . control@debbugs.gnu.org)
87y0yw1jb7.fsf@envs.net
retitle 75688 Replace wrapper scripts with search path value files in search-paths.d
quit
M
M
Maxim Cournoyer wrote on 28 Jan 03:41 +0100
Re: bug#75688: Replace wrapper scripts with search path value files in search-paths.d
(address . iyzsong@envs.net)
87wmefu0bo.fsf_-_@gmail.com
Hi,

A few additional comments.

iyzsong@envs.net writes:

Toggle quote (4 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> Add a new function "g_build_guix_search_path_dirs" to GLIB, which in addition

nitpick: It's spelled GLib, not GLIB.

Toggle quote (13 lines)
> to environment variables, reads search path values from the etc/search-paths.d
> directory of the current executable. This can be used to replace wrapper
> scripts.
>
> Use it for GUIX_GSETTINGS_SCHEMA_DIR, GUIX_GIO_EXTRA_MODULES,
> GUIX_XDG_DATA_DIRS and GUIX_XDG_CONFIG_DIRS.
>
> * gnu/packages/patches/glib-guix-search-paths.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/glib.scm (glib)[source]: Add patch.
> [native-search-paths]: Add GUIX_GSETTINGS_SCHEMA_DIR. Replace
> GIO_EXTRA_MODULES with GUIX_GIO_EXTRA_MODULES.

Sounds good.

[...]

Toggle quote (9 lines)
> ++gchar **
> ++g_build_guix_search_path_dirs (const gchar *variable)
> ++{
> ++ gchar **dirs = NULL;
> ++ char *value = NULL;
> ++ GStrvBuilder *builder = g_strv_builder_new ();
> ++
> ++ /* First add paths from the etc/search-paths.d, which can be used to replace wrapper script. */

Please wrap at 80 chars width, here and elsewhere.

Toggle quote (10 lines)
> ++ gchar *out_path = NULL;
> ++ gchar *search_paths_d = NULL;
> ++ gchar *exe_path = g_file_read_link ("/proc/self/exe", NULL);
> ++
> ++ /* For scripts, we use GUIX_MAIN_SCRIPT_PATH to find its location. */
> ++ if (g_strcmp0 (exe_path, g_getenv ("GUIX_INTERPRETER_PATH")) == 0) {
> ++ g_free (exe_path);
> ++ exe_path = g_getenv ("GUIX_MAIN_SCRIPT_PATH");
> ++ }

I don't have an opinion yet about GUIX_MAIN_SCRIPT_PATH and
GUIX_INTERPRETER_PATH, defined in the next patch of this series. I'll
comment there.

Toggle quote (2 lines)
> ++ /* We install executables under "bin" or "libexec", can also be a subdirectory of "libexec". */

nitpick: I'd reword to something like:

Executables are installed under the bin or libexec prefix; they may also
be found in a sub-directory of libexec.

Toggle quote (4 lines)
> ++ if (exe_path && (g_str_match_string("/bin/", exe_path, FALSE) ||
> ++ g_str_match_string("/libexec/", exe_path, FALSE))) {
> ++ /* Find output directory, which is the parent directory of "bin" or "libexec". */

nitpick: Find *the* output

The rest still LGTM.

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 28 Jan 05:55 +0100
(address . iyzsong@envs.net)
87o6zrtu57.fsf_-_@gmail.com
Hi,

iyzsong@envs.net writes:

Toggle quote (8 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> This is used by 'g_build_guix_search_path_dirs' in our patched GLIB.
>
> * gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch: New patch.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/python.scm (python-3.10)[source]<patches>: Add it.

[...]

Toggle quote (8 lines)
> diff --git a/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch b/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch
> new file mode 100644
> index 0000000000..2f173c68c8
> --- /dev/null
> +++ b/gnu/packages/patches/python-3-set-GUIX_INTERPRETER_PATH.patch
> @@ -0,0 +1,28 @@
> +The 'g_build_guix_search_path_dirs' function in our patched GLIB requires

s/function/procedure/, s/GLIB/GLib/

Toggle quote (3 lines)
> +2 environment variables (GUIX_INTERPRETER_PATH and
> GUIX_MAIN_SCRIPT_PATH) to

I'd reword this to 'the GUIX_INTERPRETER_FILE and GUIX_MAIN_SCRIPT_FILE
environment variables to'

Toggle quote (3 lines)
> +check if the current executable is a script launched by an interpreter, and
> +find the script path if it is.

I'd replace 'path' by 'file name' everywhere except perhaps in
environment variables where I'd use just '_FILE' for brevitiy, as path
conventionally implies multiple entries, and the GNU project prefers to
call the path of files 'file names' (see info '(Standards) GNU Manuals',
a manual from the autoconf package):

Toggle snippet (5 lines)
Please do not use the term "pathname" that is used in Unix
documentation; use "file name" (two words) instead. We use the term
"path" only for search paths, which are lists of directory names.

Toggle quote (16 lines)
> +---
> +diff --git a/Modules/main.c b/Modules/main.c
> +index 5bb1de2..83ada3d 100644
> +--- a/Modules/main.c
> ++++ b/Modules/main.c
> +@@ -636,6 +636,18 @@ pymain_run_python(int *exitcode)
> + prepended to sys.path.
> +
> + Otherwise, main_importer_path is left unchanged. */
> ++
> ++ /* Set environment variables to support 'search-paths.d'. */
> ++ char *exe_path = realpath("/proc/self/exe", NULL);
> ++ PyObject *filename = PyUnicode_FromWideChar(config->run_filename, -1);
> ++ const char *main_script_path = PyUnicode_AsUTF8(filename);
> ++ if (exe_path != NULL && main_script_path != NULL) {

I assume the != NULL guards to avoid the condition where allocating memory
failed? Perhaps prefer to do something like:

if (!(exe_path && main_script_path)) {
printf("memory allocation failure, aborting\n");
exit(1);
}

Toggle quote (7 lines)
> ++ setenv("GUIX_INTERPRETER_PATH", exe_path, 1);
> ++ setenv("GUIX_MAIN_SCRIPT_PATH", main_script_path, 1);
> ++ }
> ++ free(exe_path);
> ++ Py_DECREF(filename);
> ++

I was wondering, if instead of having to patch every interpreter out
there, would it be possible to infer this inside glib by having a list
of interpreter base names? e.g. python, guile, etc., and when we find
these in /proc/self/exe, we look at the /proc/self/comm and extract arg1
from there, which hopefully would be the script? But that'd be fragile
given the script argument position is not guaranteed to come as arg1, it
could come later, e.g. for some scripts launched via '#/usr/bin/env -S
python3 --some-option ...'. Your solution should be more reliable for
that reason, so perhaps it's necessary.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 28 Jan 06:06 +0100
(address . iyzsong@envs.net)
87jzafttlz.fsf_-_@gmail.com
Hi,

iyzsong@envs.net writes:

Toggle quote (7 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> * gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch,
> gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch,
> gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch: Rewrite in terms
> of 'g_build_guix_search_path_dirs'.

Nitpick: not documented in info '(standards) Style of Change Logs', but
I think you don't need to put commas after the file names above. It
should rather look like below, I think:

Toggle snippet (5 lines)
* file_one
* file_two
* file_three: Rewrite in terms of ...

[...]

Toggle quote (61 lines)
> diff --git a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
> index 93a8ddc242..fb6c7809f9 100644
> --- a/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
> +++ b/gnu/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch
> @@ -1,46 +1,20 @@
> -This patch makes GTK+ look for additional modules in a list of directories
> -specified by the environment variable "GUIX_GTK2_PATH". This can be used
> -instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
> -other major versions of GTK+.
> +diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
> +index 50729b61a5..2d4c2c2a85 100644
> +--- a/gtk/gtkmodules.c
> ++++ b/gtk/gtkmodules.c
> +@@ -96,5 +96,15 @@ get_module_path (void)
> + result = pango_split_file_list (module_path);
> + g_free (module_path);
>
> ---- a/gtk/gtkmodules.c 2014-09-29 22:02:17.000000000 +0200
> -+++ b/gtk/gtkmodules.c 2015-12-02 18:41:53.306396938 +0100
> -@@ -55,6 +55,7 @@
> - get_module_path (void)
> - {
> - const gchar *module_path_env;
> -+ const gchar *module_guix_gtk2_path_env;
> - const gchar *exe_prefix;
> - const gchar *home_dir;
> - gchar *home_gtk_dir = NULL;
> -@@ -70,6 +71,7 @@
> - home_gtk_dir = g_build_filename (home_dir, ".gtk-2.0", NULL);
> -
> - module_path_env = g_getenv ("GTK_PATH");
> -+ module_guix_gtk2_path_env = g_getenv ("GUIX_GTK2_PATH");
> - exe_prefix = g_getenv ("GTK_EXE_PREFIX");
> -
> - if (exe_prefix)
> -@@ -77,9 +79,21 @@
> - else
> - default_dir = g_build_filename (GTK_LIBDIR, "gtk-2.0", NULL);
> -
> -- if (module_path_env && home_gtk_dir)
>
> -+ if (module_guix_gtk2_path_env && module_path_env && home_gtk_dir)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk2_path_env, module_path_env, home_gtk_dir, default_dir, NULL);
> -+ else if (module_guix_gtk2_path_env && home_gtk_dir)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk2_path_env, home_gtk_dir, default_dir, NULL);
> -+ else if (module_guix_gtk2_path_env && module_path_env)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk2_path_env, module_path_env, default_dir, NULL);
> -+ else if (module_path_env && home_gtk_dir)
> - module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> - module_path_env, home_gtk_dir, default_dir, NULL);
> -+ else if (module_guix_gtk2_path_env)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk2_path_env, default_dir, NULL);
> - else if (module_path_env)
> - module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> - module_path_env, default_dir, NULL);
> ++ /* GUIX: Load additional modules from GUIX_GTK2_PATH. */

I'd drop 'GUIX: ' at the front of the above comment.

Toggle quote (129 lines)
> ++ GStrvBuilder *builder = g_strv_builder_new ();
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_build_guix_search_path_dirs ("GUIX_GTK2_PATH");
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_strv_builder_end (builder);
> ++ g_strv_builder_unref (builder);
> ++
> + return result;
> + }
>
> diff --git a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
> index 66fd2fd1c4..28e232a812 100644
> --- a/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
> +++ b/gnu/packages/patches/gtk3-respect-GUIX_GTK3_PATH.patch
> @@ -1,38 +1,21 @@
> -This patch makes GTK+ look for additional modules in a list of directories
> -specified by the environment variable "GUIX_GTK3_PATH". This can be used
> -instead of "GTK_PATH" to make GTK+ find modules that are incompatible with
> -other major versions of GTK+.
> -
> ---- a/gtk/gtkmodules.c 2015-09-20 20:09:05.060590217 +0200
> -+++ b/gtk/gtkmodules.c 2015-09-20 20:10:33.423124833 +0200
> -@@ -52,6 +52,7 @@
> - get_module_path (void)
> - {
> - const gchar *module_path_env;
> -+ const gchar *module_guix_gtk3_path_env;
> - const gchar *exe_prefix;
> - gchar *module_path;
> - gchar *default_dir;
> -@@ -61,6 +62,7 @@
> - return result;
> +diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
> +index f93101c272..b57e1da802 100644
> +--- a/gtk/gtkmodules.c
> ++++ b/gtk/gtkmodules.c
> +@@ -81,6 +81,16 @@ get_module_path (void)
> + result = gtk_split_file_list (module_path);
> + g_free (module_path);
>
> - module_path_env = g_getenv ("GTK_PATH");
> -+ module_guix_gtk3_path_env = g_getenv ("GUIX_GTK3_PATH");
> - exe_prefix = g_getenv ("GTK_EXE_PREFIX");
> ++ /* GUIX: Load additional modules from GUIX_GTK3_PATH. */
> ++ GStrvBuilder *builder = g_strv_builder_new ();
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_build_guix_search_path_dirs ("GUIX_GTK3_PATH");
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_strv_builder_end (builder);
> ++ g_strv_builder_unref (builder);
> ++
> + return result;
> + }
>
> - if (exe_prefix)
> -@@ -68,7 +70,13 @@
> - else
> - default_dir = g_build_filename (_gtk_get_libdir (), "gtk-3.0", NULL);
> -
> -- if (module_path_env)
>
> -+ if (module_guix_gtk3_path_env && module_path_env)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk3_path_env, module_path_env, default_dir, NULL);
> -+ else if (module_guix_gtk3_path_env)
> -+ module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> -+ module_guix_gtk3_path_env, default_dir, NULL);
> -+ else if (module_path_env)
> - module_path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
> - module_path_env, default_dir, NULL);
> - else
>
> diff --git a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
> index 4a60023bf7..56c202ecf4 100644
> --- a/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
> +++ b/gnu/packages/patches/gtk4-respect-GUIX_GTK4_PATH.patch
> @@ -1,51 +1,21 @@
> -From 889294a93fc6464c2c2919bc47f6fd85ec823363 Mon Sep 17 00:00:00 2001
> -From: Raghav Gururajan <rg@raghavgururajan.name>
> -Date: Tue, 18 May 2021 19:57:00 -0400
> -Subject: [PATCH] [PATCH]: Honor GUIX_GTK4_PATH.
> -
> -This patch makes GTK look for additional modules in a list of directories
> -specified by the environment variable "GUIX_GTK4_PATH". This can be used
> -instead of "GTK_PATH" to make GTK find modules that are incompatible with
> -other major versions of GTK.
> ----
> - gtk/gtkmodules.c | 10 +++++++++-
> - 1 file changed, 9 insertions(+), 1 deletion(-)
> -
> diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c
> -index aace5dcbc9..193b6a02e9 100644
> +index 51b0916624..0cd6ee7e30 100644
> --- a/gtk/gtkmodules.c
> +++ b/gtk/gtkmodules.c
> -@@ -105,6 +105,7 @@ static char **
> - get_module_path (void)
> - {
> - const char *module_path_env;
> -+ const gchar *module_guix_gtk4_path_env;
> - const char *exe_prefix;
> - char *module_path;
> - char *default_dir;
> -@@ -114,6 +115,7 @@ get_module_path (void)
> - return result;
> +@@ -132,6 +132,16 @@ get_module_path (void)
> + result = split_file_list (module_path);
> + g_free (module_path);
>
> - module_path_env = g_getenv ("GTK_PATH");
> -+ module_guix_gtk4_path_env = g_getenv ("GUIX_GTK4_PATH");
> - exe_prefix = g_getenv ("GTK_EXE_PREFIX");
> ++ /* GUIX: Load additional modules from GUIX_GTK4_PATH. */
> ++ GStrvBuilder *builder = g_strv_builder_new ();
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_build_guix_search_path_dirs ("GUIX_GTK4_PATH");
> ++ g_strv_builder_addv (builder, (const gchar **) result);
> ++ g_strfreev (result);
> ++ result = g_strv_builder_end (builder);
> ++ g_strv_builder_unref (builder);
> ++
> + return result;
> + }

While I find it difficult to review a diff of a diff, the above looks
reasonable to me.

Reviewed-by: Maxim Cournoyer <maxim.cournoyer@gmail>

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 28 Jan 14:10 +0100
(address . iyzsong@envs.net)
87ed0nt76t.fsf_-_@gmail.com
Hi,

iyzsong@envs.net writes:

Toggle quote (9 lines)
> From: ??? <iyzsong@member.fsf.org>
>
> * guix/build/glib-or-gtk-build-system.scm (write-search-path-file):
> New procedure.
>
> (gtk-module-directories): Add version to arguments.
> (gsettings-schema-directories): New procedure.
> (data-directories): Don't check for "/glib-2.0/schemas".

Why not? Because it's handled by the new procedure above?

Toggle quote (3 lines)
> (conf-directories): New procedure.
> (wrap-all-programs): Rewrite in terms of 'write-search-path-file'.

OK!

[...]

Toggle quote (3 lines)
> +++ b/guix/build/glib-or-gtk-build-system.scm
> @@ -4,6 +4,7 @@

[...]

Toggle quote (7 lines)
> +(define* (write-search-path-file output variable value)
> + "Write VALUE to @file{etc/search-paths.d/VARIABLE} under OUTPUT."
> + (let ((search-paths.d (string-append output "/etc/search-paths.d")))
> + (mkdir-p search-paths.d)
> + (with-output-to-file (string-append search-paths.d "/" variable)
> + (lambda () (display value)))))

I think (call-with-output-port (lambda (port) (format port "...")))
would be a bit cleaner (as in more explicit/less magical).

Toggle quote (60 lines)
> (define (subdirectory-exists? parent sub-directory)
> (directory-exists? (string-append parent sub-directory)))
>
> @@ -47,32 +55,12 @@ (define (directory-included? directory directories-list)
> (fold (lambda (s p) (or (string-ci=? s directory) p))
> #f directories-list))
>
> -;; We do not include $HOME/.guix-profile/gtk-v.0 (v=2 or 3) because we do not
> -;; want to mix gtk+-2 and gtk+-3 modules. See
> -;; https://developer.gnome.org/gtk3/stable/gtk-running.html
> -(define (gtk-module-directories inputs)
> - "Check for the existence of \"libdir/gtk-v.0\" in INPUTS. Return a list
> +;; We load GTK modules via the GUIX_GTK2_PATH, GUIX_GTK3_PATH and GUIX_GTK4_PATH
> +;; search paths.
> +(define (gtk-module-directories inputs version)
> + "Check for the existence of \"libdir/gtk-VERSION\" in INPUTS. Return a list
> with all found directories."
> - (let* ((version
> - (cond
> - ((string-match "gtk-4"
> - (or (assoc-ref inputs "gtk")
> - (assoc-ref inputs "source")
> - ""))
> - "4.0")
> - ((string-match "gtk\\+-3"
> - (or (assoc-ref inputs "gtk+")
> - (assoc-ref inputs "source")
> - ""))
> - "3.0")
> - ((string-match "gtk\\+-2"
> - (or (assoc-ref inputs "gtk+")
> - (assoc-ref inputs "source")
> - ""))
> - "2.0")
> - (else
> - "4.0"))) ; We default to version 4.0.
> - (gtk-module
> + (let ((gtk-module
> (lambda (input prev)
> (let* ((in (match input
> ((_ . dir) dir)
> @@ -85,27 +73,22 @@ (define (gtk-module-directories inputs)
> prev)))))
> (fold gtk-module '() inputs)))
>
> -;; See
> +;; XDG data files include themes, sounds, icons, etc. See:
> ;; http://www.freedesktop.org/wiki/DesktopThemeSpec
> ;; http://freedesktop.org/wiki/Specifications/sound-theme-spec
> ;; http://freedesktop.org/wiki/Specifications/icon-theme-spec
> ;;
> -;; Currently desktop themes are not well supported and do not honor
> -;; XDG_DATA_DIRS. One example is evince which only looks for desktop themes
> -;; in $HOME/.themes (for backward compatibility) and in XDG_DATA_HOME (which
> -;; defaults to $HOME/.local/share). One way to handle these applications
> -;; appears to be by making $HOME/.themes a symlink to
> -;; $HOME/.guix-profile/share/themes.
> +;; We load them via XDG_DATA_DIRS (from profile, has higher priority) and
> +;; GUIX_XDG_DATA_DIRS (application specified) search paths.

I'd reword to "The data files are searched from the XDG_DATA_DIRS and
GUIX_XDG_DATA_DIRS search paths, with the former having precedence over
the later.

Toggle quote (13 lines)
> (define (data-directories inputs)
> - "Check for the existence of \"$datadir/glib-2.0/schemas\" or XDG themes data
> -in INPUTS. Return a list with all found directories."
> + "Check for the existence of XDG data files in INPUTS. Return a list with all found
> +directories."
> (define (data-directory input previous)
> (let* ((in (match input
> ((_ . dir) dir)
> (_ "")))
> (datadir (string-append in "/share")))
> - (if (and (or (subdirectory-exists? datadir "/glib-2.0/schemas")
> - (subdirectory-exists? datadir "/sounds")

OK, I see that's handled in gsettings-schema-directories below, as I
expected.

Toggle quote (12 lines)
> + (if (and (or (subdirectory-exists? datadir "/sounds")
> (subdirectory-exists? datadir "/themes")
> (subdirectory-exists? datadir "/cursors")
> (subdirectory-exists? datadir "/wallpapers")
> @@ -117,15 +100,45 @@ (define (data-directories inputs)
>
> (fold data-directory '() inputs))
>
> +;;; XDG configuration files are expected to be installed in etc/xdg directory.
> +;;; We load them via XDG_CONFIG_DIRS (from profile, has higher priority) and
> +;;; GUIX_XDG_CONFIG_DIRS (application specified) search paths.

Likewise, I'd use something like "The configuration files are searched
from the XDG_CONFIG_DIRS and GUIX_XDG_CONFIG_DIRS search paths, with the
former having precedence over the later."

Toggle quote (4 lines)
> +(define (conf-directories inputs)
> + "Check for the existence of XDG configuration files in INPUTS. Return a list with
> +all found directories."

80 chars limit (sorry I sound like a broken record; maybe turn on
auto-fill-mode with the fill-column variable set at 78 or the
equivalent in your editor of choice :-)).

Toggle quote (15 lines)
> + (define (conf-directory input previous)
> + (let* ((in (match input
> + ((_ . dir) dir)
> + (_ "")))
> + (conf-dir (string-append in "/etc/xdg")))
> + (if (and (directory-exists? conf-dir)
> + (not (directory-included? conf-dir previous)))
> + (cons conf-dir previous)
> + previous)))
> +
> + (fold conf-directory '() inputs))
> +
> +;;; GIO GSettings schemas are expected to be installed in $datadir/glib-2.0/schemas
> +;;; directory. We load them via the GUIX_GSETTINGS_SCHEMA_DIR search path.

in *the* $datadir/glib-2.0/schemas directory

Maybe reword to "The schemas are searched from the GSETTINGS_SCHEMA_DIR
search path."

Toggle quote (15 lines)
> +(define (gsettings-schema-directories inputs)
> + "Check for the existence of \"$datadir/glib-2.0/schemas\" in INPUTS.
> +Return a list with all found directories."
> + (define (gsettings-schema-directory input previous)
> + (let* ((in (match input
> + ((_ . dir) dir)
> + (_ "")))
> + (schema-dir (string-append in "/share/glib-2.0/schemas")))
> + (if (and (directory-exists? schema-dir)
> + (not (directory-included? schema-dir previous)))
> + (cons schema-dir previous)
> + previous)))
> +
> + (fold gsettings-schema-directory '() inputs))

I see most of the code of the above procedure gets repeated twice; you
could extract some procedure this out with a prefix argument returning a
specialized procedure.

Toggle quote (12 lines)
> ;; All GIO modules are expected to be installed in GLib's $libdir/gio/modules
> ;; directory. That directory has to include a file called giomodule.cache
> -;; listing all available modules. GIO can be made aware of modules in other
> -;; directories with the help of the environment variable GIO_EXTRA_MODULES.
> -;; The official GIO documentation states that this environment variable should
> -;; only be used for testing and not in a production environment. However, it
> -;; appears that there is no other way of specifying multiple modules
> -;; directories (NIXOS also does use this variable). See
> -;; https://developer.gnome.org/gio/stable/running-gio-apps.html
> +;; listing all available modules. We load them via the GUIX_GIO_EXTRA_MODULES
> +;; search path.

Is there a reason why GIO_EXTRA_MODULES (the natively supported
environment variable) is not mentioned like in the other cases above?
Now that I think about it, since it's not a Guix's responsibility to
process these, perhaps we can simply avoid mentioning these native
variants in the above comments as well.

Toggle quote (10 lines)
> (define (gio-module-directories inputs)
> "Check for the existence of \"$libdir/gio/modules\" in the INPUTS and
> returns a list with all found directories."
> @@ -141,50 +154,61 @@ (define (gio-module-directories inputs)
>
> (fold gio-module-directory '() inputs))
>
> +
> +;;; XXX: Only works for ELF executables and python3 scripts.

Could we fallback to what is currently done (classic wrapping) for the
other cases? We would have to detect such situation and conditionalize
what happens based on that. Or is the plan to extend to also other
scripting languages and cover every known package from the start? That
could work also, but it sounds a bit more tedious/risky.

Toggle quote (4 lines)
> (define* (wrap-all-programs #:key inputs outputs
> (glib-or-gtk-wrap-excluded-outputs '())
> #:allow-other-keys)

In its current form, this phase should be renamed
'write-search-path-files' or similar; we could introduce a deprecated
symbol for wrap-all-programs for backward compatibility.

Toggle quote (5 lines)
> "Implement phase \"glib-or-gtk-wrap\": look for GSettings schemas and
> -gtk+-v.0 libraries and create wrappers with suitably set environment variables
> +GTK libraries and create @file{etc/search-paths.d} with suitably set of files
> if found.

Maybe rewrite to: [...] create the @file{etc/search-paths.d} directory
containing one file per search path variable for which there were
matches found.

Toggle quote (74 lines)
> Wrapping is not applied to outputs whose name is listed in
> GLIB-OR-GTK-WRAP-EXCLUDED-OUTPUTS. This is useful when an output is known not
> to contain any GLib or GTK+ binaries, and where wrapping would gratuitously
> -add a dependency of that output on GLib and GTK+."
> - ;; Do not require bash to be present in the package inputs
> - ;; even when there is nothing to wrap.
> - ;; Also, calculate (sh) only once to prevent some I/O.
> - (define %sh (delay (search-input-file inputs "bin/bash")))
> - (define (sh) (force %sh))
> +add a dependency of that output on GLib and GTK."
> (define handle-output
> (match-lambda
> ((output . directory)
> (unless (member output glib-or-gtk-wrap-excluded-outputs)
> - (let* ((bindir (string-append directory "/bin"))
> - (libexecdir (string-append directory "/libexec"))
> - (bin-list (filter (negate wrapped-program?)
> - (append (find-files bindir ".*")
> - (find-files libexecdir ".*"))))
> - (datadirs (data-directories
> + (let* ((datadirs (data-directories
> (alist-cons output directory inputs)))
> - (gtk-mod-dirs (gtk-module-directories
> + (confdirs (conf-directories
> (alist-cons output directory inputs)))
> - (gio-mod-dirs (gio-module-directories
> + (schemadirs (gsettings-schema-directories
> (alist-cons output directory inputs)))
> - (env-vars `(,@(if (not (null? datadirs))
> - (list `("XDG_DATA_DIRS" ":" prefix ,datadirs))
> - '())
> - ,@(if (not (null? gtk-mod-dirs))
> - (list `("GTK_PATH" ":" prefix ,gtk-mod-dirs))
> - '())
> - ,@(if (not (null? gio-mod-dirs))
> - (list `("GIO_EXTRA_MODULES" ":"
> - prefix ,gio-mod-dirs))
> - '()))))
> - (for-each (lambda (program)
> - (apply wrap-program program #:sh (sh) env-vars))
> - bin-list))))))
> + (gtk2-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "2.0"))
> + (gtk3-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "3.0"))
> + (gtk4-mod-dirs (gtk-module-directories
> + (alist-cons output directory inputs)
> + "4.0"))
> + (gio-mod-dirs (gio-module-directories
> + (alist-cons output directory inputs))))
> + (when (not (null? datadirs))
> + (write-search-path-file directory "GUIX_XDG_DATA_DIRS"
> + (string-join datadirs ":")))
> + (when (not (null? confdirs))
> + (write-search-path-file directory "GUIX_XDG_CONFIG_DIRS"
> + (string-join confdirs ":")))
> + (when (not (null? schemadirs))
> + (write-search-path-file directory "GUIX_GSETTINGS_SCHEMA_DIR"
> + (string-join schemadirs ":")))
> + (when (not (null? gtk2-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK2_PATH"
> + (string-join gtk2-mod-dirs ":")))
> + (when (not (null? gtk3-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK3_PATH"
> + (string-join gtk3-mod-dirs ":")))
> + (when (not (null? gtk4-mod-dirs))
> + (write-search-path-file directory "GUIX_GTK4_PATH"
> + (string-join gtk4-mod-dirs ":")))
> + (when (not (null? gio-mod-dirs))
> + (write-search-path-file directory "GUIX_GIO_EXTRA_MODULES"
> + (string-join gio-mod-dirs ":"))))))))

Perhaps the check for null could be done in write-search-path-file to
avoid the repetitive checks here. Also, instead of (when (not ...)) you
can use (unless ...).

Other than that, it looks reasonable to me; it'll need a feature branch;
I'm not so sure it requires a GCD anymore since if it works correctly,
it won't impact the API nor the usage of the system. What do you think,
Liliana?

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 28 Jan 14:15 +0100
Re: bug#75688: GUIX_ify harmful environment variables and replace wrapper scripts with search path value files
(name . ???)(address . iyzsong@envs.net)
87a5bbt6zo.fsf@gmail.com
Hello,

??? <iyzsong@envs.net> writes:

Toggle quote (12 lines)
> Maxim Cournoyer <maxim.cournoyer@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].


[...]

Toggle quote (23 lines)
>>> ++ /* 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.

Toggle quote (12 lines)
>> [...]
>>
>> 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
L
L
Liliana Marie Prikler wrote on 28 Jan 19:07 +0100
Re: bug#75688: Replace wrapper scripts with search path value files in search-paths.d
f1f0dceaba6d11c1f290026f4f5a0af1682e2225.camel@gmail.com
Am Dienstag, dem 28.01.2025 um 22:10 +0900 schrieb Maxim Cournoyer:
Toggle quote (4 lines)
> Other than that, it looks reasonable to me; it'll need a feature
> branch; I'm not so sure it requires a GCD anymore since if it works
> correctly, it won't impact the API nor the usage of the system.  What
> do you think, Liliana?
I think it does deserve a GCD. I'm very much not interested in
maintaining a specific search-pathesque feature on gnome-team only. In
my opinion, there are several design decisions to discuss, e.g. file
naming, content layout, …, and if we do go that route, then IMHO we
should also offer at least one — nicer would be multiple — reference
implementation(s) if not proper ready-to-use packages.

I share both ???'s concern that this feels hacky and your concern that
we ought to improve over environment variables. Now, they do say that
too many cooks spoil the broth, but in my opinion cooking our own soup
is ill-advised in this instance :)

Cheers
M
M
Maxim Cournoyer wrote 7 days ago
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87ed0mqlov.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (17 lines)
> Am Dienstag, dem 28.01.2025 um 22:10 +0900 schrieb Maxim Cournoyer:
>> Other than that, it looks reasonable to me; it'll need a feature
>> branch; I'm not so sure it requires a GCD anymore since if it works
>> correctly, it won't impact the API nor the usage of the system.  What
>> do you think, Liliana?
> I think it does deserve a GCD. I'm very much not interested in
> maintaining a specific search-pathesque feature on gnome-team only. In
> my opinion, there are several design decisions to discuss, e.g. file
> naming, content layout, …, and if we do go that route, then IMHO we
> should also offer at least one — nicer would be multiple — reference
> implementation(s) if not proper ready-to-use packages.
>
> I share both ???'s concern that this feels hacky and your concern that
> we ought to improve over environment variables. Now, they do say that
> too many cooks spoil the broth, but in my opinion cooking our own soup
> is ill-advised in this instance :)

???, a GCD involves some writing and time to allow for discussing,
but if you have the will to drive it I'd be happy to sponsor it.

--
Thanks,
Maxim
?
?
??? wrote 7 days ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87jzaei2tp.fsf@envs.net
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (16 lines)
> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>> Am Dienstag, dem 28.01.2025 um 22:10 +0900 schrieb Maxim Cournoyer:
>>> Other than that, it looks reasonable to me; it'll need a feature
>>> branch; I'm not so sure it requires a GCD anymore since if it works
>>> correctly, it won't impact the API nor the usage of the system.  What
>>> do you think, Liliana?
>> I think it does deserve a GCD. I'm very much not interested in
>> maintaining a specific search-pathesque feature on gnome-team only. In
>> my opinion, there are several design decisions to discuss, e.g. file
>> naming, content layout, …, and if we do go that route, then IMHO we
>> should also offer at least one — nicer would be multiple — reference
>> implementation(s) if not proper ready-to-use packages.

Sure, the other target is qt-build-system and KDE, it's not GNOME only.

Toggle quote (9 lines)
>>
>> I share both ???'s concern that this feels hacky and your concern that
>> we ought to improve over environment variables. Now, they do say that
>> too many cooks spoil the broth, but in my opinion cooking our own soup
>> is ill-advised in this instance :)
>
> ???, a GCD involves some writing and time to allow for discussing,
> but if you have the will to drive it I'd be happy to sponsor it.

No problem, I'll write a GCD later. While I'm not optimistic about that
we can upstream the patches we needed, but I think the maintaince burden
is reasonable.


Thank you!
?
?
??? wrote 6 days ago
Re: [bug#75688] Replace wrapper scripts with search path value files in search-paths.d
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87cyg499gc.fsf@envs.net
Hello, I write a draft GCD as "Set search paths without program wrappers",
need feedbacks and corrections.

Maxim and Liliana, could I list you as sponsors and send it to
guix-devel? Thank you!
?
?
??? wrote 6 days ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
878qqs98gw.fsf@envs.net
??? <iyzsong@envs.net> writes:

Toggle quote (3 lines)
> Hello, I write a draft GCD as "Set search paths without program wrappers",
> need feedbacks and corrections.

Add drawbacks to GCD below.

Toggle quote (2 lines)
> # Drawbacks or Open Questions

If implemented, we would likely carry several custom patches for GLib,
Qt, Python, etc. forever, and programs could fail in some subtle ways.

Toggle quote (4 lines)
> This propose focus to solve problems caused by program wrappers in desktop
> environments, namely GNOME and KDE, individual `wrap-progam` usages are not
> addressed. We plan to handle that in build systems later, for examples:
> [...]
M
M
Maxim Cournoyer wrote 3 days ago
(name . ???)(address . iyzsong@envs.net)
87y0ypk4wn.fsf@gmail.com
Hello!

??? <iyzsong@envs.net> writes:

Toggle quote (6 lines)
> Hello, I write a draft GCD as "Set search paths without program wrappers",
> need feedbacks and corrections.
>
> Maxim and Liliana, could I list you as sponsors and send it to
> guix-devel? Thank you!

Yes from me!

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote 3 days ago
Re: bug#75688: Replace wrapper scripts with search path value files in search-paths.d
(name . ???)(address . iyzsong@envs.net)
87tt9djgbl.fsf_-_@gmail.com
Hi,

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

Toggle quote (12 lines)
> Hello!
>
> ??? <iyzsong@envs.net> writes:
>
>> Hello, I write a draft GCD as "Set search paths without program wrappers",
>> need feedbacks and corrections.
>>
>> Maxim and Liliana, could I list you as sponsors and send it to
>> guix-devel? Thank you!
>
> Yes from me!

Some typos spot in the text:

Toggle quote (4 lines)
> # The Cost Of Reverting

> By limiting the creating of search path configuration files in build systems,

s/creating/creation/

Toggle quote (2 lines)
> we can revert to program wrappers by several commits.

/by several commits/by manually adding wrap phases on a case by case
basis, if needed/

Toggle quote (4 lines)
> # Drawbacks or Open Questions

> This propose focus to solve problems caused by program wrappers in desktop

This proposal focuses solving problems [...]

Toggle quote (2 lines)
> environments, namely GNOME and KDE, individual `wrap-progam` usages are not

I'd put a full stop (period) after GNOME and KDE. Individual [...]

Toggle quote (2 lines)
> addressed. We plan to handle that in build systems later, for examples:

s/for examples/for example/

Toggle quote (8 lines)
> - Handle `GUIX_GI_TYPELIB_PATH` and `GUIX_GDK_PIXBUF_MODULE_FILES` in
> `glib-or-gtk-build-system`.
> - Handle `GUIX_PYTHONPATH` in `python-build-system`.


> There are still ABI problems caused by environment variables from
> profiles, which maybe addressed later as suggested by Maxime Devos in

s/maybe/may be/

--
Thanks,
Maxim
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 75688@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 75688
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch