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 2022/11/04 16:29:27 UTC
[GitHub] [maven-plugin-tools] kwin opened a new pull request, #171: [MPLUGIN-425] Add required Java and Maven version to the generated
kwin opened a new pull request, #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171
plugin descriptor
--
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-plugin-tools] kwin commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017123368
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/scanner/DefaultMojoAnnotationsScanner.java:
##########
@@ -85,12 +87,18 @@ public Map<String, MojoAnnotatedClass> scan( MojoAnnotationsScannerRequest reque
for ( Artifact dependency : request.getDependencies() )
{
scan( mojoAnnotatedClasses, dependency.getFile(), request.getIncludePatterns(), dependency, true );
+ if ( request.getMavenApiVersion() == null
+ && dependency.getFile().getName().contains( "maven-plugin-api" ) )
+ {
+ request.setMavenApiVersion( getSpecificationVersionOfJar( dependency.getFile() ) );
+ }
Review Comment:
I don’t understand. This value is actually derived from the maven-plugin-api dependency. Per plugin there is only one classpath and dependency so this cannot differ per mojo…
--
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-plugin-tools] kwin commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017532143
##########
maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/DescriptorGeneratorMojo.java:
##########
@@ -235,6 +239,44 @@
@Parameter( defaultValue = "${localRepository}", required = true, readonly = true )
private ArtifactRepository local;
+ /**
+ * The required Java version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>One of the values as for <a href="https://maven.apache.org/pom.html#Activation">POM profile activation
+ * element {@code jdk}</a>, i.e. version ranges, version prefixes
+ * and negated version prefixes (starting with '!').</li>
+ * <li>{@code "auto"} to determine the minimum Java version from the binary class version being generated during
+ * compilation (determined by the extractor).</li>
+ * <li>{@code "none"} to not set a required Java version.</li>
+ * </ul>
+ *
+ * @since 3.8.0
+ */
+ @Parameter( defaultValue = VALUE_AUTO )
+ String requiredJavaVersion;
+
+ /**
+ * The required Maven version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>A version range which specifies the supported Maven versions. It can either use the usual mathematical
+ * syntax like {@code "[2.0.10,2.1.0),[3.0,)"} or use a single version like {@code "2.2.1"}. The latter is a short
+ * form for {@code "[2.2.1,)"}, i.e. denotes the minimum version required.</li>
+ * <li>{@code "auto"} to determine the minimum Maven version from the POMs Maven prerequisite, or if not set the
+ * referenced Maven Plugin API version.</li>
+ * <li>{@code "none"} to not set a required Maven version.</li>
Review Comment:
IMHO MPLUGIN-418 is about the prerequisites in the pom.xml (relevant for Maven 3), while this PR is only about plugin descriptor (only relevant for Maven 4). Therefore even removing the "none" value here does not require a Maven prerequisite in the pom.xml. Still removing the none is probably a good idea, fixed in https://github.com/apache/maven-plugin-tools/pull/171/commits/4c8e2499919b75c2e1cd5a12e474dc65070f7d4f
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -102,6 +106,36 @@
private static final GroupKey GROUP_KEY = new GroupKey( GroupKey.JAVA_GROUP, 100 );
+ /**
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se19/html/jvms-4.html#jvms-4.1">JVMS 4.1</a>
+ */
+ private static final Map<Integer, String> CLASS_VERSION_TO_JAVA_STRING;
Review Comment:
Done in https://github.com/apache/maven-plugin-tools/pull/171/commits/4c8e2499919b75c2e1cd5a12e474dc65070f7d4f.
--
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-plugin-tools] kwin commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017532626
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/scanner/DefaultMojoAnnotationsScanner.java:
##########
@@ -85,12 +87,18 @@ public Map<String, MojoAnnotatedClass> scan( MojoAnnotationsScannerRequest reque
for ( Artifact dependency : request.getDependencies() )
{
scan( mojoAnnotatedClasses, dependency.getFile(), request.getIncludePatterns(), dependency, true );
+ if ( request.getMavenApiVersion() == null
+ && dependency.getFile().getName().contains( "maven-plugin-api" ) )
+ {
+ request.setMavenApiVersion( getSpecificationVersionOfJar( dependency.getFile() ) );
+ }
Review Comment:
Yes, a lot simpler, done in https://github.com/apache/maven-plugin-tools/pull/171/commits/4c8e2499919b75c2e1cd5a12e474dc65070f7d4f.
--
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-plugin-tools] slawekjaranowski commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017094991
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -102,6 +106,36 @@
private static final GroupKey GROUP_KEY = new GroupKey( GroupKey.JAVA_GROUP, 100 );
+ /**
+ *
+ * @see <a href="https://docs.oracle.com/javase/specs/jvms/se19/html/jvms-4.html#jvms-4.1">JVMS 4.1</a>
+ */
+ private static final Map<Integer, String> CLASS_VERSION_TO_JAVA_STRING;
Review Comment:
We should add reference to this code from pom to remember that we need update together with asm
##########
maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/DescriptorGeneratorMojo.java:
##########
@@ -235,6 +239,44 @@
@Parameter( defaultValue = "${localRepository}", required = true, readonly = true )
private ArtifactRepository local;
+ /**
+ * The required Java version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>One of the values as for <a href="https://maven.apache.org/pom.html#Activation">POM profile activation
+ * element {@code jdk}</a>, i.e. version ranges, version prefixes
+ * and negated version prefixes (starting with '!').</li>
+ * <li>{@code "auto"} to determine the minimum Java version from the binary class version being generated during
+ * compilation (determined by the extractor).</li>
+ * <li>{@code "none"} to not set a required Java version.</li>
+ * </ul>
+ *
+ * @since 3.8.0
+ */
+ @Parameter( defaultValue = VALUE_AUTO )
+ String requiredJavaVersion;
+
+ /**
+ * The required Maven version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>A version range which specifies the supported Maven versions. It can either use the usual mathematical
+ * syntax like {@code "[2.0.10,2.1.0),[3.0,)"} or use a single version like {@code "2.2.1"}. The latter is a short
+ * form for {@code "[2.2.1,)"}, i.e. denotes the minimum version required.</li>
+ * <li>{@code "auto"} to determine the minimum Maven version from the POMs Maven prerequisite, or if not set the
+ * referenced Maven Plugin API version.</li>
+ * <li>{@code "none"} to not set a required Maven version.</li>
Review Comment:
I would like to require always java version and maven version
It will resolve MPLUGIN-418
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/scanner/DefaultMojoAnnotationsScanner.java:
##########
@@ -85,12 +87,18 @@ public Map<String, MojoAnnotatedClass> scan( MojoAnnotationsScannerRequest reque
for ( Artifact dependency : request.getDependencies() )
{
scan( mojoAnnotatedClasses, dependency.getFile(), request.getIncludePatterns(), dependency, true );
+ if ( request.getMavenApiVersion() == null
+ && dependency.getFile().getName().contains( "maven-plugin-api" ) )
+ {
+ request.setMavenApiVersion( getSpecificationVersionOfJar( dependency.getFile() ) );
+ }
Review Comment:
Why not detect on Mojo level from dependencies?
--
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-plugin-tools] kwin merged pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
kwin merged PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171
--
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-plugin-tools] slawekjaranowski commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017181479
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/scanner/DefaultMojoAnnotationsScanner.java:
##########
@@ -85,12 +87,18 @@ public Map<String, MojoAnnotatedClass> scan( MojoAnnotationsScannerRequest reque
for ( Artifact dependency : request.getDependencies() )
{
scan( mojoAnnotatedClasses, dependency.getFile(), request.getIncludePatterns(), dependency, true );
+ if ( request.getMavenApiVersion() == null
+ && dependency.getFile().getName().contains( "maven-plugin-api" ) )
+ {
+ request.setMavenApiVersion( getSpecificationVersionOfJar( dependency.getFile() ) );
+ }
Review Comment:
ok, other question
why not use
```
dependency.getArtifactId();
dependency.getVersion();
```
instead of checking artifact file name and next read content from artifact manifest.
--
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-plugin-tools] slawekjaranowski commented on a diff in pull request #171: [MPLUGIN-425] Add required Java and Maven version to the generated
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #171:
URL: https://github.com/apache/maven-plugin-tools/pull/171#discussion_r1017556756
##########
maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/DescriptorGeneratorMojo.java:
##########
@@ -235,6 +238,42 @@
@Parameter( defaultValue = "${localRepository}", required = true, readonly = true )
private ArtifactRepository local;
+ /**
+ * The required Java version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>One of the values as for <a href="https://maven.apache.org/pom.html#Activation">POM profile activation
+ * element {@code jdk}</a>, i.e. version ranges, version prefixes
+ * and negated version prefixes (starting with '!').</li>
+ * <li>{@code "auto"} to determine the minimum Java version from the binary class version being generated during
+ * compilation (determined by the extractor).</li>
+ * </ul>
+ *
+ * @since 3.8.0
+ */
+ @Parameter( defaultValue = VALUE_AUTO )
+ String requiredJavaVersion;
+
+ /**
+ * The required Maven version to set in the plugin descriptor. This is evaluated by Maven 4 and ignored by earlier
+ * Maven versions. Can be either one of the following formats:
+ *
+ * <ul>
+ * <li>A version range which specifies the supported Maven versions. It can either use the usual mathematical
+ * syntax like {@code "[2.0.10,2.1.0),[3.0,)"} or use a single version like {@code "2.2.1"}. The latter is a short
+ * form for {@code "[2.2.1,)"}, i.e. denotes the minimum version required.</li>
+ * <li>{@code "auto"} to determine the minimum Maven version from the POMs Maven prerequisite, or if not set the
+ * referenced Maven Plugin API version.</li>
+ * </ul>
+ * This value (if not set to {@code "none"}) takes precedence over the
Review Comment:
`none` is not present
--
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