-
-
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
build: use libwhich to improve installation reliability #24796
Conversation
This will require vtjnash/libwhich#1 to work on FreeBSD. |
d22cbc4
to
866228f
Compare
OK to merge? |
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.
Overall looks good to me! Just two small questions
- make $BUILDOPTS NO_GIT=1 build-stats | ||
- du -sk /tmp/julia/* | ||
- ls -l /tmp/julia/lib | ||
- ls -l /tmp/julia/lib/julia |
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.
Is this still necessary?
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.
nah, not essential, just thought it would be useful output (given how much non-useful output we have ...)
SYMLINK_SYSTEM_LIBRARIES += symlink_$(LIBMNAME) | ||
else ifneq ($(USE_SYSTEM_OPENLIBM),0) | ||
SYMLINK_SYSTEM_LIBRARIES += symlink_$(LIBMNAME) | ||
endif |
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.
I'm confused about this $(LIBMNAME)
business, it looks like you're calling symlink_system_library()
on $(LIBMNAME)
always, but then you add it on to SYMLINK_SYSTEM_LIBRARIES
which is itself eventually going to symlink_$(LIBNAME)
eventually? Aren't we doing work twice here?
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.
I'm calling symlink_system_library
with one argument in this particular case, which skips adding it to SYMLINK_SYSTEM_LIBRARIES
(so I can do different custom logic here than standard for the rest)
I had missed this, but that's really cool. This is exactly what I was doing for Fedora RPMs, and it allowed me to get rid of that code. (Though the drawback is that I'll have to manage the libwhich tarball manually.) |
endif | ||
ifeq ($(USE_SYSTEM_LIBSSH2),0) | ||
JL_PRIVATE_LIBS += ssh2 | ||
JL_PRIVATE_LIBS-0 += $(LIBMNAME) |
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.
@vtjnash I've noticed that the libopenlibm.so
symlink is created but not installed when USE_SYSTEM_LIBM=0 USE_SYSTEM_OPENLIBM=1
. Is this because we lack JL_PRIVATE_LIBS-1 += $(LIBMNAME)
in that case? Currently I have to create the link manually to fix the RPM package, and it will likely affect other distributions.
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.
Yes, please fix this
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.
OK. I've filed #29388.
This removes the last remaining dependence on
ldconfig -p
(to merge #22828), as well as DYLD_FALLBACK_LIBRARY_PATH (to fix #24789).Rather than depending on the unreliable implementation of
ldconfig -p
that we use right now, or DYLD_FALLBACK_LIBRARY_PATH (which is now often broken by macOS SIP), this uses thelibwhich
program to create the correct symlinks in lib/julia during build-time for anyUSE_SYSTEM_*
libraries. This ensures that the Julia runtime will now be able to reliably load the correct library.fix #6742