Skip to content
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

Revert "meson: De-dup rpath arguments" #13852

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

eli-schwartz
Copy link
Member

This reverts commit 8242187.

This same change was previously introduced in #12495 / commit 30ab974 and subsequently reverted in #12672 / commit 2fbc7b5

Reintroduced in #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.

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.
@eli-schwartz eli-schwartz merged commit 6091ef0 into mesonbuild:master Oct 31, 2024
28 of 29 checks passed
@eli-schwartz eli-schwartz deleted the revert-13819 branch October 31, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant