You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/10/28 20:11:01 UTC
[GitHub] [maven-jlink-plugin] sparsick opened a new pull request #9: [MJLINK-50] -Upgrade to Java 11
sparsick opened a new pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9
- set Java 11 as minimum
- update compiler plugin
- update maven-plugin-plugin because of java 11 issues
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MJLINK-50) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[MJLINK-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MJLINK-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r514121647
##########
File path: pom.xml
##########
@@ -67,6 +67,7 @@
<properties>
<mavenVersion>3.1.0</mavenVersion>
<maven.compiler.release>8</maven.compiler.release>
+ <maven.compiler.target>8</maven.compiler.target> <!-- this property is needed by enforcer plugin configured in maven-parent -->
Review comment:
Fix see next commit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] rfscholte commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513752468
##########
File path: pom.xml
##########
@@ -66,8 +66,7 @@
<properties>
<mavenVersion>3.0</mavenVersion>
- <maven.compiler.source>1.7</maven.compiler.source>
- <maven.compiler.target>1.7</maven.compiler.target>
+ <maven.compiler.release>11</maven.compiler.release>
Review comment:
It would make sense to call JLink via toolchain, so there's no need for this requirement. Recently we've decided to make the minimum Java 8 for Maven plugins
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] aalmiray commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
aalmiray commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513743325
##########
File path: pom.xml
##########
@@ -132,6 +131,16 @@
<build>
<pluginManagement>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-plugin-plugin</artifactId>
+ <version>3.6.0</version>
+ </plugin>
<plugin>
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.0.0-M1</version>
Review comment:
Perhaps the enforcer configuration requires an update as well. Please check the rules currently applied.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r514121512
##########
File path: pom.xml
##########
@@ -146,7 +169,7 @@
<version>3.0.0-M3</version>
<executions>
<execution>
- <id>enforce-bytecode-version</id>
+ <id>enforce-bytecode-version</id> <!-- todo this configuration doesn't overwrite the execution defined in maven-parent -->
Review comment:
Fix see next commit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] bmhm commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
bmhm commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513802569
##########
File path: pom.xml
##########
@@ -66,8 +66,7 @@
<properties>
<mavenVersion>3.0</mavenVersion>
- <maven.compiler.source>1.7</maven.compiler.source>
- <maven.compiler.target>1.7</maven.compiler.target>
+ <maven.compiler.release>11</maven.compiler.release>
Review comment:
But jlink doesn't come with java 8?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] bmhm commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
bmhm commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513802569
##########
File path: pom.xml
##########
@@ -66,8 +66,7 @@
<properties>
<mavenVersion>3.0</mavenVersion>
- <maven.compiler.source>1.7</maven.compiler.source>
- <maven.compiler.target>1.7</maven.compiler.target>
+ <maven.compiler.release>11</maven.compiler.release>
Review comment:
But jlink doesn't come with java 8?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] bmhm commented on a change in pull request #9: [MJLINK-50] upgrade to Java 8 / Maven 3.1.0
Posted by GitBox <gi...@apache.org>.
bmhm commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r514263074
##########
File path: pom.xml
##########
@@ -118,42 +117,76 @@
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
- <version>2.19.0</version>
+ <version>3.5.13</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
- <version>2.9.1</version>
+ <version>3.16.1</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.vintage</groupId>
+ <artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.7.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
<build>
<pluginManagement>
<plugins>
<plugin>
- <artifactId>maven-enforcer-plugin</artifactId>
- <version>3.0.0-M1</version>
- <executions>
- <execution>
- <id>enforce-bytecode-version</id>
- <configuration>
- <rules>
- <enforceBytecodeVersion>
- <maxJdkVersion>1.7</maxJdkVersion>
- <excludes>
- <exclude>org.ow2.asm:asm</exclude>
- </excludes>
- </enforceBytecodeVersion>
- <requireSameVersions />
- </rules>
- </configuration>
- </execution>
- </executions>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-plugin-plugin</artifactId>
+ <version>3.6.0</version>
</plugin>
</plugins>
</pluginManagement>
+ <plugins>
+ <plugin>
+ <artifactId>maven-enforcer-plugin</artifactId>
+ <version>3.0.0-M3</version>
+ <executions>
+ <execution>
+ <id>enforce-bytecode-version</id>
+ <configuration>
+ <rules>
+ <enforceBytecodeVersion>
+ <maxJdkVersion>8</maxJdkVersion>
Review comment:
wouldn’t it make sense to use this and line 69 (compiler.release) as property?
##########
File path: pom.xml
##########
@@ -65,9 +65,8 @@
</distributionManagement>
<properties>
- <mavenVersion>3.0</mavenVersion>
- <maven.compiler.source>1.7</maven.compiler.source>
- <maven.compiler.target>1.7</maven.compiler.target>
+ <mavenVersion>3.1.0</mavenVersion>
+ <maven.compiler.release>8</maven.compiler.release>
Review comment:
defining `maven.compiler.release` will make it incompatible with Java 8.
Please use `maven.compiler.source` and `maven.compiler.target` for now. Also see my other comment.
`jlink` will be obtained from the `$PATH` or `toolchains.xml`, so this does not prevent this plugin from running with Java 8.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r514105741
##########
File path: pom.xml
##########
@@ -146,7 +169,7 @@
<version>3.0.0-M3</version>
<executions>
<execution>
- <id>enforce-bytecode-version</id>
+ <id>enforce-bytecode-version</id> <!-- todo this configuration doesn't overwrite the execution defined in maven-parent -->
Review comment:
It seems that this configuration doesn't overwrite the configuration with the same defined in maven-parent
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#issuecomment-718578254
@bmhm good point. Thank you. I will update the issue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] rfscholte commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513751225
##########
File path: pom.xml
##########
@@ -132,6 +131,16 @@
<build>
<pluginManagement>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-plugin-plugin</artifactId>
+ <version>3.6.0</version>
Review comment:
We want to keep the plugin useful for as many maven version as possible. Most likely this plugin doesn't use any feature that requires 3.6.3. Recently we've decided to change the minimum for plugin from 3.0 to 3.1.0
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on pull request #9: [MJLINK-50] upgrade to Java 8 / Maven 3.1.0
Posted by GitBox <gi...@apache.org>.
sparsick commented on pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#issuecomment-718777426
@bmhm :+1: I changed it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r514103082
##########
File path: pom.xml
##########
@@ -67,6 +67,7 @@
<properties>
<mavenVersion>3.1.0</mavenVersion>
<maven.compiler.release>8</maven.compiler.release>
+ <maven.compiler.target>8</maven.compiler.target> <!-- this property is needed by enforcer plugin configured in maven-parent -->
Review comment:
I had to insert this property because enforcer-maven-plugin configured in maven-parent uses this property. Without this property I got following error:
```
[INFO] --- maven-enforcer-plugin:3.0.0-M3:enforce (enforce-bytecode-version) @ maven-jlink-plugin ---
[INFO] Adding ignore: module-info
[INFO] Restricted to JDK 1.7 yet org.mockito:mockito-core:jar:3.5.13:test contains org/mockito/AdditionalAnswers.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.platform:junit-platform-commons:jar:1.7.0:test contains org/junit/platform/commons/JUnitException.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.objenesis:objenesis:jar:3.1:test contains org/objenesis/ObjenesisHelper.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.vintage:junit-vintage-engine:jar:5.7.0:test contains org/junit/vintage/engine/JUnit4VersionCheck.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.assertj:assertj-core:jar:3.16.1:test contains org/assertj/core/condition/Not.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.jupiter:junit-jupiter-params:jar:5.7.0:test contains org/junit/jupiter/params/ParameterizedTest.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.jupiter:junit-jupiter-engine:jar:5.7.0:test contains org/junit/jupiter/engine/Constants.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.jupiter:junit-jupiter-api:jar:5.7.0:test contains org/junit/jupiter/api/AfterAll.class targeted to JDK 8
[INFO] Restricted to JDK 1.7 yet org.junit.platform:junit-platform-engine:jar:1.7.0:test contains org/junit/platform/engine/CompositeFilter$1.class targeted to JDK 8
[WARNING] Rule 0: org.apache.maven.plugins.enforcer.EnforceBytecodeVersion failed with message:
Found Banned Dependency: org.mockito:mockito-core:jar:3.5.13
Found Banned Dependency: org.junit.platform:junit-platform-commons:jar:1.7.0
Found Banned Dependency: org.objenesis:objenesis:jar:3.1
Found Banned Dependency: org.junit.vintage:junit-vintage-engine:jar:5.7.0
Found Banned Dependency: org.assertj:assertj-core:jar:3.16.1
Found Banned Dependency: org.junit.jupiter:junit-jupiter-params:jar:5.7.0
Found Banned Dependency: org.junit.jupiter:junit-jupiter-engine:jar:5.7.0
Found Banned Dependency: org.junit.jupiter:junit-jupiter-api:jar:5.7.0
Found Banned Dependency: org.junit.platform:junit-platform-engine:jar:1.7.0
Use 'mvn dependency:tree' to locate the source of the banned dependencies.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 3.723 s
[INFO] Finished at: 2020-10-29T10:05:48+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M3:enforce (enforce-bytecode-version) on project maven-jlink-plugin: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed. -> [Help 1]
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] sparsick commented on pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
sparsick commented on pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#issuecomment-718475463
@rfscholte @aalmiray Thanks for your review. I change the pom to address your comment.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-jlink-plugin] aalmiray commented on a change in pull request #9: [MJLINK-50] -Upgrade to Java 11
Posted by GitBox <gi...@apache.org>.
aalmiray commented on a change in pull request #9:
URL: https://github.com/apache/maven-jlink-plugin/pull/9#discussion_r513742812
##########
File path: pom.xml
##########
@@ -132,6 +131,16 @@
<build>
<pluginManagement>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-plugin-plugin</artifactId>
+ <version>3.6.0</version>
Review comment:
Wouldn't it make more sense to update `<mavenVersion>3.0</mavenVersion>` to `3.6.3`? Other dependencies in the POM are tied to that property.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org