Force to build on JDK 11+ #23550

zhfeng · 2023-01-13T15:36:45Z

It sould be helpful to fix #19990 and #20935

Changes proposed in this pull request:

Force to use JDK 11 above to build shardingsphere
We still run all the e2e tests on JDK 8 to make sure there is no compatitable issues.
Update antlr4 to 4.10+ #19990 which needs to run antlr-maven-plugin to generate the parser classes. The antlr Tool are only for JDK 11 after upgrading to 4.10.1. So if upgrading antlr, it will force user to build shardingsphere with JDK 11

It seems that we need to support JDK 8 for a long time. There is no such plan to drop it, see the relevant discussion https://lists.apache.org/thread/kgtfx5vnw73o61lkvdshyfvjfvvzk3js

Before committing this PR, I'm sure that I have checked the following options:

My code follows the code of conduct of this project.
I have self-reviewed the commit code.
I have (or in comment I request) added corresponding labels for the pull request.
I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
I have made corresponding changes to the documentation.
I have added corresponding unit tests for my changes.

linghengqian

I checked CI.

Failed to execute goal org.codehaus.mojo:cobertura-maven-plugin:2.7:instrument (default-cli) on project shardingsphere: Execution default-cli of goal org.codehaus.mojo:cobertura-maven-plugin:2.7:instrument failed : Plugin org.codehaus.mojo:cobertura-maven-plugin:2.7 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:0 at specified path /usr/lib/jvm/temurin- 11-jdk-amd64/../lib/tools.jar -> [Help 1]

It looks like tools.jar is being called somewhere, which is a unique built-in JAR on JDK 8.

zhfeng · 2023-01-13T15:43:23Z

Yeah, I'm taking a look at it.

zhfeng · 2023-01-13T15:57:34Z

unfortunately it seem that cobertura-maven-plugin is not compatible with JDK 11 yet. see

Update cobertura-maven plugin to Java 11 mojohaus/cobertura-maven-plugin#43

linghengqian · 2023-01-13T16:25:59Z

I'm a bit confused after looking at http://cobertura.github.io/cobertura/, shouldn't there be a more modern maven plugin for uploading to codecov? This even goes back to 2017, e727b71. It looks like @haocao has withdrawn from the github, maybe it can be replaced by jacoco-maven-plugin? Like https://github.com/codecov/example-java-maven/blob/main/pom.xml .
I wouldn't be surprised if this warrants opening another issue to discuss.

codecov-commenter · 2023-01-13T16:58:23Z

Codecov Report

Merging #23550 (66b761a) into master (f6ad3b2) will increase coverage by 0.47%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #23550      +/-   ##
============================================
+ Coverage     49.75%   50.22%   +0.47%     
+ Complexity     2510     1610     -900     
============================================
  Files          4110     3228     -882     
  Lines         58609    53626    -4983     
  Branches      10134     9764     -370     
============================================
- Hits          29158    26934    -2224     
+ Misses        26930    24324    -2606     
+ Partials       2521     2368     -153

Impacted Files	Coverage Δ
...trics/prometheus/collector/BuildInfoCollector.java	`58.33% <0.00%> (-16.67%)`	⬇️
...e/schema/loader/common/AbstractDataTypeLoader.java	`71.42% <0.00%> (-14.29%)`	⬇️
...ere/datetime/database/DatabaseDatetimeService.java	`57.14% <0.00%> (-14.29%)`	⬇️
...ipeline/mysql/ingest/MySQLPositionInitializer.java	`54.54% <0.00%> (-13.64%)`	⬇️
...pe/MGRMySQLDatabaseDiscoveryProviderAlgorithm.java	`53.73% <0.00%> (-13.44%)`	⬇️
...stgresql/ingest/PostgreSQLPositionInitializer.java	`56.36% <0.00%> (-12.73%)`	⬇️
...andler/supporter/ShadowRuleStatementSupporter.java	`44.44% <0.00%> (-11.12%)`	⬇️
.../schema/loader/dialect/H2SchemaMetaDataLoader.java	`76.74% <0.00%> (-10.47%)`	⬇️
...hardingsphere/driver/jdbc/context/JDBCContext.java	`60.00% <0.00%> (-10.00%)`	⬇️
...rdingsphere/proxy/database/DatabaseServerInfo.java	`80.00% <0.00%> (-10.00%)`	⬇️
... and 1006 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

zhfeng · 2023-01-14T01:20:17Z

@terrymanu @TeslaCN can you take a review? Thanks!

linghengqian

LGTM.

terrymanu

Please hold on, I need some time to double check

TeslaCN · 2023-01-16T05:49:52Z

Hi @zhfeng @linghengqian
If we upgrade antlr to 4.10.+, even though ShardingSphere is Java 8 bytecode, but Java 8 runtime still cannot using antlr 4.10+.
Is it possible to build ShardingSphere parser modules with different classifiers?

TeslaCN · 2023-01-16T06:28:29Z

Hi @zhfeng @linghengqian If we upgrade antlr to 4.10.+, even though ShardingSphere is Java 8 bytecode, but Java 8 runtime still cannot using antlr 4.10+. Is it possible to build ShardingSphere parser modules with different classifiers?

Sorry, I thought the antlr 4.10.+ was compiled as Java 11 bytecode. It seems that only antlr tools require Java 11. The runtime is still Java 8 bytecode.

TeslaCN · 2023-01-16T06:31:04Z

pom.xml

@@ -944,7 +944,7 @@
                                    <version>${maven.version.range}</version>
                                </requireMavenVersion>
                                <requireJavaVersion>
-                                    <version>${java.version}</version>
+                                    <version>11</version>


Could we just keep the origin version here, otherwise we have to add -Denforcer.skip=true to do any other operations with Java 8.

Yeah, we can. But It cuould confuse people that building with Java 8 will fail after upgrading antlr to 4.10.1. Anyway, we need to add a NOTE in README to mention that Building from source of shardingsphere must use JDK 11 above. @terrymanu WDYT?

The compile should in JDK 11 (release for JDK 8) only.
I prefer to create compile java version and release java version properties to separate the different versions.

linghengqian

I think the problem that CI had were solved. You can try to rebase the branch.

zhfeng · 2023-01-22T13:55:26Z

Thanks @linghengqian and I will take care of the conflicts after the Chinese New Year holiday.

TeslaCN · 2023-01-28T03:08:29Z

pom.xml

@@ -944,7 +945,7 @@
                                    <version>${maven.version.range}</version>
                                </requireMavenVersion>
                                <requireJavaVersion>
-                                    <version>${java.version}</version>
+                                    <version>${compile.java.version}</version>


Hi @zhfeng
I prefer to set 8 as requireJavaVersion. Java 11 is required only when compiling parser modules.
Sometimes we just need to compile a part of modules (such as protocol, agent), build an image, do some tests, which could be done using Java 8. Adding -Denforcer.skip=true is not elegant.
And the error of compiling antlr 4.10+ with Java 8 is clear enough for developers.
We could clarify Java requirement in Wiki and docs.

Or is it possible to add maven-enforcer-plugin with Java 11 requirement to those modules require Java 11? The other modules keep using Java 8

Yeah, I think it makes sense and I will investigate.

zhfeng · 2023-01-28T06:44:09Z

Hi guys,

I just update to run CI build on JDK 11 but test it on JDK 8. <requireJavaVersion> is still with <java.version>. After upgrading to antlr 4.10+, the CI should work because it uses JDK 11 but if you build sharding-sphere from source with JDK 8 the following modules which use antlr4-maven-plugin will fail.

shardingsphere-distsql-parser
shardingsphere-db-discovery-distsql-parser
shardingsphere-encrypt-distsql-parser
shardingsphere-readwrite-splitting-distsql-parser
shardingsphere-shadow-distsql-parser
shardingsphere-sharding-distsql-parser
shardingsphere-mask-distsql-parser
shardingsphere-authority-distsql-parser
shardingsphere-data-pipeline-distsql-parser
shardingsphere-parser-distsql-parser
shardingsphere-sql-federation-optimizer
shardingsphere-sql-translator-distsql-parser
shardingsphere-traffic-distsql-parser
shardingsphere-transaction-distsql-parser
shardingsphere-sql-parser-mysql
shardingsphere-sql-parser-opengauss
shardingsphere-sql-parser-oracle
shardingsphere-sql-parser-postgresql
shardingsphere-sql-parser-sql92
shardingsphere-sql-parser-sqlserver

zhfeng · 2023-01-28T07:18:15Z

@TeslaCN @terrymanu can you take a review of these changes? I agree to clarify JDK requirement on wiki and docs. Only the modules which uses antlr-maven-plugin will be impact. If the community is happy to accept this PR, I'm going to prepare for the antlr 4.10+ upgrade which could be

Upgarde <antlr4.version> to 4.10.1
Run antlr-maven-plugin only in jdk11+ profile
In jdk8 profile, try to add the antlr generation directoy as source codes directory. if not, fail the building with the message Please use JDK 11 to re-build artifact to generate the anltr codes

any thought?

TeslaCN

LGTM

terrymanu · 2023-01-29T14:26:24Z

@zhfeng LGTM, thank you!

darren-wang · 2024-05-25T17:54:58Z

@zhfeng hi, I'm new to sharding's source code, could you have a look at question #31379 thanks a lot!

zhfeng mentioned this pull request Jan 13, 2023

Discuss the time to increase the minimum JDK compilation version of the ShardingSphere project to 11 #20935

Closed

linghengqian reviewed Jan 13, 2023

View reviewed changes

zhfeng requested a review from linghengqian January 14, 2023 01:20

zhfeng mentioned this pull request Jan 14, 2023

Update antlr4 to 4.10.1 close #19990 #20008

Closed

linghengqian previously approved these changes Jan 14, 2023

View reviewed changes

terrymanu requested changes Jan 14, 2023

View reviewed changes

TeslaCN reviewed Jan 16, 2023

View reviewed changes

zhfeng dismissed linghengqian’s stale review via cbf02c3 January 16, 2023 14:04

zhfeng force-pushed the force_to_build_on_jdk11 branch from 66b761a to cbf02c3 Compare January 16, 2023 14:04

linghengqian reviewed Jan 22, 2023

View reviewed changes

TeslaCN reviewed Jan 28, 2023

View reviewed changes

Run CI build on JDK 11 but compatitable with 8

8e30fe0

zhfeng force-pushed the force_to_build_on_jdk11 branch from cbf02c3 to 8e30fe0 Compare January 28, 2023 06:27

TeslaCN approved these changes Jan 29, 2023

View reviewed changes

TeslaCN requested a review from terrymanu January 29, 2023 01:17

terrymanu approved these changes Jan 29, 2023

View reviewed changes

terrymanu added this to the 5.3.2 milestone Jan 29, 2023

terrymanu added the type: build label Jan 29, 2023

terrymanu merged commit 41a417e into apache:master Jan 29, 2023

linghengqian mentioned this pull request Feb 5, 2023

Remove cobertura-maven-plugin and look for a replacement that works on JDK11+ #24009

Closed

darren-wang mentioned this pull request May 24, 2024

which is the last release version that can be compiled with jdk8? #31379

Closed

linghengqian mentioned this pull request Aug 10, 2024

Bump the minimum compilation JDK version to JDK17, while keeping the minimum runtime JDK version at JDK8 apache/shardingsphere-elasticjob#2420

Closed

linghengqian mentioned this pull request Nov 30, 2024

Bump the Antlr4 Maven Plugin version and bump the minimum JDK compilation level to JDK11 apache/incubator-seata#7036

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Force to build on JDK 11+ #23550

Force to build on JDK 11+ #23550

zhfeng commented Jan 13, 2023 •

edited

Loading

linghengqian left a comment

zhfeng commented Jan 13, 2023

zhfeng commented Jan 13, 2023 •

edited

Loading

linghengqian commented Jan 13, 2023 •

edited

Loading

codecov-commenter commented Jan 13, 2023

zhfeng commented Jan 14, 2023

linghengqian left a comment

terrymanu left a comment

TeslaCN commented Jan 16, 2023 •

edited

Loading

TeslaCN commented Jan 16, 2023

TeslaCN Jan 16, 2023

zhfeng Jan 16, 2023

terrymanu Jan 16, 2023 •

edited

Loading

linghengqian left a comment

zhfeng commented Jan 22, 2023

TeslaCN Jan 28, 2023

TeslaCN Jan 28, 2023

zhfeng Jan 28, 2023

zhfeng commented Jan 28, 2023

zhfeng commented Jan 28, 2023

TeslaCN left a comment

terrymanu commented Jan 29, 2023

darren-wang commented May 25, 2024

Force to build on JDK 11+ #23550

Force to build on JDK 11+ #23550

Conversation

zhfeng commented Jan 13, 2023 • edited Loading

linghengqian left a comment

Choose a reason for hiding this comment

zhfeng commented Jan 13, 2023

zhfeng commented Jan 13, 2023 • edited Loading

linghengqian commented Jan 13, 2023 • edited Loading

codecov-commenter commented Jan 13, 2023

Codecov Report

zhfeng commented Jan 14, 2023

linghengqian left a comment

Choose a reason for hiding this comment

terrymanu left a comment

Choose a reason for hiding this comment

TeslaCN commented Jan 16, 2023 • edited Loading

TeslaCN commented Jan 16, 2023

TeslaCN Jan 16, 2023

Choose a reason for hiding this comment

zhfeng Jan 16, 2023

Choose a reason for hiding this comment

terrymanu Jan 16, 2023 • edited Loading

Choose a reason for hiding this comment

linghengqian left a comment

Choose a reason for hiding this comment

zhfeng commented Jan 22, 2023

TeslaCN Jan 28, 2023

Choose a reason for hiding this comment

TeslaCN Jan 28, 2023

Choose a reason for hiding this comment

zhfeng Jan 28, 2023

Choose a reason for hiding this comment

zhfeng commented Jan 28, 2023

zhfeng commented Jan 28, 2023

TeslaCN left a comment

Choose a reason for hiding this comment

terrymanu commented Jan 29, 2023

darren-wang commented May 25, 2024

zhfeng commented Jan 13, 2023 •

edited

Loading

zhfeng commented Jan 13, 2023 •

edited

Loading

linghengqian commented Jan 13, 2023 •

edited

Loading

TeslaCN commented Jan 16, 2023 •

edited

Loading

terrymanu Jan 16, 2023 •

edited

Loading