Skip to content

TEZ-4387: use new doclet API in Java 9+ - #187

Closed
LA-Toth wants to merge 2 commits into
apache:masterfrom
LA-Toth:a/TEZ-4387
Closed

TEZ-4387: use new doclet API in Java 9+#187
LA-Toth wants to merge 2 commits into
apache:masterfrom
LA-Toth:a/TEZ-4387

Conversation

@LA-Toth

Copy link
Copy Markdown
Contributor

No description provided.

@tez-yetus

This comment was marked as outdated.

Comment threadtez-tools/pom.xml Outdated
<id>jdk9plus</id>
<activation>
<!-- JDK9 starts strong encapsulation of Java internals, JDK17 removes/hides those -->
<jdk>[9,9999)</jdk>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which version does this expression target?
if every version starting with "9", then a simply "9" should work
https://maven.apache.org/guides/introduction/introduction-to-profiles.html

The following configuration will trigger the profile when the JDK's version starts with "1.4" (eg. "1.4.0_08", "1.4.2_07", "1.4"):
<profiles>
<profile>
<activation>
<jdk>1.4</jdk>
</activation>
...
</profile>
</profiles>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a range of versions, based on https://maven.apache.org/enforcer/enforcer-rules/versionRanges.html I wanted to write for JDK version 9 and up:
<jdk>[9,)</jdk>

It also could be 17 and up as only 17 enforces the new API.

@abstractdog

Copy link
Copy Markdown
Contributor

left some comments
also, could you please make a simple validity check with javadocs on JDK17?

mvn javadoc:javadoc
#take a look at for example tez-api/target/site/apidocs/index.html


@Override
public String getName() {
return "Example";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this name used? is there a chance for a better name?
in case of no idea, I guess "Tez" would make do :)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's from the JDK sample code. I had to implement the method. Good catch, I totally forgot to update it for the current project.

@LA-Toth

Copy link
Copy Markdown
ContributorAuthor

It seems that in JDK 11 the documentation generation is quite buggy, which is a known issue and fixed in JDK 13.
SO link: https://stackoverflow.com/questions/62531431/exit-code-1-javadoc-error-the-code-being-documented-uses-modules-but
possible OpenJDK tickets: https://bugs.openjdk.java.net/browse/JDK-8212233
and https://bugs.openjdk.java.net/browse/JDK-8217177

With Java 17 the documentation was generated successfully and believable, so I go further with the JDK17-only version.

@LA-Toth
LA-Tothforce-pushed the a/TEZ-4387 branch 2 times, most recently from 24860be to cd507aaCompareFebruary 22, 2022 08:15
@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@abstractdog

Copy link
Copy Markdown
Contributor

@LA-Toth: is there any pending work on this PR I can help with?

@LA-Toth

Copy link
Copy Markdown
ContributorAuthor

I had only one problem, the tests are too strict and cannot handle properly my solution. So I always get -1 for something. I don't think it's an error in my PR, I can't do anything.

@abstractdog

Copy link
Copy Markdown
Contributor

okay thanks, let me take a look next week

@LA-Toth
LA-Tothforce-pushed the a/TEZ-4387 branch 2 times, most recently from e0a4723 to 618422dCompareFebruary 1, 2023 19:45
@tez-yetus

This comment was marked as outdated.

@abstractdog

Copy link
Copy Markdown
Contributor

@LA-Toth: is there a chance you can check the patch with JDK11? I tried your commits at #296 and I found that the new github action step fails for building it on JDK11
the goal here is to remain compatible with JDK8 + JDK11, I'm hoping it's possible with easy changes

@LA-Toth

Copy link
Copy Markdown
ContributorAuthor

@LA-Toth: is there a chance you can check the patch with JDK11? I tried your commits at #296 and I found that the new github action step fails for building it on JDK11 the goal here is to remain compatible with JDK8 + JDK11, I'm hoping it's possible with easy changes

It works with JDK 11, as I see, at least locally. I just missed the version bump in pom.xml files.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec27m 6sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+0 🆗mvndep5m 24sMaven dependency ordering for branch
+1 💚mvninstall7m 23smaster passed
+1 💚compile0m 58smaster passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1
+1 💚compile0m 55smaster passed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09
+1 💚checkstyle0m 43smaster passed
+1 💚javadoc0m 43smaster passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1
+1 💚javadoc0m 41smaster passed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09
+0 🆗spotbugs0m 33sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs1m 26smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
-1 ❌mvninstall0m 20stez-tools in the patch failed.
-1 ❌mvninstall0m 11stez-javadoc-tools in the patch failed.
-1 ❌mvninstall0m 11stez-javadoc-tools-base in the patch failed.
-1 ❌compile0m 24stez-tools in the patch failed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.
-1 ❌compile0m 12stez-javadoc-tools in the patch failed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.
-1 ❌javac0m 24stez-tools in the patch failed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.
-1 ❌javac0m 12stez-javadoc-tools in the patch failed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.
-1 ❌compile0m 20stez-tools in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-1 ❌compile0m 12stez-javadoc-tools in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-1 ❌compile0m 10stez-javadoc-tools-base in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-1 ❌javac0m 20stez-tools in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-1 ❌javac0m 12stez-javadoc-tools in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-1 ❌javac0m 10stez-javadoc-tools-base in the patch failed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09.
-0 ⚠️checkstyle0m 9sThe patch fails to run checkstyle in tez-tools
-0 ⚠️checkstyle0m 7sThe patch fails to run checkstyle in tez-javadoc-tools
-0 ⚠️checkstyle0m 8stez-tools/tez-javadoc-tools-base: The patch generated 47 new + 0 unchanged - 0 fixed = 47 total (was 0)
-0 ⚠️checkstyle0m 11stez-tools/tez-javadoc-tools-base-jdk8: The patch generated 190 new + 0 unchanged - 0 fixed = 190 total (was 0)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 7sThe patch has no ill-formed XML file.
+1 💚javadoc0m 34sthe patch passed with JDK Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1
+1 💚javadoc0m 34sthe patch passed with JDK Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09
-1 ❌findbugs0m 31stez-tools in the patch failed.
-1 ❌findbugs0m 12stez-javadoc-tools in the patch failed.
-1 ❌findbugs0m 10stez-javadoc-tools-base in the patch failed.
_ Other Tests _
-1 ❌unit2m 44stez-tools in the patch failed.
-1 ❌unit0m 11stez-javadoc-tools in the patch failed.
-1 ❌unit0m 11stez-javadoc-tools-base in the patch failed.
+1 💚unit0m 12stez-javadoc-tools-base-jdk8 in the patch passed.
+1 💚asflicense0m 30sThe patch does not generate ASF License warnings.
56m 46s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/Dockerfile
GITHUB PR#187
JIRA IssueTEZ-4387
Optional Testsdupname asflicense javac javadoc unit xml compile spotbugs findbugs checkstyle
unameLinux 5149fe41507f 4.15.0-212-generic #223-Ubuntu SMP Tue May 23 13:09:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / 154fd34
Default JavaPrivate Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u372-gaus1-0ubuntu122.04-b09
mvninstallhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-mvninstall-tez-tools.txt
mvninstallhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-mvninstall-tez-tools_tez-javadoc-tools.txt
mvninstallhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-mvninstall-tez-tools_tez-javadoc-tools-base.txt
compilehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools-jdkUbuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.txt
compilehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-jdkUbuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.txt
javachttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools-jdkUbuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.txt
javachttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-jdkUbuntu-11.0.19+7-post-Ubuntu-0ubuntu122.04.1.txt
compilehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
compilehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
compilehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-base-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
javachttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
javachttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
javachttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-compile-tez-tools_tez-javadoc-tools-base-jdkPrivateBuild-1.8.0_362-8u372-ga~us1-0ubuntu1~22.04-b09.txt
checkstylehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/buildtool-patch-checkstyle-tez-tools.txt
checkstylehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/buildtool-patch-checkstyle-tez-tools_tez-javadoc-tools.txt
checkstylehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/diff-checkstyle-tez-tools_tez-javadoc-tools-base.txt
checkstylehttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/diff-checkstyle-tez-tools_tez-javadoc-tools-base-jdk8.txt
findbugshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-findbugs-tez-tools.txt
findbugshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-findbugs-tez-tools_tez-javadoc-tools.txt
findbugshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-findbugs-tez-tools_tez-javadoc-tools-base.txt
unithttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-unit-tez-tools.txt
unithttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-unit-tez-tools_tez-javadoc-tools.txt
unithttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/artifact/out/patch-unit-tez-tools_tez-javadoc-tools-base.txt
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/testReport/
Max. process+thread count807 (vs. ulimit of 5500)
modulesC: tez-tools tez-tools/tez-javadoc-tools tez-tools/tez-javadoc-tools-base tez-tools/tez-javadoc-tools-base-jdk8 U: tez-tools
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-187/4/console
versionsgit=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@abstractdog

abstractdog commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

this is strange, I wanted to test by building locally, but I can see that in jdk8 the problematic module is not included at all:

cd tez-tools
mvn --batch-mode clean test-compile -DskipTests=true
...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for tez-tools 0.10.3-SNAPSHOT:
[INFO]
[INFO] tez-tools .......................................... SUCCESS [ 1.414 s]
[INFO] tez-perf-analyzer .................................. SUCCESS [ 0.126 s]
[INFO] tez-job-analyzer ................................... SUCCESS [ 8.361 s]
[INFO] tez-javadoc-tools .................................. SUCCESS [ 1.323 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

I expected tez-javadoc-tools-base-jdk8 to be included

mvn -version
Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)
Maven home: /Users/laszlobodor/Applications/maven
Java version: 1.8.0_292, vendor: AdoptOpenJDK, runtime: /Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk/Contents/Home/jre
Default locale: en_HU, platform encoding: UTF-8
OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac"

@abstractdog

Copy link
Copy Markdown
Contributor

currently working on this in the scope of #296

@abstractdog

Copy link
Copy Markdown
Contributor

merged as #296

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LA-Toth@tez-yetus@abstractdog