RFR: Cleaner GHA build of hermetic-java-runtime branch
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms. Thoughts? ------------- Commit messages: - Cleaner GHA build of hermetic-java-runtime branch Changes: https://git.openjdk.org/leyden/pull/63/files Webrev: https://webrevs.openjdk.org/?repo=leyden&pr=63&range=00 Stats: 17 lines in 4 files changed: 7 ins; 4 del; 6 mod Patch: https://git.openjdk.org/leyden/pull/63.diff Fetch: git fetch https://git.openjdk.org/leyden.git pull/63/head:pull/63 PR: https://git.openjdk.org/leyden/pull/63
On Mon, 28 Apr 2025 13:04:49 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
src/hotspot/share/utilities/globalDefinitions.hpp line 1031:
1029: // Redefine Thread as BaseThread to avoid duplicate symbol issues for 1030: // JDK static builds. See runtime/thread.hpp for Thread class. 1031: #define Thread HotspotBaseThread
This change and the one in src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S are related to https://bugs.openjdk.org/browse/JDK-8311846. There were discussions on https://github.com/openjdk/jdk/pull/17456 (closed without integrating), but we don't yet have a acceptable solution moving forward. Should we leave the patch in the branch until we identify a proper solution? src/java.base/share/native/launcher/main.c line 40:
38: 39: // Windows doesn't have PATH_MAX. It's MAX_PATH instead. 40: #ifdef _WIN32
Thanks. Checking https://learn.microsoft.com/en-us/cpp/c-language/cpp-integer-limits?view=msv..., PATH_MAX is not defined for Windows limits.h. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2064348725 PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2064381059
On Mon, 28 Apr 2025 19:08:42 GMT, Jiangli Zhou <jiangli@openjdk.org> wrote:
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
src/hotspot/share/utilities/globalDefinitions.hpp line 1031:
1029: // Redefine Thread as BaseThread to avoid duplicate symbol issues for 1030: // JDK static builds. See runtime/thread.hpp for Thread class. 1031: #define Thread HotspotBaseThread
This change and the one in src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S are related to https://bugs.openjdk.org/browse/JDK-8311846. There were discussions on https://github.com/openjdk/jdk/pull/17456 (closed without integrating), but we don't yet have a acceptable solution moving forward.
Should we leave the patch in the branch until we identify a proper solution?
Is this duplicate symbol issue still a problem? If so, how would I be able to reproduce? This breaks cross compiled code. For example see: https://github.com/jerboaa/leyden/actions/runs/14613421232/job/40996157052 ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2065775137
On Tue, 29 Apr 2025 08:24:01 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
Is this duplicate symbol issue still a problem? If so, how would I be able to reproduce? This breaks cross compiled code. For example see: https://github.com/jerboaa/leyden/actions/runs/14613421232/job/40996157052
The `Thread` duplicate symbol issue exists when statically linking with any application native code and native dependencies with a symbol also defined as `Thread`. We ran into the issue when prototyping on JDK 11. In file included from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.hpp:29, from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:25: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp: In destructor ‘AOTClassFilter::FilterMark::~FilterMark()’: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:42:53: error: comparison between distinct pointer types ‘Thread*’ and ‘HotspotBaseThread*’ lacks a cast 42 | assert(_current_mark == this && _filtering_thread == Thread::current(), "sanity"); Ok, we also ran into aotClassFilter.* related build failure when syncing with JDK mainline internally in early April. I've resolved the build issue in our code base. I'll push an update to hermetic-java-runtime branch to address this build issue shortly. Thanks for noticing it. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2066880516
On Tue, 29 Apr 2025 15:56:13 GMT, Jiangli Zhou <jiangli@openjdk.org> wrote:
Is this duplicate symbol issue still a problem? If so, how would I be able to reproduce? This breaks cross compiled code. For example see: https://github.com/jerboaa/leyden/actions/runs/14613421232/job/40996157052
Is this duplicate symbol issue still a problem? If so, how would I be able to reproduce? This breaks cross compiled code. For example see: https://github.com/jerboaa/leyden/actions/runs/14613421232/job/40996157052
The `Thread` duplicate symbol issue exists when statically linking with any application native code and native dependencies with a symbol also defined as `Thread`. We ran into the issue when prototyping on JDK 11.
In file included from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.hpp:29, from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:25: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp: In destructor ‘AOTClassFilter::FilterMark::~FilterMark()’: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:42:53: error: comparison between distinct pointer types ‘Thread*’ and ‘HotspotBaseThread*’ lacks a cast 42 | assert(_current_mark == this && _filtering_thread == Thread::current(), "sanity");
Ok, we also ran into aotClassFilter.* related build failure when syncing with JDK mainline internally in early April. I've resolved the build issue in our code base. I'll push an update to hermetic-java-runtime branch to address this build issue shortly. Thanks for noticing it.
I pushed https://git.openjdk.org/leyden/commit/c93d26d944b65b955a78c4227776bb0fc1d723.... ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2066976096
On Tue, 29 Apr 2025 16:58:12 GMT, Jiangli Zhou <jiangli@openjdk.org> wrote:
Is this duplicate symbol issue still a problem? If so, how would I be able to reproduce? This breaks cross compiled code. For example see: https://github.com/jerboaa/leyden/actions/runs/14613421232/job/40996157052
The `Thread` duplicate symbol issue exists when statically linking with any application native code and native dependencies with a symbol also defined as `Thread`. We ran into the issue when prototyping on JDK 11.
In file included from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.hpp:29, from /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:25: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp: In destructor ‘AOTClassFilter::FilterMark::~FilterMark()’: /home/runner/work/leyden/leyden/src/hotspot/share/cds/aotClassFilter.cpp:42:53: error: comparison between distinct pointer types ‘Thread*’ and ‘HotspotBaseThread*’ lacks a cast 42 | assert(_current_mark == this && _filtering_thread == Thread::current(), "sanity");
Ok, we also ran into aotClassFilter.* related build failure when syncing with JDK mainline internally in early April. I've resolved the build issue in our code base. I'll push an update to hermetic-java-runtime branch to address this build issue shortly. Thanks for noticing it.
I pushed https://git.openjdk.org/leyden/commit/c93d26d944b65b955a78c4227776bb0fc1d723....
OK. I've merged in the latest updates and reverted the change in `src/hotspot/share/utilities/globalDefinitions.hpp`. Let's see how GHA looks. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2068289224
On Wed, 30 Apr 2025 09:31:55 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
I pushed https://git.openjdk.org/leyden/commit/c93d26d944b65b955a78c4227776bb0fc1d723....
OK. I've merged in the latest updates and reverted the change in `src/hotspot/share/utilities/globalDefinitions.hpp`. Let's see how GHA looks.
@jerboaa The long-term plan is to handle this issue by using the linker to turn non-exported public symbols into private symbols before generating static libraries. This works on linux and macos, but I have not yet gotten it to work properly on Windows, and there is only a proof-of-concept sketch on how to get it to work on AIX. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2075234683
On Tue, 6 May 2025 11:07:08 GMT, Magnus Ihse Bursie <ihse@openjdk.org> wrote:
OK. I've merged in the latest updates and reverted the change in `src/hotspot/share/utilities/globalDefinitions.hpp`. Let's see how GHA looks.
@jerboaa The long-term plan is to handle this issue by using the linker to turn non-exported public symbols into private symbols before generating static libraries. This works on linux and macos, but I have not yet gotten it to work properly on Windows, and there is only a proof-of-concept sketch on how to get it to work on AIX.
OK. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2075586372
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Revert globalDefinitions.hpp change - Merge branch 'hermetic-java-runtime' into fix_build_issues_hermetic - Cleaner GHA build of hermetic-java-runtime branch ------------- Changes: - all: https://git.openjdk.org/leyden/pull/63/files - new: https://git.openjdk.org/leyden/pull/63/files/9488897d..61732324 Webrevs: - full: https://webrevs.openjdk.org/?repo=leyden&pr=63&range=01 - incr: https://webrevs.openjdk.org/?repo=leyden&pr=63&range=00-01 Stats: 43376 lines in 1236 files changed: 32431 ins; 7030 del; 3915 mod Patch: https://git.openjdk.org/leyden/pull/63.diff Fetch: git fetch https://git.openjdk.org/leyden.git pull/63/head:pull/63 PR: https://git.openjdk.org/leyden/pull/63
On Wed, 30 Apr 2025 09:35:35 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
- Revert globalDefinitions.hpp change - Merge branch 'hermetic-java-runtime' into fix_build_issues_hermetic - Cleaner GHA build of hermetic-java-runtime branch
src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S line 1:
1: // Copyright (c) 2015, 2022, Red Hat Inc. All rights reserved.
The src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S also needs to be reverted. It causes the linux-cross-build aarch64 build failures: ... /usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/ld: /home/runner/work/leyden/leyden/src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S:38: undefined reference to `Thread::_thr_current' collect2: error: ld returned 1 exit status * For target hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_run_ld: /usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/ld: /home/runner/work/leyden/leyden/build/linux-aarch64/hotspot/variant-server/libjvm/objs/threadLS_linux_aarch64.o: in function `JavaThread::aarch64_get_thread_helper()': /home/runner/work/leyden/leyden/src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S:34: undefined reference to `Thread::_thr_current' ... ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2068869527
On Wed, 30 Apr 2025 15:02:53 GMT, Jiangli Zhou <jiangli@openjdk.org> wrote:
Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
- Revert globalDefinitions.hpp change - Merge branch 'hermetic-java-runtime' into fix_build_issues_hermetic - Cleaner GHA build of hermetic-java-runtime branch
src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S line 1:
1: // Copyright (c) 2015, 2022, Red Hat Inc. All rights reserved.
The src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S also needs to be reverted. It causes the linux-cross-build aarch64 build failures:
... /usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/ld: /home/runner/work/leyden/leyden/src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S:38: undefined reference to `Thread::_thr_current' collect2: error: ld returned 1 exit status * For target hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_run_ld: /usr/lib/gcc-cross/aarch64-linux-gnu/10/../../../../aarch64-linux-gnu/bin/ld: /home/runner/work/leyden/leyden/build/linux-aarch64/hotspot/variant-server/libjvm/objs/threadLS_linux_aarch64.o: in function `JavaThread::aarch64_get_thread_helper()': /home/runner/work/leyden/leyden/src/hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.S:34: undefined reference to `Thread::_thr_current' ...
Done. ------------- PR Review Comment: https://git.openjdk.org/leyden/pull/63#discussion_r2069127406
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: Revert HotspotBaseThread in aarch assembly file ------------- Changes: - all: https://git.openjdk.org/leyden/pull/63/files - new: https://git.openjdk.org/leyden/pull/63/files/61732324..9027c73f Webrevs: - full: https://webrevs.openjdk.org/?repo=leyden&pr=63&range=02 - incr: https://webrevs.openjdk.org/?repo=leyden&pr=63&range=01-02 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/leyden/pull/63.diff Fetch: git fetch https://git.openjdk.org/leyden.git pull/63/head:pull/63 PR: https://git.openjdk.org/leyden/pull/63
On Wed, 30 Apr 2025 17:12:25 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision:
Revert HotspotBaseThread in aarch assembly file
Marked as reviewed by jiangli (Committer). ------------- PR Review: https://git.openjdk.org/leyden/pull/63#pullrequestreview-2808170667
On Mon, 28 Apr 2025 13:04:49 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:
Please review this simple PR which fixes build issues seen on GHA. Before this PR we see build failures in various Linux cross-builds and windows builds. This PR should fix those. Note: There are still GHA test failures, but at least we can build the branch on the main 3 platforms.
Thoughts?
This pull request has now been integrated. Changeset: 5f2b29cf Author: Severin Gehwolf <sgehwolf@openjdk.org> URL: https://git.openjdk.org/leyden/commit/5f2b29cff877efe845bd077af405f1f45f9bd7... Stats: 9 lines in 2 files changed: 7 ins; 0 del; 2 mod Cleaner GHA build of hermetic-java-runtime branch Reviewed-by: jiangli ------------- PR: https://git.openjdk.org/leyden/pull/63
participants (3)
-
Jiangli Zhou
-
Magnus Ihse Bursie
-
Severin Gehwolf