HADOOP-18224. Upgrade maven compiler plugin to 3.10.1 #4267
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@aajisaka It seems that newer versions of maven compiler and javadoc plugins are flagging issues with Javadoc. Shall we fix them later? Do you have any recommendations here? |
|
💔 -1 overall
This message was automatically generated. |
aajisaka
approved these changes
May 7, 2022
aajisaka
requested changes
May 7, 2022
There was a problem hiding this comment.
Sorry, I've changed my mind. The javadoc error should be fixed at least in java 8.
[ERROR] /home/jenkins/jenkins-home/workspace/hadoop-multibranch_PR-4267/ubuntu-focal/src/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/api/records/Component.java:120: error: invalid use of @return
[ERROR] * @return restartPolicy
Java 11 specific error can be fixed after the merge.
There are ~270 warnings for hadoop-yarn-services-core module so far. It's fine doing it as part of this PR but even after resolving the ~270 warnings, we still have some modules left, we might see more failures. |
|
There is only 1 error and all the rest are warning. You can double check by grepping |
|
Sorry maybe I am mistaken but are you also referring to https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4267/6/artifact/out/patch-javadoc-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt ? |
I see, sure let me fix that. |
virajjasani
force-pushed
the
HADOOP-18224-trunk
branch
from
1e14bfd to
9d5181c
Compare
virajjasani
changed the title
|
#4292 should help here |
|
💔 -1 overall
This message was automatically generated. |
Thanks @steveloughran
@aajisaka The above error is resolved and now JDK 8 Javadoc has 65 |
|
💔 -1 overall
This message was automatically generated. |
@aajisaka would you recommend resolving these errors? We still have many Yarn modules pending the evaluation once catalog webapp is resolved, so it's still unclear how many more are yet to go (after resolving 65 errors for catalog webapp), but if you suggest resolving them, will be happy to do so. |
|
@virajjasani Hi, I should be able to solve some YARN errors, some of which may be related to HADOOP-18167, and I am also solving some compilation problems of related Java Docs. |
Thanks @slfan1989, it seems you have addressed many Javadoc issues since the last time I saw the PR. I think it would be good to decide the order for both PRs #4267 (current) and #4292. If 4267 goes first, 4292 can resolve issues with updated Javadoc plugin. Anyways, it's all upto you @aajisaka ! |
Hi, @virajjasani the issues related to HADOOP-18167 have been fixed By HADOOP-18222, I think you can re-run Junit Test to see the results of the yarn module. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Umm. We are facing more javadoc errors after upgrading the plugin. Maybe the tag check become more strict. We can fix them in separate issues. |
Hi, @aajisaka I am fixing the java doc problem of hadoop-common, it is expected to be fixed in a few days (2-3days) |
@slfan1989 Both PRs (#4292 and current PR #4267) have different purpose. For this PR, we are bumping maven compiler and javadoc plugin to avoid pulling-in vulnerable log4j dependencies (also, these plugin versions that we are using are almost a decade old). As part of javadoc plugin upgrade, we are seeing new javadoc errors, whereas on PR #4292, several existing Javadoc errors are being being resolved, which is great. But once this PR gets in, we would see few more additional errors for both Java 8 and 11 builds.
I agree with @aajisaka that these new errors should be fixed in separate Jiras. And also the fact that tag checks have become stricter and are resulting in new Javadoc errors. If this PR gets merged first and then if we retrigger Jenkins build on PR #4292, we would see new errors. |
how about we get #4292 in, then this javadoc plugin update, with the fixes for new failures that get triggered? |
That was the first revision on this PR but when I saw Javadoc failures for Java 11, I thought of upgrading javadoc plugin as well and now it has resulted in Javadoc failures for Java 8 as well. No worries, let me keep this PR back to the initial purpose of upgrading compiler plugin only.
That is also fine. |
virajjasani
changed the title
|
Javadoc plugin upgrade can be done in a separate Jira. |
|
package-info.class exclusion on this PR is required for compiler plugin upgrade. |
|
@steveloughran @virajjasani @aajisaka I have basically completed the compilation and repair of javadoc in Jdk 11, and it can also be compiled in Jdk1.8 without error. (#4292). In JDK11(After Merge PR) In JDK8(After Merge PR) |
|
💔 -1 overall
This message was automatically generated. |
There are multiple different package-info.java using the same package. How about removing some the files and make sure there is only one package-info.java for each package? |
This sounds better than excluding it. Thanks for the suggestion. I just tried this and the build looks good locally. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Recent 3 QA results from Jenkins have #4292 included. (Thanks for the nice cleanup @slfan1989) |
Hi, @virajjasani , I am very happy to be able to do something for the hadoop project. I see some compilation problems with javac, and I will try to solve it as much as possible, but because there are different types, multiple PRs need to be submitted. |
Thanks a lot @slfan1989 !! @aajisaka @steveloughran based on the latest full build results, we are good to go with this PR? |
|
Merged. Thank you @virajjasani for your contribution and thanks @slfan1989 for cleanup the errors. |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
Signed-off-by: Akira Ajisaka <[email protected]>
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
Currently we are using maven-compiler-plugin 3.1 version, which is quite old (2013) and it's also pulling in vulnerable log4j dependency:
We should upgrade to 3.10.1 (latest Mar, 2022) version of maven-compiler-plugin.