Subject: Re: bug#75688: Replace wrapper scripts with search path value files in search-paths.d

From: Maxim Cournoyer <[email-masked]>
Date: Tue, 28 Jan 2025 14:06:32 +0900

Hi,

[email-masked] writes:

> From: 宋文武 <[email-masked]>
>
> * 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:

--8<---------------cut here---------------start------------->8---
* file_one
* file_two
* file_three: Rewrite in terms of ...
--8<---------------cut here---------------end--------------->8---

[...]

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

> ++  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 <[email-masked]>
> -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