fix bad rpath in chapel-py when using --prefix installs#26388
fix bad rpath in chapel-py when using --prefix installs#26388arezaii merged 3 commits intochapel-lang:mainfrom
Conversation
1514187 to
e9eb904
Compare
tools/chapel-py/setup.py
Outdated
| if chpl_install_lib_path is not None: | ||
| LDFLAGS += [ | ||
| "-L{}".format(chpl_install_lib_path), | ||
| "-Wl,-rpath,{}".format(chpl_install_lib_path), | ||
| ] | ||
|
|
There was a problem hiding this comment.
Does this not mean that we are now embedding 2 rpaths? one for the source directory and one for the installed directory?
There was a problem hiding this comment.
yes, and I believe even more are added
There was a problem hiding this comment.
Should it be that way? We are embedding rpaths that we never intend to use, which seems like an issue to me
There was a problem hiding this comment.
The python build system seems to need the rpath during the build, and we need a different one later after install, so unless we go back to using chrpath or other post build binary manipulation, I think we're stuck with it :(
There was a problem hiding this comment.
Ugh yeah its required to generate the pyi files. Ok well thats very dissatisfying and is going to be a pain elsewhere (fedora namely, although the chrpath stuff should continue to work around this). But its fine for this PR
There was a problem hiding this comment.
The Linux loader should ignore extraneous RPATH entries, but some of the Linux (namely Fedora) packaging stuff does complain if you have unused/dead/broken RPATH entries.
There was a problem hiding this comment.
Well chrpath should always be safe to remove unused RPATH entries at install time, it's just fragile to assume that it can always successfully replace/insert arbitrary entries.
There was a problem hiding this comment.
Yes, that is what I said here. I am ok with these dead paths, since chrpath is already being used to rewrite RPATHs in the package builds.
There was a problem hiding this comment.
some of the Linux (namely Fedora) packaging stuff does complain if you have unused/dead/broken RPATH entries
I ran into this in the spack builds (on some linux) until I fixed the formatting of the previously existing LDFLAGS (per the diff you slacked me at one point). Just curious if that might be the same symptom or something else.
There was a problem hiding this comment.
Possibly related, although I have had issues with other binaries besides just chapel-py
tools/chapel-py/setup.py
Outdated
| if chpl_install_lib_path is not None: | ||
| LDFLAGS += [ | ||
| "-L{}".format(chpl_install_lib_path), | ||
| "-Wl,-rpath,{}".format(chpl_install_lib_path), | ||
| ] | ||
|
|
There was a problem hiding this comment.
The Linux loader should ignore RPATH directories that are inaccessible (or don't contain a matching *.so), so extraneous rpath entries should be "mostly harmless".
On the other hand, relying on chrpath at install time is a fragile design in general, because usually chrpath can only replace an RPATH with one of equal or shorter length. So installing linked executables to a sufficiently long install prefix might "just break" if that insertion is delayed until a post-link chrpath step.
Signed-off-by: Ahmad Rezaii <[email protected]>
eebf11a to
fe6cf54
Compare
Signed-off-by: Ahmad Rezaii <[email protected]>
Signed-off-by: Ahmad Rezaii <[email protected]>
c2238f6 to
061f944
Compare
|
🎉 |
This fixes an issue where installing Chapel using
--prefixwould not encode the correct rpath in the shared library that was created for python. The change here looks for the presence of aconfigured-prefixfile in the build tree and uses that and the chapel version fromCMakeLists.txtto encode the proper rpath based on the installation destination.[reviewed by @jabraham17 and @bonachea - thanks!]