-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
meson: De-dup rpath arguments #13819
Merged
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
This fixes spammy warnings on apple clang: ld: warning: duplicate -rpath 'build/dist/darwin_universal/arm64/lib/pkgconfig/../../lib' ignored
Trivially correct patch, IMO. Will merge in 5 days if there are no complaints. |
This was merged with failing CI and the CI failure is directly related to the code change itself. (OT: still getting back into the flow of things after being entirely off for the holiday.) |
eli-schwartz
added a commit
to eli-schwartz/meson
that referenced
this pull request
Oct 31, 2024
This reverts commit 8242187. This same change was previously introduced in mesonbuild#12495 / commit 30ab974 and subsequently reverted in mesonbuild#12672 / commit 2fbc7b5 Reintroduced in mesonbuild#13819 but it's still the same old problem. To repeat the problem: This breaks at least: - frameworks/17 mpi - frameworks/30 scalapack The problem is that openmpi's pkg-config emitted link arguments includes: ``` -Wl,-rpath -Wl,/path/to/libdir ``` The deduplication logic in meson doesn't contain sufficient information to tell when the compiler is passing an argument that requires values, and definitely cannot tell when that argument is split across argv. But for arguments that *can* do this, it is not possible to deduplicate a single argument as standalone, because it is not standalone. The argument for deduplicating rpath here was that if you have multiple dependencies that all add the same rpath, the Apple ld64 emits a non-fatal warning "duplicate -rpath ignored". Since this is non-fatal, it's not a major issue. A major issue is when builds fatally error out with: ``` FAILED: scalapack_c cc -o scalapack_c scalapack_c.p/main.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group /usr/lib64/libscalapack.so /usr/lib64/liblapack.so /usr/lib64/libblas.so -Wl,-rpath -Wl,/usr/lib64 -Wl,/usr/lib64 -Wl,--enable-new-dtags /usr/lib64/libmpi.so -Wl,--end-group /usr/libexec/gcc/x86_64-pc-linux-gnu/ld: error: /usr/lib64: read: Is a directory ```
eli-schwartz
added a commit
to eli-schwartz/meson
that referenced
this pull request
Oct 31, 2024
This reverts commit 8242187. This same change was previously introduced in mesonbuild#12495 / commit 30ab974 and subsequently reverted in mesonbuild#12672 / commit 2fbc7b5 Reintroduced in mesonbuild#13819 but it's still the same old problem. To repeat the problem: This breaks at least: - frameworks/17 mpi - frameworks/30 scalapack The problem is that openmpi's pkg-config emitted link arguments includes: ``` -Wl,-rpath -Wl,/path/to/libdir ``` The deduplication logic in meson doesn't contain sufficient information to tell when the compiler is passing an argument that requires values, and definitely cannot tell when that argument is split across argv. But for arguments that *can* do this, it is not possible to deduplicate a single argument as standalone, because it is not standalone. The argument for deduplicating rpath here was that if you have multiple dependencies that all add the same rpath, the Apple ld64 emits a non-fatal warning "duplicate -rpath ignored". Since this is non-fatal, it's not a major issue. A major issue is when builds fatally error out with: ``` FAILED: scalapack_c cc -o scalapack_c scalapack_c.p/main.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group /usr/lib64/libscalapack.so /usr/lib64/liblapack.so /usr/lib64/libblas.so -Wl,-rpath -Wl,/usr/lib64 -Wl,/usr/lib64 -Wl,--enable-new-dtags /usr/lib64/libmpi.so -Wl,--end-group /usr/libexec/gcc/x86_64-pc-linux-gnu/ld: error: /usr/lib64: read: Is a directory ``` Today we have CI for this so the change actually caused our own unittest CI to go red.
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.
This fixes spammy warnings on apple clang:
ld: warning: duplicate -rpath 'build/dist/darwin_universal/arm64/lib/pkgconfig/../../lib' ignored