Created attachment 208853 [details]
archive with minimal code to illustrate the bug

i suspect libtool may end up using a name like libfoo.dll, so perhaps that's why we do the same.

it could be the case that only the documentation should be fixed.

As far as my (little) experience goes, with all the third party libraries I had to load so far, the linux version is named lib<mylibrary>.so on linux and <mylibrary>.dll on windows. I tend to think this is "the standard way", so IMHO, this function should do that.

I must say I did not find a reference explicitly confirming that, but the best hint I have about it is in the tutorials on creating dlls with mingw32, where they never speak about prepending "lib" to the ".dll" names (While they do it for the ".a" - the import library - files). Oh, and the windows version of libtool's libltdl - more or less equivalent to gmodule - is named ltdl.dll.

This bug was introduced by https://git.gnome.org/browse/glib/commit/gmodule/gmodule-win32.c?id=ceea1612c4cb7b0c085e8487eb03c2177ad83f3b

This mess was started by libtool when they decided that DLL names should be prefixed with 'lib'. AFAIK, no other linker script or compiler or build system does this (f.ex, cmake builds 'foo.dll' instead of 'libfoo.dll'). Sadly, most of the GNOME ecosystem still has to use libtool, so (almost?) all DLLs built by GNOME are prefixed with 'lib', which is why that commit was (incorrectly, IMO) made.

However, this bug means that it is impossible to build a path to a non-libtool DLL on Windows; which is the majority of DLLs out there. We need to have a way to allow the user to build paths to both types of DLLs at runtime.

There is an implementation on bug 660761 which has a compile-time switch for g_module_build_path, but I think that's a bad idea since applications will want to use both types of DLLs.

I think the best way to support this is by adding new Windows-only API:

  enum GModuleDllPrefixType {
    G_MODULE_PREFIX_TYPE_INVALID,
    G_MODULE_PREFIX_TYPE_NONE,
    G_MODULE_PREFIX_TYPE_LIB,
    G_MODULE_PREFIX_TYPE_CYGWIN,
  };
  void g_module_set_dll_prefix (GModuleDllPrefixType type);

_NONE = no library prefix, _LIB = 'lib', _CYGWIN = 'cyg'

Once that's called, all further invocations of g_module_build_path() will use the prefix type that's been set.

This can be implemented with a global variable, but that will not be thread-safe. It should ideally be implemented with the Windows-specific "Thread Local Storage" API[1]. I'd recommend GPrivate but it's a very limited resource in comparison.

1. https://msdn.microsoft.com/en-us/library/windows/desktop/ms686991%28v=vs.85%29.aspx

A simpler alternative is to deprecate g_module_build_path() and recommend a new API: g_module_open_name()


 GModule *
 g_module_open_name (const gchar *directory,
                     const gchar *module_name,
                     GModuleFlags flags);

This can build the path internally with 'lib' as a prefix, check if the filename exists, and do fallbacks with '' and 'cyg' as prefixes if needed. Nothing that wants to be cross-platform should then use g_module_build_path() since it has no way of knowing what naming your DLL uses.

This is totally broken on macOS too, as discovered here:

https://github.com/valum-framework/example/issues/2
https://github.com/mesonbuild/meson/issues/3053

G_MODULE_SUFFIX should be "dylib", not "so". However, we can fix this in a backwards-compatible way by implementing g_module_open_name (or similar) as specified above.

(In reply to Nirbheek Chauhan from comment #5)
> A simpler alternative is to deprecate g_module_build_path() and recommend a
> new API: g_module_open_name()
> 
> 
>  GModule *
>  g_module_open_name (const gchar *directory,
>                      const gchar *module_name,
>                      GModuleFlags flags);
> 
> This can build the path internally with 'lib' as a prefix, check if the
> filename exists, and do fallbacks with '' and 'cyg' as prefixes if needed.
> Nothing that wants to be cross-platform should then use
> g_module_build_path() since it has no way of knowing what naming your DLL
> uses.

I’m a bit wary of APIs which check various names to see if they exist and pick the first one which does. That could lead to situations where (for example) the .la version of a library is being loaded in preference over the real library, or an old version is loaded accidentally.

It seems like, given the right set of flags, we should be able to deterministically build the path for a given library. We’d need to know whether it was built with libtool, Cygwin, MingW, etc., and combine that knowledge with the platform-specific rules to build the path.

How about

   gchar *g_module_build_path_full (const gchar *directory, const gchar *module_name, GModulePathFlags flags);

   typedef enum
     {
       G_MODULE_PATH_NONE = 0,
       G_MODULE_PATH_LIBTOOL = (1 << 0),
       G_MODULE_PATH_CYGWIN = (1 << 1),
       G_MODULE_PATH_LIB_PREFIX = (1 << 2),
       …
     } GModulePathFlags;

(I haven’t put any thought into that list of flags; it’s just to illustrate the general idea.)

We shouldn’t need a new version of g_module_open(), since passing an existing path to it means the path should be used unmodified.

Keeping the path construction separate from g_module_open() also means that g_module_path_build_full() would be a *lot* more testable (and I’d want tests for it).

Fancy putting together a patch, since you’re working on related issues at the moment? :-)

Although if we do end up doing a new version of g_module_open(), it should have a GError parameter, as per bug #574660.

*** Bug 660761 has been marked as a duplicate of this bug. ***

(In reply to Philip Withnall from comment #8)
> We shouldn’t need a new version of g_module_open(), since passing an existing 
> path to it means the path should be used unmodified.
...
> Although if we do end up doing a new version of g_module_open(), it should
> have a GError parameter, as per bug #574660.

Currently, g_module_open has some funky heuristic in it where if you pass it a path to a file without an extension (/usr/lib/projfoo/modules/libbar), it will look for .la first, then look for the platform-specific extension (so or dll).

The easiest thing to do, would be to add more heuristics there. Make it check for both `dylib` and `so` on macOS and on Windows add "lib" to `foo.dll` or remove "lib" from `libfoo.dll` till the plugin is found.

(In reply to Nirbheek Chauhan from comment #10)
> (In reply to Philip Withnall from comment #8)
> > We shouldn’t need a new version of g_module_open(), since passing an existing 
> > path to it means the path should be used unmodified.
> ...
> > Although if we do end up doing a new version of g_module_open(), it should
> > have a GError parameter, as per bug #574660.
> 
> Currently, g_module_open has some funky heuristic in it where if you pass it
> a path to a file without an extension (/usr/lib/projfoo/modules/libbar), it
> will look for .la first, then look for the platform-specific extension (so
> or dll).

Yes. My point was that if you pass an absolute path to an existing library, it won’t do that.

> The easiest thing to do, would be to add more heuristics there. Make it
> check for both `dylib` and `so` on macOS and on Windows add "lib" to
> `foo.dll` or remove "lib" from `libfoo.dll` till the plugin is found.

See my first paragraph in comment #7. If we start changing the existing heuristics around, existing applications may break. Heuristics are a bad approach to a problem which should be entirely deterministic.

(In reply to Philip Withnall from comment #11)
> (In reply to Nirbheek Chauhan from comment #10)
> > The easiest thing to do, would be to add more heuristics there. Make it
> > check for both `dylib` and `so` on macOS and on Windows add "lib" to
> > `foo.dll` or remove "lib" from `libfoo.dll` till the plugin is found.
> 
> See my first paragraph in comment #7. If we start changing the existing
> heuristics around, existing applications may break. Heuristics are a bad
> approach to a problem which should be entirely deterministic.

I'll summarize the current situation, because it feels like we're not on the same page:

macOS modules:
* Built with autotools are libfoo.so
* Built with everything else (including meson) are libfoo.dylib

Both these can be loaded by applications built with autotools or meson.

Windows modules:
* Built with autotools+MinGW are libfoo.dll
* Built with meson+MinGW are libfoo.dll
* Built with meson+MSVC are foo.dll
* Built with Cygwin (meson or autotools) are cygfoo.dll
* Built with everything else are foo.dll

All these can be loaded by applications built with any stack (perhaps not cygwin, I do not know).

Linux modules:
* Always called libfoo.so

On top of this, plugins on any platform built with autotools can be libfoo.la

GModule is a platform-agnostic API, so you should never need to know what platform your code is running on, or what platform's modules you are loading.

g_module_open() already has a priority list: .la over .PLATFORM_SUFFIX. I merely proposed that if we *don't* want a new API that makes this "search" behaviour explicit, we should extend g_module_open()'s priority list in this way:

macOS: libfoo.la > libfoo.dylib > libfoo.so
Windows: libfoo.la > foo.dll > libfoo.dll > cygfoo.dll

With this, existing applications or libraries (f.ex. libpeas) don't need to change anything to work correctly on macOS and Windows.

However, I am all for new API that corrects past mistakes, so I would also support g_module_open_at() as I described above. IMO GModulePathFlags is the wrong solution since it exposes platform details in the API, and you now need totally unnecessary platform-specific #ifdefs.

(In reply to Nirbheek Chauhan from comment #12)
> GModule is a platform-agnostic API, so you should never need to know what
> platform your code is running on, or what platform's modules you are loading.

Indeed, but my suggestion is not quite that. My suggestion is to use the flags to say “I will always build my modules using these build systems”. So you could say that you’d always build using autotools+MinGW (in which case, the platform-specific naming schemes for autotools would be used on Linux and Mac OS, and autotools+MinGW on Windows); or that you use meson+MSVC, etc.

So the flags specify for all platforms, and then GModule chooses the naming scheme from the flags which is appropriate to the currently running platform. It would expose specific build systems in the API, so runs the risk of having a lot of outdated flags in the list as build systems come and go, but it shouldn’t require users to use any #ifdefs.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/520.