HBASE-29225 [hbase-thirdparty] Add module for Jetty 12 with EE8 and add support for toolchain based JDK selection #142
Conversation
…che#131) This change adds a new set of modules which we will need for Jetty 12 migration. The code has been modularised into following modules: - `hbase-shaded-jetty-12-plus-core`: Contains shaded jetty 12 core jars - `hbase-shaded-jetty-12-plus-ee8`: Contains shaded jetty EE8 specific jars So basically: - Branches which want to consume EE8 may need to add `hbase-shaded-jetty-12-plus-core` and `hbase-shaded-jetty-12-plus-ee8` in their dependency replacing the former `hbase-shaded-jetty` - Branches which want to consume EE9/EE10 may need to add a new module for same in `hbase-thirdparty` in future Signed-off-by: Istvan Toth <stoty@apache.org>
…modules Implement Maven toolchains-based build system to support both JDK 8 and JDK 17 compilation in a single release with configurable setup: - Add toolchains plugin configuration for automatic JDK selection - Configure Jetty 12 modules (hbase-shaded-jetty-12-plus-*) to use JDK 17 - Maintain JDK 8 compatibility for existing modules - Split maven-enforcer-plugin into separate JDK 8/17 bytecode checks - Add generate-toolchains.sh script for dynamic toolchains.xml generation - Support JAVA8_HOME/JAVA17_HOME environment variable configuration - Auto-detect Java installations at /usr/lib/jvm/java-8 and /usr/lib/jvm/java-17 for CI - Force explicit environment variable setup for local development - Remove dependency on ~/.m2/toolchains.xml for project-specific configuration - Add documentation with setup commands and CI/local instructions This allows HBase 2.x to continue using JDK 8 compatible Jetty 9 modules while HBase 3.x can leverage JDK 17 Jetty 12 modules, with seamless Jenkins/CI builds and clear local development setup, eliminating the need for separate branches or releases.
|
Adds back reverted commit 6016e75 as is in first commit. |
|
Hi reviewers, requesting you for early feedback here, before I move ahead with Jenkins integration. |
This comment was marked as outdated.
This comment was marked as outdated.
Apache9
left a comment
Apache9
left a comment
There was a problem hiding this comment.
We only need to use JDK8 for compiling hbase-unsafe, for other modules, it is OK to use JDK17 to compile and set --release to 8.
Maybe we could set the default JDK version to 17, and use JDK8 for hbase-unsafe only.
Thank you Duo for the feedback, sounds good to me. Let me update PR with the suggestion! |
This comment was marked as outdated.
This comment was marked as outdated.
… toolchains.xml and copy to ~/.m2
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
pom.xml
Outdated
| <configuration> | ||
| <rules> | ||
| <enforceBytecodeVersion> | ||
| <maxJdkVersion>${java.min.version}</maxJdkVersion> |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
XML not sure what is the problem. |
|
(!) A patch to the testing environment has been detected. |
Javac warnings are expected, not sure why we are seeing now. Was it not coming before w/o this change? Maybe because build fails for branch and so the diff? |
|
💔 -1 overall
This message was automatically generated. |
| <source>1.8</source> | ||
| <target>1.8</target> | ||
| </configuration> | ||
| <version>3.13.0</version> |
There was a problem hiding this comment.
Then do we still need to set maven.compiler.source and maven.compiler.target?
| <name>Apache HBase Patched and Relocated (Shaded) Protobuf</name> | ||
| <description>Pulls down protobuf, patches it, compiles, and then relocates/shades.</description> | ||
| <properties> | ||
| <!-- Override parent's JDK 17 settings to force JDK 8 for hbase-shaded-protobuf. |
There was a problem hiding this comment.
So protobuf also uses Unsafe? What is the compile error you see?
| <compileSource>1.8</compileSource> | ||
| <java.min.version>${compileSource}</java.min.version> | ||
| <!-- Project level compilation properties --> | ||
| <hbase.unsafe.and.protobuf.java.version>1.8</hbase.unsafe.and.protobuf.java.version> |
There was a problem hiding this comment.
Question: any better name suggestion?
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
What is our yetus version? In java 17 we do not have js script engine, IIRC we switch to use xmllint to check xml format in newer version of yetus. |
|
And the javac output are all warnings, not sure why yetus report it as error... |
Apache9
left a comment
Apache9
left a comment
There was a problem hiding this comment.
I'm OK with the current approach.
I think the failures can be fixed after upgrading yetus. Can be a separated issue.
Thanks @NihalJain for the hard work!
|
Thank you Duo and David for the approvals, will wait another day and merge this tomorrow. |
NihalJain
changed the title
|
|
||
| # Script to generate toolchains.xml with configurable Java paths | ||
|
|
||
| # Set default paths if environment variables are not set |
There was a problem hiding this comment.
This is great.
The release scripts should also be amended to to call this when releasing hbase-thirdparty
(assuming that the release script supports hbase-thirdparty)
There was a problem hiding this comment.
Hi @stoty yes you are absolutely right, release script would need some patching to understanding toolchains. Will take this up as part of release process. Thanks for highlighting.
5 participants
Part 1
First, we add a new set of modules which we will need for Jetty 12 migration. The code has been modularised into following modules:
hbase-shaded-jetty-12-plus-corehbase-shaded-jetty-12-plus-ee8So basically:
hbase-shaded-jetty-12-plus-coreandhbase-shaded-jetty-12-plus-ee8in their dependency replacing the formerhbase-shaded-jettyhbase-thirdpartyin futurePart 2
Implement a maven toolchains-based build system to support both JDK 8 and JDK 17 compilation in a single branch with configurable setup:
hbase-unsafeandhbase-shaded-protobufmodulesmaven-enforcer-plugininto separate profiles for JDK 8/JDK 17 bytecode validationgenerate-toolchains.shscript for dynamic toolchains.xml generation with environment variable supportJAVA8_HOME/JAVA17_HOMEenvironment variable configuration for local development/usr/lib/jvm/java-8and/usr/lib/jvm/java-17(CI/Jenkins paths)Part 3:
Fix Jenkins build failures by implementing Docker-based multi-JDK environment with automated toolchain management:
-t dev-support/toolchains-jenkins.xmlto all mvn invocationsdev-support/toolchains-jenkins.xmlwith fixed JDK paths for Jenkins environmentThis resolves Jenkins build failures introduced by the toolchains-based build system (Part 2) by providing the required dual JDK environment and automatic toolchain configuration. Jenkins builds now work seamlessly with:
Completes the three-part implementation enabling HBase 2.x (JDK 8 + Jetty 9) and HBase 3.x (JDK 17 + Jetty 12) support in a single branch.