Skip to content
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

[Build] Support property: maven.compiler.release for building BK with higher version JDK #4090

Closed
wants to merge 3 commits into from

Conversation

Reidddddd
Copy link
Contributor

Motivation

With this feature, the compiler will detect and generate an error when using APIs that don't exist in previous releases of Java SE in advance.

Changes

Add property: maven.compiler.release

Master Issue: #4089

@Reidddddd
Copy link
Contributor Author

rerun failure checks

@Reidddddd
Copy link
Contributor Author

Reidddddd commented Sep 26, 2023

Failure is about maven-javadoc-plugin, but i didn't change it, seems to be not related?

@Reidddddd
Copy link
Contributor Author

Failure is about maven-javadoc-plugin, but i didn't change it, seems to be not related?

seems related, checking on local

@Reidddddd
Copy link
Contributor Author

Reidddddd commented Sep 28, 2023

I could reproduce the CI failure on local, it is all about groovy, it could be fixed by removing "-T 1C" which indicates: can't run in parallel, don't know why..

I also tried upgrading to the latest groovy 3.0.19 for 3, it also failed unless i remove "-T 1C"

any thought?

@Reidddddd
Copy link
Contributor Author

Let me explain more about this PR. (I'm using JDK 17 to build on local)

First of all, only with <maven.compiler.release>${release.target}</maven.compiler.release>, during PR Validation stage, it would fail as following

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.2.0:jar (attach-javadocs) on project bookkeeper-stats-api: MavenReportException: Error while generating Javadoc:
[ERROR] Exit code: 4 - Loading source files for package org.apache.bookkeeper.stats...
[ERROR] Constructing Javadoc information...
[ERROR] error: fatal error encountered: java.lang.AssertionError
[ERROR] error: Please file a bug against the javadoc tool via the Java bug reporting page
[ERROR]   (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com)
[ERROR]   for duplicates. Include error messages and the following diagnostic in your report. Thank you.
[ERROR] java.lang.AssertionError
[ERROR] 	at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:155)
[ERROR] 	at jdk.compiler/com.sun.tools.javac.util.Assert.checkNonNull(Assert.java:62)
[ERROR] 	at jdk.compiler/com.sun.tools.javac.comp.Modules.allModules(Modules.java:1216)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.findModuleOfPackageName(ElementsTable.java:857)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.addPackagesFromLocations(ElementsTable.java:537)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.computeSubpackages(ElementsTable.java:510)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.computeSpecifiedPackages(ElementsTable.java:747)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.analyze(ElementsTable.java:347)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.JavadocTool.getEnvironment(JavadocTool.java:220)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:546)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:393)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:342)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.Main.execute(Main.java:63)
[ERROR] 	at jdk.javadoc/jdk.javadoc.internal.tool.Main.main(Main.java:52)
[ERROR] 2 errors
[ERROR]
[ERROR] Command line was: /usr/local/Cellar/openjdk@17/17.0.7/libexec/openjdk.jdk/Contents/Home/bin/javadoc @options

@Reidddddd
Copy link
Contributor Author

Reidddddd commented Sep 28, 2023

:maven-javadoc-plugin

Then from this issue: https://bugs.openjdk.org/browse/JDK-8193030, the last comment gives me a hint.
That's why I removed all <subpackages> tag.

In the meantime, I checked the maven-javadoc-plugin, as tag <packages> in <group> does the same thing, so I was double confirmed it is safe to be removed.

@Reidddddd
Copy link
Contributor Author

Reidddddd commented Sep 28, 2023

:maven-javadoc-plugin

Then from this issue: https://bugs.openjdk.org/browse/JDK-8193030, the last comment gives me a hint. That's why I removed all tag.

In the meantime, I checked the maven-javadoc-plugin, as tag in does the same thing, so I was double confirmed it is safe to be removed.

After these two issues got fixed, here comes to the 3rd one:

[ERROR] Constructing Javadoc information...
[ERROR] /Users/reid.chen/Shopee/bookkeeper/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/conf/ConfigKeyGroup.java:59: error: cannot find symbol
[ERROR]     public static ConfigKeyGroupBuilder builder(String name) {
[ERROR]                   ^
[ERROR]   symbol:   class ConfigKeyGroupBuilder
[ERROR]   location: class ConfigKeyGroup
[ERROR] /Users/reid.chen/Shopee/bookkeeper/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/conf/ConfigKey.java:64: error: cannot find symbol
[ERROR]     public static ConfigKeyBuilder builder(String name) {
[ERROR]                   ^
[ERROR]   symbol:   class ConfigKeyBuilder
[ERROR]   location: class ConfigKey
[ERROR] 2 errors
[ERROR]
[ERROR] Command line was: /usr/local/Cellar/openjdk@17/17.0.7/libexec/openjdk.jdk/Contents/Home/bin/javadoc @options @argfile

As this answer suggests, that's why I added public static class XXXBuilder {}. And it passed

@Reidddddd
Copy link
Contributor Author

:maven-javadoc-plugin

Then from this issue: https://bugs.openjdk.org/browse/JDK-8193030, the last comment gives me a hint. That's why I removed all tag.
In the meantime, I checked the maven-javadoc-plugin, as tag in does the same thing, so I was double confirmed it is safe to be removed.

After these two issues got fixed, here comes to the 3rd one:

[ERROR] Constructing Javadoc information...
[ERROR] /Users/reid.chen/Shopee/bookkeeper/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/conf/ConfigKeyGroup.java:59: error: cannot find symbol
[ERROR]     public static ConfigKeyGroupBuilder builder(String name) {
[ERROR]                   ^
[ERROR]   symbol:   class ConfigKeyGroupBuilder
[ERROR]   location: class ConfigKeyGroup
[ERROR] /Users/reid.chen/Shopee/bookkeeper/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/conf/ConfigKey.java:64: error: cannot find symbol
[ERROR]     public static ConfigKeyBuilder builder(String name) {
[ERROR]                   ^
[ERROR]   symbol:   class ConfigKeyBuilder
[ERROR]   location: class ConfigKey
[ERROR] 2 errors
[ERROR]
[ERROR] Command line was: /usr/local/Cellar/openjdk@17/17.0.7/libexec/openjdk.jdk/Contents/Home/bin/javadoc @options @argfile

As this answer suggests, that's why I added public static class XXXBuilder {}. And it passed

Final issue is this comment

@Reidddddd
Copy link
Contributor Author

Reidddddd commented Sep 28, 2023

-T 1C

with "-T 1C" 100% failure
Screenshot 2023-09-28 at 18 18 53

without "-T 1C" 100% success
Screenshot 2023-09-28 at 18 13 56

shoothzj
shoothzj previously approved these changes Apr 16, 2024
@shoothzj shoothzj dismissed their stale review April 16, 2024 06:05

We are still using java8, we should keep the release.version to 8

Copy link
Member

@shoothzj shoothzj left a comment

Choose a reason for hiding this comment

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

@Reidddddd Thanks for your contribution. But It seems contains some javadoc、constructor changes? I suggest to split them. And we are still supporting java8, I think we should keep release.version to 8. WDYT?

@shoothzj
Copy link
Member

@Reidddddd Closed by no update, feel free to reopen it if you still want to work on it.

@shoothzj shoothzj closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants