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

Update antlr4 to 4.10.1 close #19990 #20008

Closed
wants to merge 1 commit into from
Closed

Conversation

shanrenxj
Copy link

Fixes #19990.
Update antlr4 to 4.10.1
Changes proposed in this pull request:

Copy link
Member

@linghengqian linghengqian left a comment

Choose a reason for hiding this comment

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

@linghengqian
Copy link
Member

I noticed the magic antlr/antlr4#3748. Maybe you can try setting to 4.10.0 locally to edit.

@shanrenxj
Copy link
Author

shanrenxj commented Aug 9, 2022

I noticed the magic antlr/antlr4#3748. Maybe you can try setting to 4.10.0 locally to edit.

Since antlr4 4.10, antlr4 has started to use Java 11 for source code compile.

Going forward, we are using Java 11 for the source code and the compiled .class files for the ANTLR tool. The Java runtime target, however, and the associated runtime tests use Java 8 (bumping up from Java 7).

@linghengqian
Copy link
Member

linghengqian commented Aug 9, 2022

4.10.1 is "Going forward", but 4.10.0 is not. You need to reprocess dynamically generated files for the breaking changes of antlr 4.10.0. This is the first step in fixing CI.

@shanrenxj
Copy link
Author

4.10.1 is "Going forward", but 4.10.0 is not. You need to reprocess dynamically generated files for the breaking changes of antlr 4.10.0. This is the first step in fixing CI.


I looked at the pom file for ANTLR 4.10 and it has started using java11

@linghengqian
Copy link
Member

linghengqian commented Aug 9, 2022

  • I pointed out a fun PR. Although maven.compiler.source and maven.compiler.target are set to 11, the release of maven-compiler-plugin for all subprojects is 8. This is also why the ShardingSphere project can use Groovy 4.0.3, even though the master branch of Groovy requires JDK 16 builds.

@shanrenxj
Copy link
Author

  • I pointed out a fun PR. Although maven.compiler.source and maven.compiler.target are set to 11, the release of maven-compiler-plugin for all subprojects is 8. This is also why the ShardingSphere project can use Groovy 4.0.3, even though the master branch of Groovy requires JDK 16 builds.

It seems that shardingsphere needs to do the same

@linghengqian
Copy link
Member

  • I pointed out a fun PR. Although maven.compiler.source and maven.compiler.target are set to 11, the release of maven-compiler-plugin for all subprojects is 8. This is also why the ShardingSphere project can use Groovy 4.0.3, even though the master branch of Groovy requires JDK 16 builds.

It seems that shardingsphere needs to do the same

I don't think so. I'm currently working on some other PR preparations, so I can't test my ideas right away. If you think there are some easy ideas to deal with it, it's a good idea to submit a new commit to be verified by CI.

@shanrenxj
Copy link
Author

  • I pointed out a fun PR. Although maven.compiler.source and maven.compiler.target are set to 11, the release of maven-compiler-plugin for all subprojects is 8. This is also why the ShardingSphere project can use Groovy 4.0.3, even though the master branch of Groovy requires JDK 16 builds.

It seems that shardingsphere needs to do the same

I don't think so. I'm currently working on some other PR preparations, so I can't test my ideas right away. If you think there are some easy ideas to deal with it, it's a good idea to submit a new commit to be verified by CI.

As the project is in a hurry, our team currently uses hibernate core: 6.0.0 (ANTLR 4.9.1) to avoid ANTLR conflicts. Looking forward to shardingsphere update, thanks!

@linghengqian
Copy link
Member

This PR actually drops ShardingSphere JDBC support for versions of hibernate < 6.0.1, the description of the PR should be changed to reflect this. I'm not sure if you are still working on this PR, as this PR is still marked for review and has unfixed CI bugs.

@linghengqian
Copy link
Member

Synchronous comments: I suggest you open a new issue to discuss the point in time to migrate the base JDK version of ShardingSphere to JDK11. This issue will be blocked before the new issue is resolved, so I suggest closing this PR.

@terrymanu
Copy link
Member

Does anyone still focus this PR?

@linghengqian
Copy link
Member

This PR has no point of continuing to resolve unless #20935 is resolved.

@chenzhenjia
Copy link

my project upgrade to 4.10.1 not working

ANTLR Tool version 4.9.2 used for code generation does not match the current runtime version 4.10.1
ANTLR Runtime version 4.9.2 used for parser compilation does not match the current runtime version 4.10.1

jakarta.servlet.ServletException: Handler dispatch failed: java.lang.ExceptionInInitializerError
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1095)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:973)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1003)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:895)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:527)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:880)
    .....
Caused by: java.lang.UnsupportedOperationException: java.io.InvalidClassException: org.antlr.v4.runtime.atn.ATN; Could not deserialize ATN with version 3 (expected 4).
	at org.antlr.v4.runtime.atn.ATNDeserializer.deserialize(ATNDeserializer.java:56)
	at org.antlr.v4.runtime.atn.ATNDeserializer.deserialize(ATNDeserializer.java:48)
	at org.apache.shardingsphere.sql.parser.autogen.MySQLStatementLexer.<clinit>(MySQLStatementLexer.java:4533)
	... 246 common frames omitted
Caused by: java.io.InvalidClassException: org.antlr.v4.runtime.atn.ATN; Could not deserialize ATN with version 3 (expected 4).
	... 249 common frames omitted

@linghengqian
Copy link
Member

linghengqian commented Nov 24, 2022

@chenzhenjia I'm assuming you have a PR that goes through CI to fix the issue. This only affects users using Hibernate V6.

@chenzhenjia
Copy link

@linghengqian I use spring boot3 rc2 and hibernate 6.1.5

@linghengqian
Copy link
Member

linghengqian commented Nov 24, 2022

@chenzhenjia

@chenzhenjia
Copy link

chenzhenjia commented Nov 25, 2022

I am not willing to downgrade hibernate to 5.x or 6.x, I provide a gradle solution:

  1. copy antlr4 dir all file to project src/main/antlr dir
// my project tree
project/src/
├── main
│   ├── antlr
│   ├── java
│   ├── resources
  1. Modify build.gradle.kts
//build.gradle.kts
plugins {
    idea
    antlr
    id("org.springframework.boot") 
}
dependencies {
    constraints {
        implementation("org.antlr:antlr4-runtime:4.10.1")
    }
    antlr("org.antlr:antlr4:4.10.1")
    compileOnly("org.antlr:antlr4-runtime:4.10.1")
}
tasks.generateGrammarSource {
    exclude("imports/**")
    arguments = listOf(
        "-lib",
        project.file("/src/main/antlr/imports/mysql").absolutePath,
        "-package",
        "org.apache.shardingsphere.sql.parser.autogen",
        "-visitor"
    )
}
  1. run project

@linghengqian
Copy link
Member

  • As a reminder, for anyone who wants to bypass the master branch to solve this problem, they can directly modify the master branch according to the content of Update Anltr to 4.10 to support Hibernate 6 integration #20119 , and then compile with jdk11+ to generate a new shardingsphere jdbc distribution containing new antlr4 runtime file. Until someone is ready to initiate a poll on the maillist.

@TeslaCN TeslaCN mentioned this pull request Jan 11, 2023
6 tasks
@zhfeng
Copy link
Contributor

zhfeng commented Jan 11, 2023

@zhfeng
Copy link
Contributor

zhfeng commented Jan 11, 2023

@terrymanu @linghengqian should we start a thread on mailing list to discuss this topic? I think it is a formal way and maybe we need a voting?

@linghengqian
Copy link
Member

@zhfeng I'm assuming you'll go to the maillist and open a new thread, since I'm not at all sure how big of a backlash there is to raising the JDK version. If we don't initiate a vote, everything is unknown.

@zhfeng
Copy link
Contributor

zhfeng commented Jan 11, 2023

@linghengqian yeah, I will open a thread on the mailing list for this topic soon. And for sure, the voting is definitely helpful.

@terrymanu WDYT?

@zhfeng
Copy link
Contributor

zhfeng commented Jan 11, 2023

@zhfeng
Copy link
Contributor

zhfeng commented Jan 12, 2023

@linghengqian It seems that only org.antlr:antlr is JDK 11, the org.antlr:antlr4-runtime is still compatitable with JDK 1.8. I just try to build master branch with antlr-4.10.1 by using Java 11 and swith to use JDK 1.8 to run a examples/shardingsphere-parser-example. It works !

So is it possible to just change the CI to build with JDK 11 and run all the tests & examples by JDK 8?

@linghengqian
Copy link
Member

@zhfeng

  • I wanted to do this from the beginning, but I don't know how to change the Github Actions files. The unit tests are always with the generated class files, and it seems difficult to stand alone test files.

@zhfeng
Copy link
Contributor

zhfeng commented Jan 12, 2023

@linghengqian you can check github/workflows/ci.yml at first.

The unit tests are always with the generated class files, and it seems difficult to stand alone test files.

I don't understand what you mean here.

@linghengqian
Copy link
Member

@zhfeng

  • What I mean is that we must improve the JDK compilation version of the ShardingSphere project, remove all JDK 8 configurations from the existing CI configuration, and determine what the new JDK 8-related Github Actions file looks like. All modules seem to require changes similar to the following.
<properties>
     <maven.compiler.source>11</maven.compiler.source>
     <maven.compiler.target>11</maven.compiler.target>
</properties>

<build>
     <plugins>
         <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-compiler-plugin</artifactId>
             <configuration>
                <release>8</release>
             </configuration>
         </plugin>
     </plugins>
</build>
  • I don't use Github Actions a lot, so I don't know how to tune CI. I'm assuming a friend will open a new PR?

@zhfeng
Copy link
Contributor

zhfeng commented Jan 12, 2023

Thanks @linghengqian and it makes sense to add <release>8</release>. Also it should make sure only run antlr-maven-plugin in profile which jdk.version is [11,). I suppose that antlr-maven-plugin has a propery antlr.skip but it doesn't. So it might be useful to raise to add it in antlr.

And it will add a step in ci.yml to just run all the unit tests on JDK 8 to make sure we don't break anything.

@zhfeng
Copy link
Contributor

zhfeng commented Jan 13, 2023

Please review #20935

@zhfeng
Copy link
Contributor

zhfeng commented Jan 14, 2023

Well, antlr.skip is no useful for our case. It needs to load the ExecuteMojo which has dependency of org/antlr/v4/Tool targeting for 11. So there is still throwing java.lang.UnsupportedClassVersionError at build time.

The only way is to have antlr4-maven-plugin in the jdk-11 profile. I will take care of upgrading antlr4 if

@linghengqian
Copy link
Member

@zhfeng Hi, I was wondering if you are preparing a PR to change the version of Antlr4? I'm trying to confirm the Antlr4 version of the GraalVM reachability metadata I need to submit at oracle/graalvm-reachability-metadata#198 .

@zhfeng
Copy link
Contributor

zhfeng commented Jan 31, 2023

@linghengqian Yeah, I'm working on the PR and hope to get it out this week.

@zhfeng
Copy link
Contributor

zhfeng commented Feb 2, 2023

It should be suppressed #23937

@linghengqian
Copy link
Member

@TeslaCN TeslaCN closed this Feb 3, 2023
Sign up for free to 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.

Update antlr4 to 4.10+
6 participants