8385123: Devkit for Linux x86_64 couldn't be built#31227
Conversation
|
👋 Welcome back ysuenaga! A progress list of the required criteria for merging this PR into |
|
@YaSuenag This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 97 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
| LINKER_CONFIG := --enable-gold=default | ||
| endif | ||
|
|
||
| # --enable-multilib would be specified to other than x86_64 |
There was a problem hiding this comment.
Hi, do we really need it on Linux ppc64le ? @TheRealMDoerr , @ArnoZeller do you know ?
There was a problem hiding this comment.
Probably not. I wonder which platforms really need it.
There was a problem hiding this comment.
Probably not. I wonder which platforms really need it.
Maybe 32/64 bit ARM ? Not sure about this.
There was a problem hiding this comment.
I could make devkit without --enable-multilib on AArch64, and it could be used with building JDK of course.
I'm not sure, but I guess we can remove it for all platforms (but I want to test on all platforms if we do, and I do not have all of them of course...)
|
I think this makes sense; just one question - after JEP503 , do we want to still be able to create devkits that are used for older OpenJDK releases? Or is this out of scope? |
I think it is out of scope because we can use devkit in its repository (e.g. |
There was a problem hiding this comment.
Does this needed after the 32bit x86 has been removed.
There was a problem hiding this comment.
Following error happened in GCC build log when I removed them:
/usr/src/jdk/build/devkit/result/x86_64-linux-gnu-to-x86_64-linux-gnu/x86_64-linux-gnu/bin/ld: error: cannot find libc.so.6
/usr/src/jdk/build/devkit/result/x86_64-linux-gnu-to-x86_64-linux-gnu/x86_64-linux-gnu/bin/ld: error: cannot find ld-linux-x86-64.so.2
collect2: error: ld returned 1 exit status
make[5]: *** [Makefile:1005: libgcc_s.so] Error 1
In Tools.gmk, rewrite to relative path in linker script of libc and libpthread to ensure to use libs in sysroot. I'm not sure, but it might attempts to load /usr/lib rather than /usr/lib64. I think it is incorrect because --with-sysroot would be passed when configure scripts for GCC is run. Thus it is not necessary.
We should apply following patch if we want to remove them. It works on my PC and JDK binaries which were built with devkit seem to be linked glibc in sysroot correctly. I think it implies this patch is correct. However I wonder I can merge it to this PR because it does not seem to be another problem.
diff --git a/make/devkit/Tools.gmk b/make/devkit/Tools.gmk
index 1eb0a702747..ffed8ffcb99 100644
--- a/make/devkit/Tools.gmk
+++ b/make/devkit/Tools.gmk
@@ -202,9 +202,6 @@ SRCDIR := $(OUTPUT_ROOT)/src
# Marker file for unpacking rpms
RPMS := $(SYSROOT)/rpms_unpacked
-# Need to patch libs that are linker scripts to use non-absolute paths
-LIBS := $(SYSROOT)/libs_patched
-
################################################################################
# Download RPMs
download-rpms:
@@ -263,12 +260,7 @@ $(foreach dep,$(DEPENDENCIES),$(eval $(call DownloadVerify,$(dep))))
# Unpack RPMS
RPM_ARCHS := $(ARCH) noarch
-ifeq ($(ARCH),x86_64)
- # Enable mixed mode.
- RPM_ARCHS += i386 i686
-else ifeq ($(ARCH),i686)
- RPM_ARCHS += i386
-else ifeq ($(ARCH), armhfp)
+ifeq ($(ARCH), armhfp)
RPM_ARCHS += armv7hl
endif
@@ -298,26 +290,6 @@ endef
$(foreach p,$(RPM_FILE_LIST),$(eval $(call unrpm,$(p))))
-################################################################################
-
-# Note: MUST create a <sysroot>/usr/lib even if not really needed.
-# gcc will use a path relative to it to resolve lib64. (x86_64).
-# we're creating multi-lib compiler with 32bit libc as well, so we should
-# have it anyway, but just to make sure...
-# Patch libc.so and libpthread.so to force linking against libraries in sysroot
-# and not the ones installed on the build machine.
-$(LIBS) : $(RPMS)
- @echo Patching libc and pthreads
- @(for f in `find $(SYSROOT) -name libc.so -o -name libpthread.so`; do \
- (cat $$f | sed -e 's|/usr/lib64/||g' \
- -e 's|/usr/lib/||g' \
- -e 's|/lib64/||g' \
- -e 's|/lib/||g' ) > $$f.tmp ; \
- mv $$f.tmp $$f ; \
- done)
- @mkdir -p $(SYSROOT)/usr/lib
- @touch $@
-
################################################################################
# Create links for ffi header files so that they become visible by default when using the
# devkit.
@@ -702,8 +673,7 @@ MISSING_LINKS += $(PREFIX)/lib/bfd-plugins/liblto_plugin.so
bfdlib : $(BFDLIB)
binutils : $(BINUTILS)
rpms : $(RPMS)
-libs : $(LIBS)
-sysroot : rpms libs
+sysroot : rpms
gcc : sysroot $(GCC) $(GCC_PATCHED)
gdb : $(GDB)
all : binutils gcc bfdlib $(PREFIX)/devkit.info $(MISSING_LINKS) $(SYSROOT_LINKS) \
@@ -712,4 +682,4 @@ all : binutils gcc bfdlib $(PREFIX)/devkit.info $(MISSING_LINKS) $(SYSROOT_LINKS
# this is only built for host. so separate.
ccache : $(CCACHE)
-.PHONY : gcc all binutils bfdlib link_libs rpms libs sysroot
+.PHONY : gcc all binutils bfdlib link_libs rpms sysrootThere was a problem hiding this comment.
Removed in new commit with a patch in above.
|
|
||
| # Create a TARGET bfd + libiberty only. | ||
| # Configure one or two times depending on mulitlib arch. | ||
| # If multilib, the second should be 32-bit, and we resolve |
There was a problem hiding this comment.
I'm not sure whether we can remove it so far.
I know you mentioned for the comment for BFD, but configure options for GCC, following architectures do not need to support 32 bit, in other words, other archs (e.g. aarch64) multilib might be enabled.
ifneq ($(filter riscv64 ppc64le s390x x86_64, $(ARCH)), )
# We only support 64-bit on these platforms anyway
CONFIG += --disable-multilib
endif
openjdk
Bot
changed the title
While I agree with this reasoning in principle, in practice it has been rather convenient for us to be able to just use devkits from mainline in update releases without the need to produce new ones, and consequently also not needed to maintain the devkit generator scripts in update release repositories. I don't have a clear opinion on this right now, I will need to discuss internally and get back. |
We agreed that dropping 32bit support in the devkit is the right way forward. |
Ok, so I keep this PR. |
I would prefer #31303 going in first and spend some more time getting this one right. There is no hurry or need for this to reach a particular JDK version, it just needs to end up in mainline eventually. |
|
I think the bug title needs to be updated to describe the actual action here, removing 32bit support for Linux devkits. |
5 participants
I tried to build devkit for Linux x86_64, but I couldn't with following message:
Similar issue has been reported in JDK-8373624. We can remove 32 bit support based on JEP 503: Remove the 32-bit x86 Port, it means "disabling multilib" in configure error.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31227/head:pull/31227$ git checkout pull/31227Update a local copy of the PR:
$ git checkout pull/31227$ git pull https://git.openjdk.org/jdk.git pull/31227/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31227View PR using the GUI difftool:
$ git pr show -t 31227Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31227.diff
Using Webrev
Link to Webrev Comment