GNOME Bugzilla – Bug 757261
g-ir-scanner fails incorrectly on systems linking with --as-needed by default
Last modified: 2018-02-08 12:39:39 UTC
Created attachment 314338 [details] [review] patch to re-add support for systems defaulting to --as-needed when linking. Changes in gobject-introspection 1.46.0 to support Microsoft toolchains have eliminated the fix for https://bugzilla.gnome.org/show_bug.cgi?id=699442 causing failure when processing a library with no GObjects in distributions that default to --as-needed when linking. Running g-ir-scanner results in an error "ERROR: can't resolve libraries to shared libraries: XXXXX" where XXXXX is the argument passed to --library= on the command line. See the above-linked bug for a test case to reproduce the failure.
Review of attachment 314338 [details] [review]: Thanks for the patch! First, can you please submit the patch in `git format-patch` style? See https://wiki.gnome.org/Newcomers/SubmittingPatches The matrix of cases here makes my head spin... One thing jumps out at me - you're modifying the `if not libtool` code. Are you really building something here without libtool? I thought that code was basically only used by dconf. ::: giscanner/ccompiler.py @@ +38,3 @@ compiler = None _cflags_no_deprecation_warnings = '' + _linker_preargs = [] Okay, so `extra_preargs` was unused, but the `get_internal_link_flags()` function is intended to append an array of arguments that gets passed down - if there's something going wrong with that logic we should fix it. Rather than caching it secretly here.
> One thing jumps out at me - you're modifying the `if not libtool` code. > Are you really building something here without libtool? I thought that > code was basically only used by dconf. Well, libtool is a part of the autotools suite, and our project is using CMake, so I can reliably say we're building without libtool. We use glib and gir, but not GObject. We're not the only project in this boat. It's possible the same problem would obtain if using libtool. > Okay, so `extra_preargs` was unused, but the `get_internal_link_flags()` > function is intended to append an array of arguments that gets passed > down - if there's something going wrong with that logic we should fix it. Appending all arguments is invalid logic for a program like ld in which order of arguments is contextual. In this case, the --no-as-needed needs to come *before* the library to be linked (ie. prepended) and not after (appended) otherwise it has zero effect on platforms such as Debian and Ubuntu in which the default is to only resolve referenced symbols. Order counts for arguments to ld. Perhaps the correct solution is to have get_internal_link_flags() accept linker flags and prepend them as well as library names and search paths that get appended?
(In reply to Stephen M. Webb from comment #2) > Well, libtool is a part of the autotools suite, and our project is using > CMake, so I can reliably say we're building without libtool. We use glib > and gir, but not GObject. We're not the only project in this boat. Ah, of course. Makes sense, thanks. > It's possible the same problem would obtain if using libtool. Yeah, it's possible. > Appending all arguments is invalid logic for a program like ld in which > order of arguments is contextual. In this case, the --no-as-needed needs to > come *before* the library to be linked (ie. prepended) Okay; but what felt strange is we were hardcoding the argument in one function but using it distantly in another. I think I'd say let's try just hardcoding the flag always, regardless of libtool - except not on MSVC?
Created attachment 347381 [details] [review] Pass -Wl,--no-as-needed at the beginning of the commandline when not using MSVC This flag needs to go at the start, as ther order of the arguments matters in ld. -- I just came across this bug / patch again, as we accidentally dropped it in Ubuntu and it broke building some packages. Maybe this version addresses the feedback? It moves the -Wl,--no-as-needed into link() directly (so it's not being added distantly), and also adds it into the libtool case. I didn't find a way to unify adding them - it really needs to go into extra_preargs as distutils assembles the commandline itself.
Review of attachment 347381 [details] [review]: In general, and since it's tested, this looks good to me, except for an issue… ::: giscanner/ccompiler.py @@ -135,3 @@ - # out their names later - if sys.platform != 'darwin': - args.append('-Wl,--no-as-needed') Isn't this a behavioural change?
(In reply to Emmanuele Bassi (:ebassi) from comment #5) > Review of attachment 347381 [details] [review] [review]: > > In general, and since it's tested, this looks good to me, except for an > issue… Ah, not that this version of the patch is not shipped by us - I was trying to respond to Colin's review. > Isn't this a behavioural change? You mean the 'if not darwin' bit, right? Aha, I see what happened - I was working from the patch that existed before, but that seems to be from before 4a724ac699f0c34fba2fb452cfadea11540325e8 was committed. So I think I should fix both places to be 'if not msvc and not darwin'?
Created attachment 347465 [details] [review] Pass -Wl,--no-as-needed at the beginning of the commandline Only when not using MSVC and not on darwin, as these do not understand the flag. This flag needs to go at the start, as ther order of the arguments matters in ld.
I'm getting conflicts due to bug #781525. Does anyone know offhand whether those changes obsolete the ones here?
-- 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/gobject-introspection/issues/149.