HADOOP-19375. Organize JDK version-specific code in IDEA friendly approach #7245
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pan3793
commented
Dec 24, 2024
| <id>jdk1.8</id> | ||
| <id>jdk8</id> | ||
| <activation> | ||
| <jdk>1.8</jdk> |
Member
Author
There was a problem hiding this comment.
Maven supports both 1.8 and 8, but seems IDEA only supports 1.8
Member
Author
|
cc @jojochuang |
|
💔 -1 overall
This message was automatically generated. |
Member
Author
|
The files complaint by Yetus are renamed without changes, I tend to keep them as-is to avoid introducing any unnecessary noisy changes. |
Member
Author
Member
Author
|
cc @slfan1989 @cnauroth, it's a minor change that affects developers who use IDEA with JDK 17, could you please take a look, thanks in advance |
Contributor
|
@pan3793 LGTM. I personally believe that this modification is feasible, but I still need @cnauroth and @steveloughran to review it for confirmation. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
cnauroth
approved these changes
Feb 8, 2025
Contributor
cnauroth
left a comment
cnauroth
left a comment
There was a problem hiding this comment.
+1
This looks fine to me. Thank you, @pan3793 !
Even though this is meant to help with Java 17 support, I wonder if we should merge it down to branch-3.4 and branch-3.3 anyway to reduce potential merge confliclts on cherry-picks.
I won't commit this one right away in case others want to comment.
Contributor
|
There are some test failures, but I think that's because we're in a partial state with the JUnit5 upgrade. The hadoop-hdfs test subclasses a base class from hadoop-common, where it's using the Jupiter annotations, but hadoop-hdfs doesn't have the new runner yet. |
Contributor
@cnauroth This seems to be caused by our upgrade of Hadoop-Common. I will continue to follow up on this issue. Once Hadoop Common Part4/Part5 is merged, I will submit Part6 and complete the unit tests for the modules dependent on Hadoop Common. I expect to work on this next week. cc: @pan3793 |
Contributor
|
into 3.4.x please; I'm always mixing java versions and things get really confused in the IDE. |
Contributor
|
I have committed this to trunk and branch-3.4. @pan3793 , thank you for the patch. @steveloughran , thank you for adding your feedback. |
Member
Author
|
Thanks for merging, it's able to run tests on IDEA with JDK 17 now, which simplifies the debugging process a lot to tackle UT failure after upgrading to JDK 17 |
adideshpande
pushed a commit
to adideshpande/hadoop
that referenced
this pull request
Feb 27, 2025
…roach Closes apache#7245 Reviewed-by: Steve Loughran <stevel@apache.org> Signed-off-by: Chris Nauroth <cnauroth@apache.org>
YanivKunda
pushed a commit
to YanivKunda/hadoop
that referenced
this pull request
Mar 23, 2025
…roach Closes apache#7245 Reviewed-by: Steve Loughran <stevel@apache.org> Signed-off-by: Chris Nauroth <cnauroth@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
The Hadoop codebase has some JDK version-specific code, and uses
maven-compiler-pluginto exclude the specific packages, while IDEA does not work well for this plugin. For example, when setting project JDK to Java 17, the IDEA still loads those excluded source code and complains of class reference issues.The PR proposes to use
build-helper-maven-pluginto organize JDK version-specific code, which gets better support from IDEA.BTW,
hadoop-commonalso uses the same way to organize the arch-specific code, insrc/main/arm-java.How was this patch tested?
When setting the project JDK to Java 8
When setting the project JDK to Java 17
Also, verified by running
mvn clean install -DskipTestsin both Java 8 and 17.For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?