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

@@ -35,9 +35,15 @@ static gboolean
 is_valid_module_name (const gchar *basename)
 {
 #if !defined(G_OS_WIN32) && !defined(G_WITH_CYGWIN)
+  #if defined(G_OS_DARWIN)
+  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

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(G_OS_DARWIN)
+  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;

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(G_OS_DARWIN)
+  #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);

gio/tests/meson.build

@@ -66,10 +66,7 @@ gio_tests = {
   'file-thumbnail' : {},
   'fileattributematcher' : {},
   'filter-streams' : {},
-  'giomodule' : {
-    # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/1392
-    'should_fail' : host_system == 'darwin',
-  },
+  'giomodule' : {},
   'gsubprocess' : {},
   'g-file' : {},
   'g-file-info' : {},

glib/glibconfig.h.in

@@ -187,7 +187,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 only emit deprecation
+ * warnings on platforms where usage of this macro is broken */
+#if defined(__APPLE__) || defined(_MSC_VER) || defined(__CYGWIN__)
+#define G_MODULE_SUFFIX "@g_module_suffix@" GLIB_DEPRECATED_MACRO_IN_2_76
+#else
 #define G_MODULE_SUFFIX "@g_module_suffix@"
+#endif
 
 typedef @g_pid_type@ GPid;
 #define G_PID_FORMAT @g_pid_format@

glib/meson.build

@@ -25,7 +25,6 @@ gvisibility_h = custom_target(
 
 glib_built_headers = [gversionmacros_h, gvisibility_h]
 glib_sources += glib_built_headers
-
 glib_c_args_internal = [
   '-DGLIB_COMPILATION',
 ]

gmodule/gmodule-ar.c

@@ -167,18 +167,3 @@ _g_module_symbol (gpointer     handle,
   
   return p;
 }
-
-static gchar*
-_g_module_build_path (const gchar *directory,
-		      const gchar *module_name)
-{
-  if (directory && *directory) {
-    if (strncmp (module_name, "lib", 3) == 0)
-      return g_strconcat (directory, "/", module_name, NULL);
-    else
-      return g_strconcat (directory, "/lib", module_name, "." G_MODULE_SUFFIX, NULL);
-  } else if (strncmp (module_name, "lib", 3) == 0)
-    return g_strdup (module_name);
-  else
-    return g_strconcat ("lib", module_name, "." G_MODULE_SUFFIX, NULL);
-}

gmodule/gmodule-deprecated.c

@@ -0,0 +1,29 @@
+#include "config.h"
+
+/* 
+ * This is the only way to disable deprecation warnings for macros, and we need
+ * to continue using G_MODULE_SUFFIX in the implementation of
+ * g_module_build_path() which is also deprecated API.
+ */
+#define GLIB_DISABLE_DEPRECATION_WARNINGS
+#include <glib.h>
+
+#if (G_MODULE_IMPL == G_MODULE_IMPL_AR) || (G_MODULE_IMPL == G_MODULE_IMPL_DL)
+G_GNUC_INTERNAL gchar*    _g_module_build_path (const gchar *directory,
+                                                const gchar *module_name);
+
+gchar*
+_g_module_build_path (const gchar *directory,
+		      const gchar *module_name)
+{
+  if (directory && *directory) {
+    if (strncmp (module_name, "lib", 3) == 0)
+      return g_strconcat (directory, "/", module_name, NULL);
+    else
+      return g_strconcat (directory, "/lib", module_name, "." G_MODULE_SUFFIX, NULL);
+  } else if (strncmp (module_name, "lib", 3) == 0)
+    return g_strdup (module_name);
+  else
+    return g_strconcat ("lib", module_name, "." G_MODULE_SUFFIX, NULL);
+}
+#endif

gmodule/gmodule-dl.c

@@ -210,18 +210,3 @@ _g_module_symbol (gpointer     handle,
   
   return p;
 }
-
-static gchar*
-_g_module_build_path (const gchar *directory,
-		      const gchar *module_name)
-{
-  if (directory && *directory) {
-    if (strncmp (module_name, "lib", 3) == 0)
-      return g_strconcat (directory, "/", module_name, NULL);
-    else
-      return g_strconcat (directory, "/lib", module_name, "." G_MODULE_SUFFIX, NULL);
-  } else if (strncmp (module_name, "lib", 3) == 0)
-    return g_strdup (module_name);
-  else
-    return g_strconcat ("lib", module_name, "." G_MODULE_SUFFIX, NULL);
-}

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.
  */
 
 /**
@@ -219,8 +234,14 @@ static void		_g_module_close		(gpointer	 handle);
 static gpointer		_g_module_self		(void);
 static gpointer		_g_module_symbol	(gpointer	 handle,
 						 const gchar	*symbol_name);
+#if (G_MODULE_IMPL != G_MODULE_IMPL_DL) && (G_MODULE_IMPL != G_MODULE_IMPL_AR)
 static gchar*		_g_module_build_path	(const gchar	*directory,
 						 const gchar	*module_name);
+#else
+/* Implementation is in gmodule-deprecated.c */
+gchar*		        _g_module_build_path	(const gchar	*directory,
+						 const gchar	*module_name);
+#endif
 static inline void	g_module_set_error	(const gchar	*error);
 static inline GModule*	g_module_find_by_handle (gpointer	 handle);
 static inline GModule*	g_module_find_by_name	(const gchar	*name);
@@ -481,24 +502,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
  *
@@ -568,12 +593,58 @@ 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;
+      size_t prefix_idx = 0, suffix_idx = 0;
+      const char *prefixes[2] = {0}, *suffixes[2] = {0};
+
+      basename = g_path_get_basename (file_name);
+      dirname = g_path_get_dirname (file_name);
+#ifdef G_OS_WIN32
+      if (!g_str_has_prefix (basename, "lib"))
+        prefixes[prefix_idx++] = "lib";
+      prefixes[prefix_idx++] = "";
+      if (!g_str_has_suffix (basename, ".dll"))
+        suffixes[suffix_idx++] = ".dll";
+#else
+  #ifdef __CYGWIN__
+      if (!g_str_has_prefix (basename, "cyg"))
+        prefixes[prefix_idx++] = "cyg";
+  #else
+      if (!g_str_has_prefix (basename, "lib"))
+        prefixes[prefix_idx++] = "lib";
+      else
+        /* People commonly pass `libfoo` as the file_name and want us to
+         * auto-detect the suffix as .la or .so, etc. We need to also find
+         * .dylib and .dll in those cases. */
+        prefixes[prefix_idx++] = "";
+  #endif
+  #ifdef G_OS_DARWIN
+      if (!g_str_has_suffix (basename, ".dylib") &&
+          !g_str_has_suffix (basename, ".so"))
+        {
+          suffixes[suffix_idx++] = ".dylib";
+          suffixes[suffix_idx++] = ".so";
+        }
+  #else
+      if (!g_str_has_suffix (basename, ".so"))
+        suffixes[suffix_idx++] = ".so";
+  #endif
+#endif
+      for (guint i = 0; i < prefix_idx; i++)
+        {
+          for (guint j = 0; j < suffix_idx; 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_free (basename);
+      g_free (dirname);
     }
   /* try completing by appending libtool suffix */
   if (!name)
@@ -592,8 +663,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
@@ -687,8 +759,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.
  *
@@ -891,6 +963,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,

gmodule/gmodule.h

@@ -138,7 +138,7 @@ const gchar *         g_module_name          (GModule      *module);
  *
  * No checks are made that the file exists, or is of correct type.
  */
-GMODULE_AVAILABLE_IN_ALL
+GMODULE_DEPRECATED_IN_2_76
 gchar*                g_module_build_path    (const gchar  *directory,
 					      const gchar  *module_name);
 

gmodule/meson.build

@@ -75,7 +75,8 @@ gvisibility_h = custom_target(
   install_dir: g_module_includedir,
 )
 
-gmodule_sources = [gmodule_c, gvisibility_h]
+gmodule_sources = [gmodule_c, gvisibility_h, 'gmodule-deprecated.c']
+
 if host_system == 'windows'
   gmodule_win_rc = configure_file(
     input: 'gmodule.rc.in',