-
-
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
Workaround for FreeBSD linking to outdated system libs #21788
Conversation
Make.inc
Outdated
FCFLAGS += -Wl,-rpath=$(GCCPATH) | ||
JFFLAGS += -Wl,-rpath=$(GCCPATH) | ||
|
||
# This ensures we get the right RPATH even if we're missing FFLAGS somewhere |
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.
where does this cause problems if this is left out?
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 can't recall, honestly.
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.
would be be ideal if we can add as few flags as were necessary to get things to work
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.
Okay, probably easiest to only add it to FC
then, since that's guaranteed to end up everywhere the flags are needed.
deps/llvm.mk
Outdated
# Part of the FreeBSD libgcc_s kludge | ||
ifeq ($(OS),FreeBSD) | ||
ifneq ($(GCCPATH),) | ||
LLVM_CMAKE += -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 isn't already set to anything?
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.
Not for LLVM. Looks like the only CMake dependencies that set that flag are libgit2 and mbedtls.
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.
aren't you putting it in CMAKE_COMMON? make sure that doesn't overwrite any existing settings
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.
Added logic that avoids overriding those settings.
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.
which cmake dependencies don't currently set this? maybe cleaner to always do in CMAKE_COMMON on freebsd, if that doesn't hurt anything on its own?
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.
Seems like a good idea.
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.
All CMake dependencies should have the RPATH include $ORIGIN
, and indeed we're doing it explicitly and separately for libgit2, libssh2, and mbedtls, so perhaps we should just define it centrally in CMAKE_COMMON
for all platforms?
1e8f549
to
fa257c6
Compare
_GCCMAJOR := $(shell $(FC) -dumpversion | cut -d'.' -f1) | ||
_GCCMINOR := $(shell $(FC) -dumpversion | cut -d'.' -f2) | ||
|
||
# The ports system uses major and minor for GCC < 5 (e.g. gcc49 for GCC 4.9), otherwise major only |
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.
We have a bit of code that uses -print-search-dirs
to set a STD_LIB_PATH
variable in the Windows cross-compile case, would that give the same path you're looking for 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 don't think so, since the path where GCC lives when installed from ports isn't in the default ld
search path. Though maybe I'm misunderstanding what you mean.
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.
it's a flag for gcc to tell you its own search paths which should be where it's installed
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 played around with that a bit but this solution actually seems cleaner than what I could come up with using that flag.
deps/Makefile
Outdated
@@ -163,6 +163,21 @@ DEP_LIBS_STAGED := $(filter-out suitesparse suitesparse-wrapper osxunwind,$(DEP_ | |||
## Common build target prefixes | |||
|
|||
default: | $(build_prefix) | |||
|
|||
# Part of the FreeBSD libgcc_s kludge |
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 doesn't look deps specific to me, we should maybe rearrange the patchelf definitions so they're available in the top level makefiles
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 not sure what you mean about being deps-specific. Could you clarify?
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 fits more with the kind of things the top-level Makefile
does, rather than the rest of the things deps/Makefile
does - not a big deal but it sticks out a little
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.
Yeah, it's a little ugly here. Though I'm not sure what the best way to structure the logic elsewhere would be, given that this has to happen before any dependencies are built (or at least before LLVM).
260c7bd
to
5094a1d
Compare
cc @iblis17 may want to take a look at this? |
The default version of [1]. https://github.com/freebsd/freebsd-ports/blob/master/lang/gcc/Makefile |
Yes, because it's an issue with FreeBSD's system libraries, not the ports. The only thing that would fix it on FreeBSD's side is for them to update libgcc_s to claim to be a newer GCC version or to rename libgcc_s to libcc_s, both of which have been proposed but no action has been taken AFAICT. |
I guess i misunderstand the actual issue... I'm using this config[1] on my CI. [1]. https://gist.github.com/iblis17/b4dca8221dcb96efcccde24f9cc2fa8d |
To reproduce, just clone Julia into some local directory and build without setting anything in Make.user. It should make it to near the end then fail with a version error when linking in BLAS. For reference, the changes similar to those made in this PR are done automatically by the FreeBSD Ports system when the Makefile specifies |
@ararslan ok, I have an idea to leverage ports system. |
My original solution to this was based on Ports, but I don't think we should make our build system depend on any particular external package provider, which is why I did it this way instead. |
Ok, i got your point. |
it's useful to have both be an option, if it doesn't make for too much of a mess to support both "clone and make" builds as well as packager builds that use system libraries, custom file layouts, etc |
@ararslan In (I'm managing to add |
There's already a Julia port in the Ports tree for that
I don't think we should be using |
I revert
Is this effect we want? But I encountered some issue blocking me generating |
@ararslan I build this PR from cloning it into a new dir, but got following error
(this error msg is same as I'm on an old -CURRENT
|
No, notice that the
These system libraries link to |
@ararslan ha, permission problem diff --git a/contrib/fixup-freebsd-libs.sh b/contrib/fixup-freebsd-libs.sh
old mode 100644
new mode 100755 |
now my build stopped here
|
Are you using |
No, it's a clean clone.
└─[iblis@abeing]% g df |cat
diff --git a/contrib/fixup-freebsd-libs.sh b/contrib/fixup-freebsd-libs.sh
old mode 100644
new mode 100755
diff --git a/deps/Makefile b/deps/Makefile
index 4e153791bd..34b0326b16 100644
--- a/deps/Makefile
+++ b/deps/Makefile
@@ -168,6 +168,7 @@ default: | $(build_prefix)
ifeq ($(OS),FreeBSD)
ifneq ($(BUILD_CUSTOM_LIBCXX),1)
default: $(build_libdir)/libc++.so.1
+$(error ${USE_SYSTEM_PATCHELF})
ifeq ($(USE_SYSTEM_PATCHELF),1)
$(build_libdir)/libc++.so.1:
-$(JULIAHOME)/contrib/fixup-freebsd-libs.sh $(PATCHELF) $(build_libdir) $(GCCPATH)
┌─[~/tmp/jl2]
| [Venv(py36)] [-- INSERT --]
└─[iblis@abeing]% gmake
Makefile:171: *** 0. Stop.
gmake: *** [Makefile:85: julia-deps] Error 2 |
I was having that issue as well and I don't recall what it was I did to resolve it. Seems that patchelf expands the binary too much when inserting an RPATH into a library that doesn't already have one. |
Ah. Now I remember. It was building a custom libc++ and libc++abi (ref #21792) that fixed it for me, as doing so allows it to bypass the patchelf hack here. So I'm not sure where we should go from here, but these GCC changes are critical to get FreeBSD building out of the box. Edit: Perhaps building a custom libc++ should be the default on FreeBSD? |
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build. This commit works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done. With this change, all libraries which need to link to libgcc_s link to the proper version.
5094a1d
to
78305aa
Compare
Rather than trying to get patchelf to bite off more than it can chew, I've made it so that |
@ararslan now, it works fine! |
Fantastic! Thanks for trying it out, @iblis17. |
Now that #21792 has been merged, I rebuilt from this branch on FreeBSD after a |
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
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build. This commit works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done. With this change, all libraries which need to link to libgcc_s link to the proper version. Ref #21788 (cherry picked from commit 7d83d99)
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build. This commit works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done. With this change, all libraries which need to link to libgcc_s link to the proper version. Ref #21788 (cherry picked from commit 7d83d99)
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build. This commit works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done. With this change, all libraries which need to link to libgcc_s link to the proper version. Ref #21788 (cherry picked from commit 7d83d99)
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build. This commit works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done. With this change, all libraries which need to link to libgcc_s link to the proper version. Ref #21788 (cherry picked from commit 7d83d99)
FreeBSD's system libgcc_s declares its GCC version as 4.6, which is too old to build Julia. Since gfortran doesn't come installed by default, when it's installed it's done so through a GCC port, which includes its own libgcc_s. Linking to libgfortran from the port and to the system's libgcc_s causes versioning issues which wreck the build.
This PR works around this issue by determining the version of GCC used for gfortran, then linking in the libgcc_s corresponding to that version everywhere. It's a bit of a kludge, but it gets the job done.
With this change, all libraries which need to link to libgcc_s link to the proper version.