You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "aherbert (via GitHub)" <gi...@apache.org> on 2023/04/25 11:23:05 UTC
[GitHub] [maven-compiler-plugin] aherbert opened a new pull request, #187: [MCOMPILER-534] Document conditional setting of the --release property
aherbert opened a new pull request, #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187
Tested using:
```
mvn site:site -DgenerateReports=false
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] michael-o commented on pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#issuecomment-1529137199
> Sure, update the Jira ticket as you see fit. This is more documentation of the current intent of the plugin.
>
> I am not familiar with the CI workflow but something does not seem correct with the current build; I believe it to be unrelated to this doc change.
Will address this PR tomorrow.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] michael-o commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181275834
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
Please rework the docs. I prefer rather no docs than confusing docs.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181277904
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
Revised docs to explicitly state building "using JDK 8 and also JDK 9 or later". I think this is crux of the issue. If you only use one toolchain for the build then there is no issue, just configure it for that toolchain. If you wish your build to be flexible then you require conditional setting of the 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#issuecomment-1529135936
Sure, update the Jira ticket as you see fit. This is more documentation of the current intent of the plugin.
I am not familiar with the CI workflow but something does not seem correct with the current build; I believe it to be unrelated to this doc change.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#issuecomment-1529082748
The build fails due to something totally unrelated. This is a change to the site documentation. That is not what is failing the build. When I looked at this the other PRs, and also the master branch, were also failing. I will rebase and push to collect a fix that I presume is now in master.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181265371
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
The whole point is you want to be able to build with JDK 8 or later. If you want to only build with JDK 9 or later then you do not have to use conditional setting of the --release option.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] asfgit closed pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
URL: https://github.com/apache/maven-compiler-plugin/pull/187
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181267491
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
Change "it requires this flag" to "it requires this flag when using a JDK later than 8".
If you use the flag with JDK 8 then the build breaks. This was the original source of [MCOMPILER-534]. You can only use the flag with JDK 9+:
```
Target Tool chain --release flag Result
Java 8 JDK 8 Y build error
Java 8 JDK 8 N OK
Java 8 JDK 9 Y OK (compiler targets Java 8)
Java 8 JDK 9 N compiler targets Java 9 API methods that supersede Java 8 methods
```
If this is not the desired behaviour, i.e. the --release flag should be harmless if set on JDK 8, then something should be changed in the compiler plugin. If you should not use the flag when using JDK 8, then this is what I am trying to address with this documentation.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] michael-o commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181270810
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
The table does not look right, line 2. No JDK 8 exhibits `--release`. You can never provide it on JDK 8. What am I missing again?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] michael-o commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181266159
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
That is not what the sentence tells me. It says: Targettting Java 8 need JDK 8 or later and it requires this flag. What am I misunderstanding? I understand what you say and you are right, but the documentation is not for me.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] aherbert commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181274592
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
The table `--release` column should really read `set <maven.compiler.release>8</maven.compiler.release>` but that was too long. Forgive my shorthand with no explanation.
When you set `<maven.compiler.release>8</maven.compiler.release>` then the maven plugin will use the `--release` flag. This is not ignored by JDK 8; as you state it does not exhibit this flag as an option. Using it raises an error. So you cannot set it when building with JDK 8. You should only set it for JDK 9+.
If this is not clear from my documentation then we can revise it. It is trying to state:
1. Do not use this flag when building with JDK 8
2. Use it when building with a later JDK (i.e. 9+) and targeting Java 8
I think a perfectly valid example is a project with a toolchain that uses JDK 8 only (i.e. it can be built with JDK 8). This can still be compiled with JDK 11. But that may end up targeting the wrong Java API if some methods/classes have had updates. So your project then should _conditionally_ set the `<maven.compiler.release>` property. If you always set it then you have problems on JDK 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] garydgregory commented on pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#issuecomment-1529027225
@aherbert ping, the build failed. You might have to rerun the job if the logs have already been removed.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] michael-o commented on a diff in pull request #187: [MCOMPILER-534] Document conditional setting of the --release property
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #187:
URL: https://github.com/apache/maven-compiler-plugin/pull/187#discussion_r1181265158
##########
src/site/apt/examples/set-compiler-release.apt.vm:
##########
@@ -80,3 +80,30 @@ Setting the <<<--release>>> of the Java Compiler
since Java 9. As such, the release number does not start with 1.x anymore. Also note that the
supported <<<release>>> targets include the release of the currently used JDK plus a limited number
of previous releases.
+
+* Usage on JDK 8
+
+ The <<<--release>>> option is used to target an older Java release than the JDK used during the
+ build process. This flag is not supported using JDK 8. To enable a project that targets Java 8
+ to be built using JDK 8 or later requires the conditional usage of the <<<--release>>> option.
Review Comment:
JDK 9 or later
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org