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 2021/04/28 12:21:58 UTC
[GitHub] [maven-plugin-tools] cstamas opened a new pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
cstamas opened a new pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33
https://issues.apache.org/jira/browse/MPLUGIN-372
Changes:
- Bug: adjust descriptor mojo resolution scope to include provided scope as well
- drop use of deprecated `project.getRuntimeDependencies()`
- Sanitized generator and report mojos (injects both project and project.artifacts, just one is completely enough)
- GeneratorUtils using Artifacts not Dependencies (as they were not needed)
- JavaAnnotationsMojoDescriptorExtractor error reporting: report failed class at least
--
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-plugin-tools] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-895180401
merged as 8f354439765afdbf6ce166d299791117576e90f2
--
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] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-895108364
ping
--
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] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-845035124
Or it could be other way around: add a Mojo annotation that plugin is "happy" with MNG-7097 enabled? This seems a bit counter-productive, as IMHO 99% of plugins are happy with it, only some exceptions like cobertura (or other crafting CP and spawning JVM may suffer from 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-plugin-tools] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-845033124
@rfscholte ping.
Some explanation: MPLUGIN-372 (this PR) and MNG-7097 are not about same thing:
- this PR fixes an issue about plugin tooling that currently prevent us using `provided` scope for maven artifacts. Is needed to implement MPLUGIN-370. So is for "future" (yet to be released) plugins.
- OTOH, MNG-7097 is for "everything else" (and old plugins, not having MPLUGIN-370 implemented), but MNG-7097 needs more changes to allow cases where "all is needed", see MNG-5783... am thinking to extend Mojo annotation and make Mojos like these "ask" for full (MNG-7097 disabled) resolution?
--
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-plugin-tools] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-830621993
This change should be addressed, as it is crucial to put plugins maven deps to provided.
--
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-plugin-tools] rfscholte commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-830634627
By just looking at the code I don't see weird things. This ticket confirms my feeling that there was something wrong, but I can't see the impact yet. I guess you can see only the difference if MNG-7097 is ready.
--
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-plugin-tools] rfscholte commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r685091408
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/PluginReport.java
##########
@@ -316,13 +307,13 @@ private PluginDescriptor extractPluginDescriptor()
try
{
- List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getRuntimeDependencies() );
+ List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getArtifacts() );
pluginDescriptor.setDependencies( deps );
PluginToolsRequest request = new DefaultPluginToolsRequest( project, pluginDescriptor );
request.setEncoding( encoding );
request.setSkipErrorNoDescriptorsFound( true );
- request.setDependencies( dependencies );
+ request.setDependencies( new LinkedHashSet<>( project.getArtifacts() ) );
Review comment:
There's nothing we can do here anymore. Could be a different ticket to fix this inconsistency.
--
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] cstamas closed pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas closed pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33
--
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] cstamas commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r645499811
##########
File path: maven-plugin-tools-generators/src/main/java/org/apache/maven/tools/plugin/generator/GeneratorUtils.java
##########
@@ -120,14 +121,14 @@ public static void element( XMLWriter w, String name, String value, boolean asTe
}
/**
- * @param dependencies not null list of <code>Dependency</code>
+ * @param dependencies not null collection of <code>Artifact</code>
* @return list of component dependencies
*/
- public static List<ComponentDependency> toComponentDependencies( List<Dependency> dependencies )
+ public static List<ComponentDependency> toComponentDependencies( Collection<Artifact> dependencies )
Review comment:
fixed
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/PluginReport.java
##########
@@ -316,13 +307,13 @@ private PluginDescriptor extractPluginDescriptor()
try
{
- List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getRuntimeDependencies() );
+ List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getArtifacts() );
pluginDescriptor.setDependencies( deps );
PluginToolsRequest request = new DefaultPluginToolsRequest( project, pluginDescriptor );
request.setEncoding( encoding );
request.setSkipErrorNoDescriptorsFound( true );
- request.setDependencies( dependencies );
+ request.setDependencies( new LinkedHashSet<>( project.getArtifacts() ) );
Review comment:
fixed
--
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-plugin-tools] cstamas commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-854636778
@rfscholte for the rest of your comments: well, as it can be seen, these are all uses of `org.apache.maven.project.MavenProject#getArtifacts` method, which in turn has JavaDoc saying "All dependencies that this project has...". Also, old (removed) code was calling it "dependencies", so I did not change that. The util class is aligned though.
--
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-plugin-tools] rfscholte commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r685089152
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/AbstractGeneratorMojo.java
##########
@@ -313,7 +305,7 @@ static String getDefaultGoalPrefix( MavenProject project )
Set<Artifact> filteredDependencies;
if ( mojoDependencies == null )
{
- filteredDependencies = dependencies;
+ filteredDependencies = new LinkedHashSet<>( project.getArtifacts() );
Review comment:
Renaming `filteredDependencies` to `filteredArtifacts` should be sufficient, right?
--
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] cstamas commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r685123913
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/AbstractGeneratorMojo.java
##########
@@ -313,7 +305,7 @@ static String getDefaultGoalPrefix( MavenProject project )
Set<Artifact> filteredDependencies;
if ( mojoDependencies == null )
{
- filteredDependencies = dependencies;
+ filteredDependencies = new LinkedHashSet<>( project.getArtifacts() );
Review comment:
done
--
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] rfscholte commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r636054191
##########
File path: maven-plugin-tools-generators/src/main/java/org/apache/maven/tools/plugin/generator/GeneratorUtils.java
##########
@@ -120,14 +121,14 @@ public static void element( XMLWriter w, String name, String value, boolean asTe
}
/**
- * @param dependencies not null list of <code>Dependency</code>
+ * @param dependencies not null collection of <code>Artifact</code>
* @return list of component dependencies
*/
- public static List<ComponentDependency> toComponentDependencies( List<Dependency> dependencies )
+ public static List<ComponentDependency> toComponentDependencies( Collection<Artifact> dependencies )
{
List<ComponentDependency> componentDeps = new LinkedList<>();
- for ( Dependency dependency : dependencies )
+ for ( Artifact dependency : dependencies )
Review comment:
rename variable to `artifact`
##########
File path: maven-plugin-tools-generators/src/main/java/org/apache/maven/tools/plugin/generator/GeneratorUtils.java
##########
@@ -120,14 +121,14 @@ public static void element( XMLWriter w, String name, String value, boolean asTe
}
/**
- * @param dependencies not null list of <code>Dependency</code>
+ * @param dependencies not null collection of <code>Artifact</code>
* @return list of component dependencies
*/
- public static List<ComponentDependency> toComponentDependencies( List<Dependency> dependencies )
+ public static List<ComponentDependency> toComponentDependencies( Collection<Artifact> dependencies )
Review comment:
rename parameter to `artifacts`
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/PluginReport.java
##########
@@ -316,13 +307,13 @@ private PluginDescriptor extractPluginDescriptor()
try
{
- List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getRuntimeDependencies() );
+ List<ComponentDependency> deps = GeneratorUtils.toComponentDependencies( project.getArtifacts() );
pluginDescriptor.setDependencies( deps );
PluginToolsRequest request = new DefaultPluginToolsRequest( project, pluginDescriptor );
request.setEncoding( encoding );
request.setSkipErrorNoDescriptorsFound( true );
- request.setDependencies( dependencies );
+ request.setDependencies( new LinkedHashSet<>( project.getArtifacts() ) );
Review comment:
Are we adding artifacts or dependencies?
##########
File path: maven-plugin-plugin/src/main/java/org/apache/maven/plugin/plugin/AbstractGeneratorMojo.java
##########
@@ -313,7 +305,7 @@ static String getDefaultGoalPrefix( MavenProject project )
Set<Artifact> filteredDependencies;
if ( mojoDependencies == null )
{
- filteredDependencies = dependencies;
+ filteredDependencies = new LinkedHashSet<>( project.getArtifacts() );
Review comment:
dependencies or artifacts?
--
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-plugin-tools] cstamas commented on a change in pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#discussion_r645500107
##########
File path: maven-plugin-tools-generators/src/main/java/org/apache/maven/tools/plugin/generator/GeneratorUtils.java
##########
@@ -120,14 +121,14 @@ public static void element( XMLWriter w, String name, String value, boolean asTe
}
/**
- * @param dependencies not null list of <code>Dependency</code>
+ * @param dependencies not null collection of <code>Artifact</code>
* @return list of component dependencies
*/
- public static List<ComponentDependency> toComponentDependencies( List<Dependency> dependencies )
+ public static List<ComponentDependency> toComponentDependencies( Collection<Artifact> dependencies )
{
List<ComponentDependency> componentDeps = new LinkedList<>();
- for ( Dependency dependency : dependencies )
+ for ( Artifact dependency : dependencies )
Review comment:
fixed
--
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-plugin-tools] rfscholte commented on pull request #33: [MPLUGIN-372] Descriptor mojo fails if super class is provided scope
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #33:
URL: https://github.com/apache/maven-plugin-tools/pull/33#issuecomment-845056537
To me cobertura shouldn't block us for implementing this improvement.
--
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