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/09/26 12:55:20 UTC
[GitHub] [maven-compiler-plugin] cstamas opened a new pull request, #151: Move away from legacy maven-compat
cstamas opened a new pull request, #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151
And switch to resolver to resolver annotation processors with 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-compiler-plugin] ajarmoniuk commented on a diff in pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#discussion_r1081577683
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1841,29 +1838,36 @@ private List<String> resolveProcessorPathEntries()
ArtifactHandler handler = artifactHandlerManager.getArtifactHandler( coord.getType() );
Artifact artifact = new DefaultArtifact(
- coord.getGroupId(),
- coord.getArtifactId(),
- VersionRange.createFromVersionSpec( coord.getVersion() ),
- Artifact.SCOPE_RUNTIME,
- coord.getType(),
- coord.getClassifier(),
- handler,
- false );
-
- ArtifactResolutionRequest request = new ArtifactResolutionRequest()
- .setArtifact( artifact )
- .setResolveRoot( true )
- .setResolveTransitively( true )
- .setLocalRepository( session.getLocalRepository() )
- .setRemoteRepositories( project.getRemoteArtifactRepositories() );
-
- ArtifactResolutionResult resolutionResult = repositorySystem.resolve( request );
-
- resolutionErrorHandler.throwErrors( request, resolutionResult );
-
- for ( Artifact resolved : resolutionResult.getArtifacts() )
+ coord.getGroupId(),
+ coord.getArtifactId(),
+ coord.getClassifier(),
+ handler.getExtension(),
+ coord.getVersion()
+ );
+
+ CollectRequest collectRequest = new CollectRequest( new Dependency( artifact, JavaScopes.RUNTIME ),
+ project.getRemoteProjectRepositories() );
+ DependencyRequest dependencyRequest = new DependencyRequest();
+ dependencyRequest.setCollectRequest( collectRequest );
+ DependencyResult dependencyResult = repositorySystem.resolveDependencies(
+ session.getRepositorySession(), dependencyRequest );
+
+ ArrayList<ArtifactResult> failed = new ArrayList<>();
+ for ( ArtifactResult resolved : dependencyResult.getArtifactResults() )
+ {
+ if ( resolved.getArtifact() != null && resolved.getArtifact().getFile() != null )
+ {
+ elements.add( resolved.getArtifact().getFile().getAbsolutePath() );
+ }
+ else
+ {
+ failed.add( resolved );
Review Comment:
Correct. In addition, result.getExceptions() will contain the list of all gathered exceptions (if there are more than one).
--
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] ajarmoniuk commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1387479684
Hey, I can do that!
--
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] psiroky commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396837771
I think we can close this one in favor of #169. Thanks @cstamas for the initial 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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] cstamas commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1397336608
Superseded by https://github.com/apache/maven-compiler-plugin/pull/169
--
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] cstamas closed pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
cstamas closed pull request #151: Move away from legacy maven-compat
URL: https://github.com/apache/maven-compiler-plugin/pull/151
--
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] laeubi commented on a diff in pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
laeubi commented on code in PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#discussion_r980089524
##########
pom.xml:
##########
@@ -188,6 +188,7 @@ under the License.
<version>3.3.0</version>
<scope>test</scope>
</dependency>
+ <!-- maven-compat: is must due AbstractMojoTestCase :( -->
Review Comment:
I found that if you don not need to Inject parameters into the Mojo and thus do not test the Mojos run method, one can also go with PlexusTestcase see:
https://github.com/eclipse-tycho/tycho/blob/fdda7227e6cd714c2e120f70dd624cde2cca4ce1/tycho-p2/tycho-p2-facade/src/test/java/org/eclipse/tycho/p2/facade/test/MetadataSerializableImplTest.java
Still, I think this is a bit of a "blind-spot" that there is not a JUnit5 / Non-Compat replacement for a MojoTestCase.
--
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] psiroky commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396717655
@cstamas @ajarmoniuk I'll take a look then. I think it will be a nice way way to get a bit more understanding how the dependency resolution stuff works. I would possibly reach out to @ajarmoniuk in case of any issues / questions (if that's ok).
--
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] cstamas commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396620806
Toss a coin then :coin:
I am currently unable to continue this PR, and I'd hate anyone being hindered by sitting and waiting for this PR to land...
--
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] cstamas commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396571932
Am fine if anyone (@psiroky ?) picks up these changes and continue with them (as there is no need for else branch for example), no need for any attribution, just go on with 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.
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] ajarmoniuk commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396612678
Hi, I can help as well. I've done similar jobs for versions, enforcer and mrm already.
--
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] laeubi commented on a diff in pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
laeubi commented on code in PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#discussion_r985641086
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1841,29 +1838,36 @@ private List<String> resolveProcessorPathEntries()
ArtifactHandler handler = artifactHandlerManager.getArtifactHandler( coord.getType() );
Artifact artifact = new DefaultArtifact(
- coord.getGroupId(),
- coord.getArtifactId(),
- VersionRange.createFromVersionSpec( coord.getVersion() ),
- Artifact.SCOPE_RUNTIME,
- coord.getType(),
- coord.getClassifier(),
- handler,
- false );
-
- ArtifactResolutionRequest request = new ArtifactResolutionRequest()
- .setArtifact( artifact )
- .setResolveRoot( true )
- .setResolveTransitively( true )
- .setLocalRepository( session.getLocalRepository() )
- .setRemoteRepositories( project.getRemoteArtifactRepositories() );
-
- ArtifactResolutionResult resolutionResult = repositorySystem.resolve( request );
-
- resolutionErrorHandler.throwErrors( request, resolutionResult );
-
- for ( Artifact resolved : resolutionResult.getArtifacts() )
+ coord.getGroupId(),
+ coord.getArtifactId(),
+ coord.getClassifier(),
+ handler.getExtension(),
+ coord.getVersion()
+ );
+
+ CollectRequest collectRequest = new CollectRequest( new Dependency( artifact, JavaScopes.RUNTIME ),
+ project.getRemoteProjectRepositories() );
+ DependencyRequest dependencyRequest = new DependencyRequest();
+ dependencyRequest.setCollectRequest( collectRequest );
+ DependencyResult dependencyResult = repositorySystem.resolveDependencies(
+ session.getRepositorySession(), dependencyRequest );
+
+ ArrayList<ArtifactResult> failed = new ArrayList<>();
+ for ( ArtifactResult resolved : dependencyResult.getArtifactResults() )
+ {
+ if ( resolved.getArtifact() != null && resolved.getArtifact().getFile() != null )
+ {
+ elements.add( resolved.getArtifact().getFile().getAbsolutePath() );
+ }
+ else
+ {
+ failed.add( resolved );
Review Comment:
Can this actually happen or would this not just throw a `DependencyResolutionException` already?
--
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] psiroky commented on a diff in pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#discussion_r1080568677
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1841,29 +1838,36 @@ private List<String> resolveProcessorPathEntries()
ArtifactHandler handler = artifactHandlerManager.getArtifactHandler( coord.getType() );
Artifact artifact = new DefaultArtifact(
- coord.getGroupId(),
- coord.getArtifactId(),
- VersionRange.createFromVersionSpec( coord.getVersion() ),
- Artifact.SCOPE_RUNTIME,
- coord.getType(),
- coord.getClassifier(),
- handler,
- false );
-
- ArtifactResolutionRequest request = new ArtifactResolutionRequest()
- .setArtifact( artifact )
- .setResolveRoot( true )
- .setResolveTransitively( true )
- .setLocalRepository( session.getLocalRepository() )
- .setRemoteRepositories( project.getRemoteArtifactRepositories() );
-
- ArtifactResolutionResult resolutionResult = repositorySystem.resolve( request );
-
- resolutionErrorHandler.throwErrors( request, resolutionResult );
-
- for ( Artifact resolved : resolutionResult.getArtifacts() )
+ coord.getGroupId(),
+ coord.getArtifactId(),
+ coord.getClassifier(),
+ handler.getExtension(),
+ coord.getVersion()
+ );
+
+ CollectRequest collectRequest = new CollectRequest( new Dependency( artifact, JavaScopes.RUNTIME ),
+ project.getRemoteProjectRepositories() );
+ DependencyRequest dependencyRequest = new DependencyRequest();
+ dependencyRequest.setCollectRequest( collectRequest );
+ DependencyResult dependencyResult = repositorySystem.resolveDependencies(
+ session.getRepositorySession(), dependencyRequest );
+
+ ArrayList<ArtifactResult> failed = new ArrayList<>();
+ for ( ArtifactResult resolved : dependencyResult.getArtifactResults() )
+ {
+ if ( resolved.getArtifact() != null && resolved.getArtifact().getFile() != null )
+ {
+ elements.add( resolved.getArtifact().getFile().getAbsolutePath() );
+ }
+ else
+ {
+ failed.add( resolved );
Review Comment:
I believe you are right. If the one the dependencies cannot be resolved, `DependencyResolutionException` is thrown on line 1858, so the manual handling of failed artifacts should no be needed?
```
Caused by: org.apache.maven.plugin.MojoExecutionException: Resolution of annotationProcessorPath dependencies failed: Could not find artifact org.issue:annotation-processor2:jar:1.0-SNAPSHOT in local.central (file:///home/psiroky/.m2/repository)
at org.apache.maven.plugin.compiler.AbstractCompilerMojo.resolveProcessorPathEntries(AbstractCompilerMojo.java:1884)
at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:792)
at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:205)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:370)
... 20 common frames omitted
Caused by: org.eclipse.aether.resolution.DependencyResolutionException: Could not find artifact org.issue:annotation-processor2:jar:1.0-SNAPSHOT in local.central (file:///home/psiroky/.m2/repository)
at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies(DefaultRepositorySystem.java:374)
at org.apache.maven.plugin.compiler.AbstractCompilerMojo.resolveProcessorPathEntries(AbstractCompilerMojo.java:1858)
... 24 common frames omitted
Caused by: org.eclipse.aether.resolution.ArtifactResolutionException: Could not find artifact org.issue:annotation-processor2:jar:1.0-SNAPSHOT in local.central (file:///home/psiroky/.m2/repository)
at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:431)
at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:235)
at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveDependencies(DefaultRepositorySystem.java:357)
... 25 common frames omitted
Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact org.issue:annotation-processor2:jar:1.0-SNAPSHOT in local.central (file:///home/psiroky/.m2/repository)
at org.eclipse.aether.connector.basic.ArtifactTransportListener.transferFailed(ArtifactTransportListener.java:48)
at org.eclipse.aether.connector.basic.BasicRepositoryConnector$TaskRunner.run(BasicRepositoryConnector.java:369)
at org.eclipse.aether.util.concurrency.RunnableErrorForwarder.lambda$wrap$0(RunnableErrorForwarder.java:73)
at org.eclipse.aether.connector.basic.BasicRepositoryConnector$DirectExecutor.execute(BasicRepositoryConnector.java:627)
at org.eclipse.aether.connector.basic.BasicRepositoryConnector.get(BasicRepositoryConnector.java:262)
at org.eclipse.aether.internal.impl.DefaultArtifactResolver.performDownloads(DefaultArtifactResolver.java:520)
at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:408)
```
--
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] psiroky commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396108191
@cstamas @laeubi Hello! It would be great to get this one merged. Is there anything I can help with?
It seems like getting rid of `AbstractMojoTestCase` might be bigger endeavor and possibly outside of scope here? `CompilerMojoTestCase` is using quite a few method from there, so either we would need to (temporarily?) copy those directly into this class, or completely re-work the test to not rely on those.
To clarify why I am even commenting here: I would like to take a look at some of the bugs / feature requests related to `annotationProcessorPaths`, so having this one merged seems like a good per-requisite.
--
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] ajarmoniuk commented on pull request #151: Move away from legacy maven-compat
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #151:
URL: https://github.com/apache/maven-compiler-plugin/pull/151#issuecomment-1396701600
@psiroky let me know if you want to do it, otherwise I'll do 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org