Merge branch 'nirbheek/gmodule-suffix-deprecation' into 'main'

Improve g_module_open(), deprecate G_MODULE_SUFFIX

Closes #520 e #1413

See merge request GNOME/glib!2950

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' : {},
@@ -859,8 +856,6 @@ if meson.can_run_host_binaries()
   gio_tests += {
     'resources' : {
       'extra_sources' : resources_extra_sources,
-      # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/1392
-      'should_fail' : host_system == 'darwin',
     },
   }
 endif

gio/tests/resources.c

@@ -805,7 +805,11 @@ test_uri_query_info (void)
   g_assert_nonnull (content_type);
   mime_type = g_content_type_get_mime_type (content_type);
   g_assert_nonnull (mime_type);
+#ifdef G_OS_DARWIN
+  g_assert_cmpstr (mime_type, ==, "text/*");
+#else
   g_assert_cmpstr (mime_type, ==, "text/plain");
+#endif
   g_free (mime_type);
 
   g_object_unref (info);

glib/glibconfig.h.in

@@ -188,7 +188,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

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

glib/tests/fileutils.c

@@ -751,8 +751,8 @@ test_mkdir_with_parents (void)
 
 #ifndef G_OS_WIN32
   g_assert_cmpint (g_mkdir_with_parents ("/usr/b/c", 0), ==, -1);
-  /* EPERM may be returned if the filesystem as a whole is read-only */
-  if (errno != EPERM)
+  /* EPERM or EROFS may be returned if the filesystem as a whole is read-only */
+  if (errno != EPERM && errno != EROFS)
     g_assert_cmpint (errno, ==, EACCES);
 #endif
 

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)
@@ -593,7 +664,8 @@ 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

@@ -77,7 +77,8 @@ gvisibility_h = custom_target(
   install_tag: 'devel',
 )
 
-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',