[glib/nirbheek/gmodule-suffix-deprecation] Improve g_module_open(), deprecate G_MODULE_SUFFIX

[Nirbheek Chauhan <nirbheekc src gnome org>]

commit ac547b383b0eba34a79fd68c1e4d50d13f476495
Author: Nirbheek Chauhan <nirbheek centricular com>
Date:   Fri Oct 14 04:34:13 2022 +0530

    Improve g_module_open(), deprecate G_MODULE_SUFFIX
    
    G_MODULE_SUFFIX is deprecated now because you will get the wrong
    results using it most of the time:
    
    1. The suffix on macOS is usually 'dylib', but it's 'so' when using
       Autotools, so there's no way to get the suffix correct using
       a pre-processor macro.
    2. Prefixes also vary in a platform-specific way. You may or may not have
       a 'lib' prefix for the name on Windows and on Cygwin the prefix is
       'cyg'.
    3. The library name itself can vary per platform. For instance, you may
       want to load foo-1.dll on Windows and libfoo.1.dylib on macOS. This
       is for libraries, not modules, but that is still a use-case that
       people use the GModule API for.
    
    g_module_build_path() does take care of (2) on Cygwin, but it
    fundamentally cannot handle the possibility of multiple options for
    the module name, since it does not do any I/O. Hence, it is also
    deprecated.
    
    Instead, g_module_open() has been improved so that it takes care of
    all this by searching the filesystem for combinations of possible
    suffixes and prefixes on each platform. Along the way, the
    documentation for it was also improved to make it clearer what it
    does.
    
    Closes https://gitlab.gnome.org/GNOME/glib/-/issues/520
    
    Closes https://gitlab.gnome.org/GNOME/glib/-/issues/1413

 gio/gio-querymodules.c  |   7 +++
 gio/giomodule.c         |   7 +++
 gio/tests/giomodule.c   |  14 ++++--
 glib/glibconfig.h.in    |   6 +++
 glib/gnulib/meson.build |   2 +-
 glib/meson.build        |   2 +-
 gmodule/gmodule.c       | 123 ++++++++++++++++++++++++++++++++++++++----------
 gmodule/gmodule.h       |   2 +-
 meson.build             |   1 +
 9 files changed, 131 insertions(+), 33 deletions(-)
---
diff --git a/gio/gio-querymodules.c b/gio/gio-querymodules.c
index faddbcfb86..62ba3c69b7 100644
--- a/gio/gio-querymodules.c
+++ b/gio/gio-querymodules.c
@@ -34,9 +34,16 @@ static gboolean
 is_valid_module_name (const gchar *basename)
 {
 #if !defined(G_OS_WIN32) && !defined(G_WITH_CYGWIN)
+#if defined(__APPLE__)
+  return
+    g_str_has_prefix (basename, "lib") &&
+    (g_str_has_suffix (basename, ".so") ||
+     g_str_has_suffix (basename, ".dylib"));
+#else
   return
     g_str_has_prefix (basename, "lib") &&
     g_str_has_suffix (basename, ".so");
+#endif
 #else
   return g_str_has_suffix (basename, ".dll");
 #endif
diff --git a/gio/giomodule.c b/gio/giomodule.c
index f5dbb4555f..8ad6d72a8f 100644
--- a/gio/giomodule.c
+++ b/gio/giomodule.c
@@ -430,9 +430,16 @@ is_valid_module_name (const gchar        *basename,
   gboolean result;
 
 #if !defined(G_OS_WIN32) && !defined(G_WITH_CYGWIN)
+#if defined(__APPLE__)
+  if (!g_str_has_prefix (basename, "lib") ||
+      !(g_str_has_suffix (basename, ".so") ||
+        g_str_has_suffix (basename, ".dylib")))
+    return FALSE;
+#else
   if (!g_str_has_prefix (basename, "lib") ||
       !g_str_has_suffix (basename, ".so"))
     return FALSE;
+#endif
 #else
   if (!g_str_has_suffix (basename, ".dll"))
     return FALSE;
diff --git a/gio/tests/giomodule.c b/gio/tests/giomodule.c
index 4ea6efebd1..962eda26de 100644
--- a/gio/tests/giomodule.c
+++ b/gio/tests/giomodule.c
@@ -23,10 +23,16 @@
 #include <gio/gio.h>
 #include <glibconfig.h>
 
-#ifdef _MSC_VER
-# define MODULE_FILENAME_PREFIX ""
+#ifdef G_OS_WIN32
+  #ifdef _MSC_VER
+    #define MODULE_FILENAME(x) "" x ".dll"
+  #else
+    #define MODULE_FILENAME(x) "lib" x ".dll"
+  #endif
+#elif defined(__APPLE__)
+  #define MODULE_FILENAME(x) "lib" x ".dylib"
 #else
-# define MODULE_FILENAME_PREFIX "lib"
+  #define MODULE_FILENAME(x) "lib" x ".so"
 #endif
 
 static void
@@ -131,7 +137,7 @@ test_module_scan_all_with_scope (void)
 
       ep = g_io_extension_point_register ("test-extension-point");
       scope = g_io_module_scope_new (G_IO_MODULE_SCOPE_BLOCK_DUPLICATES);
-      g_io_module_scope_block (scope, MODULE_FILENAME_PREFIX "testmoduleb." G_MODULE_SUFFIX);
+      g_io_module_scope_block (scope, MODULE_FILENAME ("testmoduleb"));
       g_io_modules_scan_all_in_directory_with_scope (g_test_get_filename (G_TEST_BUILT, "modules", NULL), 
scope);
       list = g_io_extension_point_get_extensions (ep);
       g_assert_cmpint (g_list_length (list), ==, 1);
diff --git a/glib/glibconfig.h.in b/glib/glibconfig.h.in
index ffedee1d2a..ea06123272 100644
--- a/glib/glibconfig.h.in
+++ b/glib/glibconfig.h.in
@@ -186,7 +186,13 @@ typedef unsigned @glib_intptr_type_define@ guintptr;
 #define GLIB_SYSDEF_POLLERR =@g_pollerr@
 #define GLIB_SYSDEF_POLLNVAL =@g_pollnval@
 
+/* No way to disable deprecation warnings for macros, so don't enable the
+ * deprecation when building glib */
+#if !defined(GLIB_COMPILATION)
+#define G_MODULE_SUFFIX "@g_module_suffix@" GLIB_DEPRECATED_MACRO_IN_2_74
+#else
 #define G_MODULE_SUFFIX "@g_module_suffix@"
+#endif
 
 typedef @g_pid_type@ GPid;
 #define G_PID_FORMAT @g_pid_format@
diff --git a/glib/gnulib/meson.build b/glib/gnulib/meson.build
index 38b530aa0b..1646b67d64 100644
--- a/glib/gnulib/meson.build
+++ b/glib/gnulib/meson.build
@@ -366,6 +366,6 @@ gnulib_lib = static_library('gnulib', gnulib_sources,
   dependencies : [libm],
   include_directories : [configinc, glibinc, include_directories ('.')],
   pic : true,
-  c_args : ['-DGCC_LINT=1', '-DLIBDIR="@0@"'.format(glib_libdir), '-DGLIB_COMPILATION', 
'-DG_LOG_DOMAIN="GLib"' ] + glib_hidden_visibility_args + extra_gnulib_args)
+  c_args : ['-DGCC_LINT=1', '-DLIBDIR="@0@"'.format(glib_libdir), '-DG_LOG_DOMAIN="GLib"' ] + 
glib_hidden_visibility_args + extra_gnulib_args)
 
 gnulib_libm_dependency = [libm]
diff --git a/glib/meson.build b/glib/meson.build
index c365901a13..26ca3a58da 100644
--- a/glib/meson.build
+++ b/glib/meson.build
@@ -368,7 +368,7 @@ if use_pcre2_static_flag
   pcre2_static_args = ['-DPCRE2_STATIC']
 endif
 
-glib_c_args = ['-DG_LOG_DOMAIN="GLib"', '-DGLIB_COMPILATION'] + pcre2_static_args + 
glib_hidden_visibility_args
+glib_c_args = ['-DG_LOG_DOMAIN="GLib"'] + pcre2_static_args + glib_hidden_visibility_args
 libglib = library('glib-2.0',
   glib_dtrace_obj, glib_dtrace_hdr,
   sources : [deprecated_sources, glib_sources],
diff --git a/gmodule/gmodule.c b/gmodule/gmodule.c
index f324882abb..8f89f0f812 100644
--- a/gmodule/gmodule.c
+++ b/gmodule/gmodule.c
@@ -161,9 +161,24 @@
 /**
  * G_MODULE_SUFFIX:
  *
- * Expands to the proper shared library suffix for the current platform
- * without the leading dot. For most Unices and Linux this is "so", and
- * for Windows this is "dll".
+ * Expands to a shared library suffix for the current platform without the
+ * leading dot. On Unixes this is "so", and on Windows this is "dll".
+ *
+ * Deprecated: 2.76: Use g_module_open() instead with @module_name as the
+ * basename of the file_name argument. You will get the wrong results using
+ * this macro most of the time:
+ *
+ * 1. The suffix on macOS is usually 'dylib', but it's 'so' when using
+ *    Autotools, so there's no way to get the suffix correct using
+ *    a pre-processor macro.
+ * 2. Prefixes also vary in a platform-specific way. You may or may not have
+ *    a 'lib' prefix for the name on Windows and on Cygwin the prefix is
+ *    'cyg'.
+ * 3. The library name itself can vary per platform. For instance, you may
+ *    want to load foo-1.dll on Windows and libfoo.1.dylib on macOS.
+ *
+ * g_module_open() takes care of all this by searching the filesystem for
+ * combinations of possible suffixes and prefixes.
  */
 
 /**
@@ -476,24 +491,28 @@ static GRecMutex g_module_global_lock;
 
 /**
  * g_module_open_full:
- * @file_name: (nullable): the name of the file containing the module, or %NULL
- *     to obtain a #GModule representing the main program itself
+ * @file_name: (nullable): the name or path to the file containing the module,
+ *     or %NULL to obtain a #GModule representing the main program itself
  * @flags: the flags used for opening the module. This can be the
  *     logical OR of any of the #GModuleFlags
  * @error: #GError.
  *
- * Opens a module. If the module has already been opened,
- * its reference count is incremented.
+ * Opens a module. If the module has already been opened, its reference count
+ * is incremented. If not, the module is searched in the following order:
  *
- * First of all g_module_open_full() tries to open @file_name as a module.
- * If that fails and @file_name has the ".la"-suffix (and is a libtool
- * archive) it tries to open the corresponding module. If that fails
- * and it doesn't have the proper module suffix for the platform
- * (%G_MODULE_SUFFIX), this suffix will be appended and the corresponding
- * module will be opened. If that fails and @file_name doesn't have the
- * ".la"-suffix, this suffix is appended and g_module_open_full() tries to open
- * the corresponding module. If eventually that fails as well, %NULL is
- * returned.
+ * 1. If @file_name exists as a regular file, it is used as-is; else
+ * 2. If @file_name doesn't have the correct suffix and/or prefix for the
+ *    platform, then possible suffixes and prefixes will be added to the
+ *    basename till a file is found and whatever is found will be used; else
+ * 3. If @file_name doesn't have the ".la"-suffix, ".la" is appended. Either
+ *    way, if a matching .la file exists (and is a libtool archive) the
+ *    libtool archive is parsed to find the actual file name, and that is
+ *    used.
+ *
+ * At the end of all this, we would have a file path that we can access on
+ * disk, and it is opened as a module. If not, @file_name is opened as
+ * a module verbatim in the hopes that the system implementation will somehow
+ * be able to access it.
  *
  * Returns: a #GModule on success, or %NULL on failure
  *
@@ -563,12 +582,60 @@ g_module_open_full (const gchar   *file_name,
   /* try completing file name with standard library suffix */
   if (!name)
     {
-      name = g_strconcat (file_name, "." G_MODULE_SUFFIX, NULL);
-      if (!g_file_test (name, G_FILE_TEST_IS_REGULAR))
-       {
-         g_free (name);
-         name = NULL;
-       }
+      char *basename, *dirname;
+      GStrv prefixes, suffixes;
+      GStrvBuilder *prefix_builder, *suffix_builder;
+
+      prefix_builder = g_strv_builder_new ();
+      suffix_builder = g_strv_builder_new ();
+      basename = g_path_get_basename (file_name);
+      dirname = g_path_get_dirname (file_name);
+#if G_OS_WIN32
+  #ifdef __CYGWIN__
+      if (!g_str_has_prefix (basename, "cyg"))
+        g_strv_builder_add (prefix_builder, "cyg");
+  #else
+      if (!g_str_has_prefix (basename, "lib"))
+        g_strv_builder_add (prefix_builder, "lib");
+      g_strv_builder_add (prefix_builder, "");
+  #endif
+      if (!g_str_has_suffix (basename, ".dll"))
+        g_strv_builder_add (suffix_builder, ".dll");
+#else
+      if (!g_str_has_prefix (basename, "lib"))
+        g_strv_builder_add (prefix_builder, "lib");
+  #ifdef __APPLE__
+      if (!g_str_has_suffix (basename, ".dylib") &&
+          !g_str_has_suffix (basename, ".so")) {
+        g_strv_builder_add (suffix_builder, ".dylib");
+        g_strv_builder_add (suffix_builder, ".so");
+      }
+  #else
+      if (!g_str_has_suffix (basename, ".so"))
+        g_strv_builder_add (suffix_builder, ".so");
+  #endif
+#endif
+      prefixes = g_strv_builder_end (prefix_builder);
+      suffixes = g_strv_builder_end (suffix_builder);
+      for (guint i = 0; i < g_strv_length (prefixes); i++)
+        {
+          for (guint j = 0; j < g_strv_length (suffixes); j++)
+            {
+              name = g_strconcat (dirname, G_DIR_SEPARATOR_S, prefixes[i],
+                  basename, suffixes[j], NULL);
+              if (g_file_test (name, G_FILE_TEST_IS_REGULAR))
+                {
+                  goto name_found;
+                }
+              g_free (name);
+              name = NULL;
+            }
+        }
+name_found:
+      g_strfreev (prefixes);
+      g_strfreev (suffixes);
+      g_free (basename);
+      g_free (dirname);
     }
   /* try completing by appending libtool suffix */
   if (!name)
@@ -587,8 +654,9 @@ g_module_open_full (const gchar   *file_name,
     {
       gchar *dot = strrchr (file_name, '.');
       gchar *slash = strrchr (file_name, G_DIR_SEPARATOR);
-      
-      /* make sure the name has a suffix */
+
+      /* we make sure the name has a suffix using the deprecated
+       * G_MODULE_SUFFIX for backward-compat */
       if (!dot || dot < slash)
        name = g_strconcat (file_name, "." G_MODULE_SUFFIX, NULL);
       else
@@ -682,8 +750,8 @@ g_module_open_full (const gchar   *file_name,
 
 /**
  * g_module_open:
- * @file_name: (nullable): the name of the file containing the module, or %NULL
- *     to obtain a #GModule representing the main program itself
+ * @file_name: (nullable): the name or path to the file containing the module,
+ *     or %NULL to obtain a #GModule representing the main program itself
  * @flags: the flags used for opening the module. This can be the
  *     logical OR of any of the #GModuleFlags.
  *
@@ -886,6 +954,9 @@ g_module_name (GModule *module)
  *
  * Returns: the complete path of the module, including the standard library
  *     prefix and suffix. This should be freed when no longer needed
+ *
+ * Deprecated: 2.76: Use g_module_open() instead with @module_name as the
+ * basename of the file_name argument. See %G_MODULE_SUFFIX for why.
  */
 gchar *
 g_module_build_path (const gchar *directory,
diff --git a/gmodule/gmodule.h b/gmodule/gmodule.h
index 9744890412..ed3b2f9955 100644
--- a/gmodule/gmodule.h
+++ b/gmodule/gmodule.h
@@ -135,7 +135,7 @@ const gchar *         g_module_name          (GModule      *module);
  *
  * No checks are made that the file exists, or is of correct type.
  */
-GLIB_AVAILABLE_IN_ALL
+GLIB_DEPRECATED_IN_2_74
 gchar*                g_module_build_path    (const gchar  *directory,
                                              const gchar  *module_name);
 
diff --git a/meson.build b/meson.build
index 43bb468322..2fcf0fbe78 100644
--- a/meson.build
+++ b/meson.build
@@ -178,6 +178,7 @@ glibconfig_conf = configuration_data()
 # use them later in test programs (autoconf does this automatically)
 glib_conf_prefix = ''
 
+glib_conf.set('GLIB_COMPILATION', true)
 glib_conf.set('GLIB_MAJOR_VERSION', major_version)
 glib_conf.set('GLIB_MINOR_VERSION', minor_version)
 glib_conf.set('GLIB_MICRO_VERSION', micro_version)