-
-
Notifications
You must be signed in to change notification settings - Fork 235
feat: Add support on Linux for JDK 25 in Docker build configurations #994
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
base: master
Are you sure you want to change the base?
feat: Add support on Linux for JDK 25 in Docker build configurations #994
Conversation
…proved compatibility
…mproved compatibility
…t-add-java-25-target-for-all-images
…t-add-java-25-target-for-all-images
…t-add-java-25-target-for-all-images
Windows image builds are failing because of |
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.
Nice!
Some suggestions below
# It is acceptable to have a larger image in arm/v7 (arm 32 bits) environment. | ||
# Because jlink fails with the error "jmods: Value too large for defined data type" error. |
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.
Question: this isn't the case anymore?
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.
Question: this isn't the case anymore?
This question may have been missed, still open.
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.
Are you referring to the error linked to the --version
option?
I haven't encountered that in years, even on arm32
.
However, I might have misunderstood your question. 🤔
If you're referring to the difference for arm/v7
, the builds succeed, so I'm fairly confident that's no longer an issue. I'm currently running this command on my small arm32
test machine to see if it fails.
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.
The jlink
command ran fine, except that since my machine has limited memory, I had to tweak the options to allocate more heap space.
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 asking if the comment is actually outdated.
If it's the case deleting it is OK, if you don't know I would keep it.
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 discussion is unrelated to the current PR (unless I miss the link between JDK25 and arm?).
The goal of the PR is to add JDK25 support, not to clean up code.
It's legit to raise an issue and/or a PR dedicated to this cleanup though, but increasing the scope of the current PR with this change and associated discussion only delays its delivery, drown the value of the discussion and makes review harder
Clarified usage of the `-ImageType` option by providing separate examples for `nanoserver` and `windowsservercore`. This improves user understanding of build command usage for different Windows image types.
Replaced nested conditional logic with a cleaner `lookup` function using a newly introduced `jdk_versions` variable. This improves code readability and maintainability while ensuring consistency in handling JDK version mappings.
Co-authored-by: lemeurherveCB <[email protected]>
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!
Still have two unanswered questions though 🙂
# It is acceptable to have a larger image in arm/v7 (arm 32 bits) environment. | ||
# Because jlink fails with the error "jmods: Value too large for defined data type" error. |
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.
Question: this isn't the case anymore?
This question may have been missed, still open.
Introduced a memory constraint (-J-Xmx512M) for the ARMv7 architecture during the jlink process to prevent resource issues. Removed redundant conditionals, simplifying the Dockerfile while maintaining runtime compatibility and reducing image size.
Note: could this PR be squashed on merge? |
fix: Improve JDK version handling in javaversion function fix: Update supported platforms for JDK 25 in debian_platforms function fix: Remove JDK 25 from linux-arm32 targets in Docker bake configuration fix: Update default value for JAVA25_VERSION to "25+9-ea-beta" fix: Enhance jlink options for JDK 17, 21, and 25 in Dockerfile fix: Update jlink options for JDK 25 in Dockerfiles for various platforms fix: Refactor javaversion function to simplify JDK version lookup fix: Add module-path option to jlink command in Dockerfile fix: Update jlink command options for better compatibility with Java versions fix(rhel/ubi9): add java 25 target fix: Add support for Java 25 in alpine_platforms function fix: Simplify target architecture selection for JDK versions in docker-bake.hcl fix: Clean up Dockerfile by removing unnecessary JDK installation checks fix: Update Dockerfile to support Java 25 and improve JAVA_HOME configuration fix: Update Dockerfile to dynamically set JAVA_HOME based on JAVA_VERSION fix: Update Dockerfile to improve error handling for unsupported Java versions fix: Update Dockerfile to improve error handling for unsupported Java versions fix: Update Dockerfile to correctly extract major Java version for improved compatibility fix: Update Dockerfile to support dynamic Java version handling for improved compatibility fix: Update Dockerfile to support Java 25 target and improve version handling fix: Update Dockerfile to enhance jlink options for Java 17, 21, and 25 docs: Remove outdated instructions for building inbound agent with JDK 25 fix(Dockerfile): Simplify JAVA_HOME setup and update PATH configuration fix(Dockerfile): Add support for Java 25 and improve JAVA_HOME setup fix(Dockerfile): Separate ENV declarations for clarity and update PATH configuration fix(Dockerfile): Update PATH to use PSHOME instead of hardcoded value for PowerShell Cherry-pick latest changes for Windows Dockerfiles from upstream/master fix(docker-bake.hcl): Update JDK variable for Windows builds to not include Java 25 chore: Update examples in README for image type build commands Clarified usage of the `-ImageType` option by providing separate examples for `nanoserver` and `windowsservercore`. This improves user understanding of build command usage for different Windows image types. refactor: JDK version lookup in docker-bake.hcl Replaced nested conditional logic with a cleaner `lookup` function using a newly introduced `jdk_versions` variable. This improves code readability and maintainability while ensuring consistency in handling JDK version mappings.
Co-authored-by: lemeurherveCB <[email protected]> chore: fix the aggressive "notice" factorization
Introduced a memory constraint (-J-Xmx512M) for the ARMv7 architecture during the jlink process to prevent resource issues. Removed redundant conditionals, simplifying the Dockerfile while maintaining runtime compatibility and reducing image size.
…https://github.com/gounthar/docker-agent into 15-feature-request-add-java-25-target-for-all-images
This could fix #993.
Testing done
make test
and.\build.ps1
.Submitter checklist