Add -Wl,-rpath-link for secondary dependencies
#5647
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SoapGentoo
force-pushed
the
extract-all-rpath-link-paths
branch
2 times, most recently
from
9b0ac1e to
d5c0545
Compare
SoapGentoo
changed the title
-Wl,-rpath-link for indirect dependencies-Wl,-rpath-link for secondary dependencies
SoapGentoo
force-pushed
the
extract-all-rpath-link-paths
branch
from
d5c0545 to
58f4680
Compare
SoapGentoo
force-pushed
the
extract-all-rpath-link-paths
branch
from
58f4680 to
6ac8be5
Compare
|
I haven't yet looked at the implementation but i've added a note to #3111 some time ago about this:
So yes, we need either -rpath or -rpath-link and i believe -rpath-link is not harmful even if for other reasons -rpath is needed for an path as well. |
|
We need to make sure we have deduplication support for this. Otherwise this will be a performance regression. |
SoapGentoo
force-pushed
the
extract-all-rpath-link-paths
branch
from
6ac8be5 to
5ef2390
Compare
dcbaker
requested changes
Sep 5, 2019
There was a problem hiding this comment.
Like @jpakkane I don't see another way to solve this. What makes me most uncomfortable about this is that only gnu ld and llvm lld implement rpath-link (and wrappers around them like xild on linux), so we are potentially setting people using other linkers (apple and solaris) up for obnoxious bugs.
I do have one coding change I'd like to see, otherwise from a code point of view I'm okay with this.
mesonbuild/dependencies/base.py
Outdated
| link_args = [] | ||
| if self.clib_compiler and get_compiler_is_linuxlike(self.clib_compiler): |
There was a problem hiding this comment.
lets not use get_compiler_is_linuxlike please. I'm trying to delete that. Something like:
if self.clib_compiler and self.clib.compiler.linker and isinstance(self.clib_compiler.linker, linkers.GnuLikeDynamicLinkerMixin):There was a problem hiding this comment.
linkers.GnuLikeDynamicLinkerMixin exists nowhere in the Meson codebase
There was a problem hiding this comment.
you need to rebase, it's in mesonbuild/linkers.py
SoapGentoo
force-pushed
the
extract-all-rpath-link-paths
branch
from
5ef2390 to
6b5a269
Compare
|
This breaks secondary dependencies finding and causes a lot of undefined reference errors under JHBuild. Please see #6027. |
gnomesysadmins
pushed a commit
to GNOME/jhbuild
that referenced
this pull request
Oct 21, 2019
Meson 0.47 changes the way to handle external dependencies on shared libraries significantly. Instead of just putting arguments provided by pkg-config on the linker command line, meson tries to convert -L and -l arguments into absolute paths to .so files by itself. This should make library handling more reliable because an absolute path is less likely to go wrong than an ordered list of search paths. However, due to the big change, a few regressions is still introduced, causing modules to fail to build in a JHBuild environment from time to time. Ideally regressions should be fixed in a few weeks and made available in subsequent releases. Unfortunately, I can't get my patches merged upstream in one year, and I have to remember to patch meson manually every time JHBuild updates meson. This is annoying because there are always modules failing to build because of these known bugs if I forget to patch it. Therefore, I think it is time to submit these patches to JHBuild, so it can be applied automatically. The above patches have been added to FreeBSD ports since Meson 0.48. The CI runner currently used to test GLib on FreeBSD also uses it. Given that they has been used for one year without issues, it should be safe to use them to JHBuild as well. mesonbuild/meson#4270 mesonbuild/meson#4324 mesonbuild/meson#4271 mesonbuild/meson#4325 Meson 0.52 introduces a new library path regression. It now tries to add -Wl,-rpath-link when pkg-config --static returns more -L arguments than the default pkg-config call. The intention is to tell GNU ld.bfd where it can find dependencies of shared libraries, but the implementation does it in a wrong order and creates more undefined reference errors than it fixes in JHBuild environments. Since JHBuild always does native builds and LD_LIBRARY_PATH is set, -Wl,-rpath-link arguments pointing to installation prefixes are unnecessary in JHBuild environments. Therefore, it is safe to work around the issue by reverting the commit introducing the feature temporarily. mesonbuild/meson#5647 mesonbuild/meson#6027 mesonbuild/meson#6031 Meson 0.52 also changes the way to handle static libraries. It is known to break builds for dconf and gnome-builder. Unfortunately, these two projects don't fix them in a week and we are going to update meson without fixing them first. We backport a patch to remove duplicates from the linker command line to resolve 'argument list too long' error when building gnome-builder. This doesn't fix the build failure, but it should make it easier to debug the link_whole issue. mesonbuild/meson#5936 mesonbuild/meson#6030 https://gitlab.gnome.org/GNOME/dconf/issues/59 https://gitlab.gnome.org/GNOME/gnome-builder/issues/1057
gnomesysadmins
pushed a commit
to GNOME/jhbuild
that referenced
this pull request
Oct 22, 2019
Meson 0.47 changes the way to handle external dependencies on shared libraries significantly. Instead of just putting arguments provided by pkg-config on the linker command line, meson tries to convert -L and -l arguments into absolute paths to .so files by itself. This should make library handling more reliable because an absolute path is less likely to go wrong than an ordered list of search paths. However, due to the big change, a few regressions is still introduced, causing modules to fail to build in a JHBuild environment from time to time. Ideally regressions should be fixed in a few weeks and made available in subsequent releases. Unfortunately, I can't get my patches merged upstream in one year, and I have to remember to patch meson manually every time JHBuild updates meson. This is annoying because there are always modules failing to build because of these known bugs if I forget to patch it. Therefore, I think it is time to submit these patches to JHBuild, so it can be applied automatically. The above patches have been added to FreeBSD ports since Meson 0.48. The CI runner currently used to test GLib on FreeBSD also uses it. Given that they has been used for one year without issues, it should be safe to use them to JHBuild as well. mesonbuild/meson#4270 mesonbuild/meson#4324 mesonbuild/meson#4271 mesonbuild/meson#4325 Meson 0.52 introduces a new library path regression. It now tries to add -Wl,-rpath-link when pkg-config --static returns more -L arguments than the default pkg-config call. The intention is to tell GNU ld.bfd where it can find dependencies of shared libraries, but the implementation does it in a wrong order and creates more undefined reference errors than it fixes in JHBuild environments. Since JHBuild always does native builds and LD_LIBRARY_PATH is set, -Wl,-rpath-link arguments pointing to installation prefixes are unnecessary in JHBuild environments. Therefore, it is safe to work around the issue by reverting the commit introducing the feature temporarily. mesonbuild/meson#5647 mesonbuild/meson#6027 mesonbuild/meson#6031 Meson 0.52 also changes the way to handle static libraries. It is known to break builds for dconf and gnome-builder. Unfortunately, these two projects don't fix them in a week and we are going to update meson without fixing them first. We backport a patch to remove duplicates from the linker command line to resolve 'argument list too long' error when building gnome-builder. This doesn't fix the build failure, but it should make it easier to debug the link_whole issue. mesonbuild/meson#5936 mesonbuild/meson#6030 https://gitlab.gnome.org/GNOME/dconf/issues/59 https://gitlab.gnome.org/GNOME/gnome-builder/issues/1057
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@nirbheek Revert my fix commit and the test case will fail on current
master