-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: Avoid setting the RPATH for LLVM #22352
Conversation
@jlbuild !nuke |
Status of 1c973df builds:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I don't know if this was related to any of your PRs, but I noticed we're creating some of these variables as normal recursively expanded ones, instead of as simply-expanded ones. We may want to add this patch to declare these variables as being of type
|
No, I think the variable declaration was a preexisting condition. I've added your patch as a separate commit. Thanks! |
Why is llvm picky about this? leave a comment by the CMAKE_COMMON saying not to put anything rpath related there for this reason, so we aren't tempted to do this again by accident? |
¯\_(ツ)_/¯ It isn't on FreeBSD, which is why I never noticed this
Good idea, will do once CI is done so I don't end up just queuing up a ton of builds here Edit: Done, squashed into first commit |
90cfa81
to
9e37db0
Compare
ifeq ($(OS),FreeBSD) | ||
ifneq ($(GCCPATH),) | ||
INSTALL_RPATH := "\$$ORIGIN:$(GCCPATH)" | ||
CMAKE_COMMON += -DCMAKE_INSTALL_RPATH="\$$ORIGIN:$(GCCPATH)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually ends up duplicating the RPATH specification on FreeBSD, though I nuked everything, rebuilt from scratch, and tested everything, and things are fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reverts commit 8b8fa11. Ref: JuliaLang#22352
Tweak the order of libgcc_s in DT_NEEDED. Make FreeBSD do not require `BUILD_CUSTOM_LIBCXX`. See also: JuliaLang#21788, JuliaLang#22352
Tweak the order of libgcc_s in DT_NEEDED. Make FreeBSD do not require `BUILD_CUSTOM_LIBCXX`. See also: JuliaLang#21788, JuliaLang#22352
Tweak the order of libgcc_s in DT_NEEDED on FreeBSD so that building on FreeBSD does not require `BUILD_CUSTOM_LIBCXX`. See also: JuliaLang#21788, JuliaLang#22352
Some of the changes in #21788 adversely affected Linux. This partially reverts/restructures some of the changes.