You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/10/14 13:04:10 UTC

[GitHub] [shardingsphere] linghengqian opened a new pull request, #21571: Update library to serve building GraalVM Native Image

linghengqian opened a new pull request, #21571:
URL: https://github.com/apache/shardingsphere/pull/21571

   For #21347.
   
   Changes proposed in this pull request:
     - Introduce `junit-vintage-engine` to support unit testing inside GraalVM Native Image. Reference https://github.com/graalvm/native-build-tools/issues/53 .
     - Update HikariCP to the highest possible version to better match the graalvm reachability metadata at https://github.com/oracle/graalvm-reachability-metadata/tree/master/metadata/com.zaxxer/HikariCP/5.0.1 .
     - Update netty to 4.1.84 to get the new automatically generate native-image `conditional` metadata for `ChannelHandler` implementations. See https://github.com/netty/netty/pull/12794 .
     - Update logback to match graalvm reachability metadata at https://github.com/oracle/graalvm-reachability-metadata/tree/master/metadata/ch.qos.logback/logback-classic/1.2.11 .
     - Update `maven-surefire-plugin` to milestone version to automatically handle use of `org.junit.platform:junit-platform-launcher`. Reference https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#testing-support-version-compatibility .
     - Update mockito to be in sync with the latest Junit Platform requirements as much as possible.
     - Update most of the dependencies to get https://github.com/oracle/graalvm-reachability-metadata support faster in the future. The exception is `atomikos`, it has some changes from `5.0.8` to `5.0.9`, but I can't see anything in its git commit log.
     - Handle most deprecated methods.
     - Update LICENSE.
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [x] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the pull request.
   - [x] I have passed maven check locally : `mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e`.
   - [x] I have made corresponding changes to the documentation.
   - [x] I have added corresponding unit tests for my changes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] codecov-commenter commented on pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#issuecomment-1279630811

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/21571?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#21571](https://codecov.io/gh/apache/shardingsphere/pull/21571?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f36a19c) into [master](https://codecov.io/gh/apache/shardingsphere/commit/00f5553fa3b5ba2b772e9fecb58fe72f6ee4119e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (00f5553) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #21571   +/-   ##
   =========================================
     Coverage     61.04%   61.05%           
     Complexity     2509     2509           
   =========================================
     Files          4111     4111           
     Lines         57293    57295    +2     
     Branches       7588     7588           
   =========================================
   + Hits          34977    34982    +5     
   + Misses        19381    19378    -3     
     Partials       2935     2935           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/21571?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...line/core/api/impl/AbstractPipelineJobAPIImpl.java](https://codecov.io/gh/apache/shardingsphere/pull/21571/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2FwaS9pbXBsL0Fic3RyYWN0UGlwZWxpbmVKb2JBUElJbXBsLmphdmE=) | `0.00% <ø> (ø)` | |
   | [...e/mode/repository/cluster/etcd/EtcdRepository.java](https://codecov.io/gh/apache/shardingsphere/pull/21571/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bW9kZS90eXBlL2NsdXN0ZXIvcmVwb3NpdG9yeS9wcm92aWRlci9ldGNkL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9tb2RlL3JlcG9zaXRvcnkvY2x1c3Rlci9ldGNkL0V0Y2RSZXBvc2l0b3J5LmphdmE=) | `56.79% <100.00%> (+1.09%)` | :arrow_up: |
   | [...handler/distsql/ral/hint/enums/HintSourceType.java](https://codecov.io/gh/apache/shardingsphere/pull/21571/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC9oYW5kbGVyL2Rpc3RzcWwvcmFsL2hpbnQvZW51bXMvSGludFNvdXJjZVR5cGUuamF2YQ==) | `42.85% <0.00%> (+42.85%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] terrymanu commented on a diff in pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
terrymanu commented on code in PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#discussion_r996323235


##########
pom.xml:
##########
@@ -611,7 +620,13 @@
             <dependency>
                 <groupId>junit</groupId>
                 <artifactId>junit</artifactId>
-                <version>${junit.version}</version>
+                <version>${junit4.version}</version>
+                <scope>test</scope>
+            </dependency>
+            <dependency>
+                <groupId>org.junit.vintage</groupId>
+                <artifactId>junit-vintage-engine</artifactId>
+                <version>${junit5.version}</version>

Review Comment:
   Does it necessary to import junit 4.x and 5.x together?



##########
pom.xml:
##########
@@ -112,18 +112,19 @@
         
         <hikari-cp.version>3.4.2</hikari-cp.version>
         <commons-dbcp2.version>2.2.0</commons-dbcp2.version>
-        
-        <junit.version>4.13.2</junit.version>
+
+        <junit5.version>5.9.1</junit5.version>
+        <junit4.version>4.13.2</junit4.version>
         <hamcrest.version>1.3</hamcrest.version>
-        <mockito.version>4.5.1</mockito.version>
+        <mockito.version>4.8.0</mockito.version>
         
         <!-- Plugin versions -->
         <maven-compiler-plugin.version>3.8.0</maven-compiler-plugin.version>
         <maven-source-plugin.version>3.2.1</maven-source-plugin.version>
         <maven-javadoc-plugin.version>3.3.0</maven-javadoc-plugin.version>
         <maven-resources-plugin.version>3.3.0</maven-resources-plugin.version>
         <maven-jar-plugin.version>3.2.0</maven-jar-plugin.version>
-        <maven-surefire-plugin.version>2.18.1</maven-surefire-plugin.version>
+        <maven-surefire-plugin.version>3.0.0-M7</maven-surefire-plugin.version>

Review Comment:
   Is the M7 version stable?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on a diff in pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on code in PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#discussion_r996326845


##########
pom.xml:
##########
@@ -112,18 +112,19 @@
         
         <hikari-cp.version>3.4.2</hikari-cp.version>
         <commons-dbcp2.version>2.2.0</commons-dbcp2.version>
-        
-        <junit.version>4.13.2</junit.version>
+
+        <junit5.version>5.9.1</junit5.version>
+        <junit4.version>4.13.2</junit4.version>
         <hamcrest.version>1.3</hamcrest.version>
-        <mockito.version>4.5.1</mockito.version>
+        <mockito.version>4.8.0</mockito.version>
         
         <!-- Plugin versions -->
         <maven-compiler-plugin.version>3.8.0</maven-compiler-plugin.version>
         <maven-source-plugin.version>3.2.1</maven-source-plugin.version>
         <maven-javadoc-plugin.version>3.3.0</maven-javadoc-plugin.version>
         <maven-resources-plugin.version>3.3.0</maven-resources-plugin.version>
         <maven-jar-plugin.version>3.2.0</maven-jar-plugin.version>
-        <maven-surefire-plugin.version>2.18.1</maven-surefire-plugin.version>
+        <maven-surefire-plugin.version>3.0.0-M7</maven-surefire-plugin.version>

Review Comment:
   - I believe `3.0.0-M7` is stable, but I don't know why the `maven-surefire-plugin` community is still using milestone versions.  
   
   - `Junit Platform` requires unit tests to be performed in `GraalVM Native Image`, `maven-surefire-plugin` version should be `>= 2.22.0`, but if we still use the version `< 3.0.0-M5`, we need to manually manage the version number of `JUnit Platform`, I think this operation of managing dependencies is unnecessary, refer to https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#testing-support-version-compatibility .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on a diff in pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on code in PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#discussion_r996325907


##########
pom.xml:
##########
@@ -611,7 +620,13 @@
             <dependency>
                 <groupId>junit</groupId>
                 <artifactId>junit</artifactId>
-                <version>${junit.version}</version>
+                <version>${junit4.version}</version>
+                <scope>test</scope>
+            </dependency>
+            <dependency>
+                <groupId>org.junit.vintage</groupId>
+                <artifactId>junit-vintage-engine</artifactId>
+                <version>${junit5.version}</version>

Review Comment:
   - Obviously, we don't need anything related to Junit 5 Jupiter.  This is essentially because Junit 4 cannot execute unit tests directly in GraalVM Native Image.  
   
   - Under Junit Platform, `native-maven-plugin` in GraalVM Native Build Tools provides `VintageConfigProvider`, refer to https://github.com/graalvm/native-build-tools/blob/master/common/junit-platform-native/src/main/java/org/graalvm/junit/platform/config/vintage/VintageConfigProvider.java .  
   
   - This allows us to continue using the `JUnit 4` API ostensibly, while actually executing the tests is `JUnit Vintage`.  The handling of this process requires the help of a later version of the `maven-surefire-plugin`.
   
   - This may be a bit difficult to understand. The simple explanation is that `Junit Platform` manages test platforms such as `Junit4`, `Junit Jupiter` and `Hamcrest` in a unified manner. The management of `Junit4` by `Junit Platform` is realized through `JUnit Vintage`.  The `GraalVM Native Build Tools` does not directly deal with the specific engine, but calls the API of the `Junit Platform` to complete the unit test in the `GraalVM Native Image`.
   
   - ![195968077-0a168cfb-4983-4f56-90df-9770d50f0ef7.png](https://user-images.githubusercontent.com/20187731/195996895-0cc3a75e-0310-46ef-90e6-b374ea3f8d28.png)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] terrymanu merged pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
terrymanu merged PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on a diff in pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on code in PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#discussion_r996326845


##########
pom.xml:
##########
@@ -112,18 +112,19 @@
         
         <hikari-cp.version>3.4.2</hikari-cp.version>
         <commons-dbcp2.version>2.2.0</commons-dbcp2.version>
-        
-        <junit.version>4.13.2</junit.version>
+
+        <junit5.version>5.9.1</junit5.version>
+        <junit4.version>4.13.2</junit4.version>
         <hamcrest.version>1.3</hamcrest.version>
-        <mockito.version>4.5.1</mockito.version>
+        <mockito.version>4.8.0</mockito.version>
         
         <!-- Plugin versions -->
         <maven-compiler-plugin.version>3.8.0</maven-compiler-plugin.version>
         <maven-source-plugin.version>3.2.1</maven-source-plugin.version>
         <maven-javadoc-plugin.version>3.3.0</maven-javadoc-plugin.version>
         <maven-resources-plugin.version>3.3.0</maven-resources-plugin.version>
         <maven-jar-plugin.version>3.2.0</maven-jar-plugin.version>
-        <maven-surefire-plugin.version>2.18.1</maven-surefire-plugin.version>
+        <maven-surefire-plugin.version>3.0.0-M7</maven-surefire-plugin.version>

Review Comment:
   - I believe `3.0.0-M7` is stable, but I don't know why the `maven-surefire-plugin` community is still using milestone versions.  
   
   - `Junit Platform` requires unit tests to be performed in `GraalVM Native Image`, `maven-surefire-plugin` version should be `>= 2.22.0`, but if we still use the version `< 3.0.0-M5`, we need to manually manage the version number of `JUnit Platform`, I  I think this operation of managing dependencies is unnecessary, refer to https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#testing-support-version-compatibility .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#issuecomment-1280108586

   - I noticed that `Groovy 4.0.6` released yesterday fixes https://issues.apache.org/jira/browse/GROOVY-10757 , which is the same as `--report-unsupported-elements-at-runtime` of GraalVM Native Build Tools' log displayed by the parameter has a certain relationship. 
   - This PR might help to validate https://github.com/apache/shardingsphere/issues/21475 , since that issue is using something like groovy web console. 
   - So I submitted a new commit, the PR is still available for review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] terrymanu commented on pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
terrymanu commented on PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#issuecomment-1282129260

   > * I have a local PR with the Conditional Agent Mode of GraalVM Native Build Tools, and its content is strongly related to the dependency version number. I am not sure if I should merge the content of these two PRs, or make another draft PR for review ? 🧐
   > * Or should I upgrade HikariCP's version number dependency in this PR and fix the problem caused by its breaking changes, and separate the HikariCP version used by `distribution/proxy` and `distribution/proxy-native`? It sounds like the first thing that needs to be added in a separate PR or this PR is what is requested by [Make ShardingSphere Proxy in GraalVM Native Image form available #21347 (comment)](https://github.com/apache/shardingsphere/issues/21347#issuecomment-1279683621) .
   
   Great, I will review soon


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#issuecomment-1281723008

   - I have a local PR with the Conditional Agent Mode of GraalVM Native Build Tools, and its content is strongly related to the dependency version number. I am not sure if I should merge the content of these two PRs, or make another draft PR for review ? 🧐


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] linghengqian commented on pull request #21571: Update library to serve building GraalVM Native Image

Posted by GitBox <gi...@apache.org>.
linghengqian commented on PR #21571:
URL: https://github.com/apache/shardingsphere/pull/21571#issuecomment-1279673602

   - It looks like some weird unit test bug is happening in places that my local device can't test. I'm waiting for the master branch to fix it.
   - ![image](https://user-images.githubusercontent.com/20187731/195972449-1f98cb36-03a7-40c1-bd37-7280bf1fb2b4.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org