Move away from legacy maven-compat #151
Conversation
And switch to resolver to resolver annotation processors with dependencies.
| @@ -188,6 +188,7 @@ under the License. | |||
| <version>3.3.0</version> | |||
| <scope>test</scope> | |||
| </dependency> | |||
| <!-- maven-compat: is must due AbstractMojoTestCase :( --> | |||
There was a problem hiding this 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.
| } | ||
| else | ||
| { | ||
| failed.add( resolved ); |
There was a problem hiding this comment.
Can this actually happen or would this not just throw a DependencyResolutionException already?
There was a problem hiding this 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)
There was a problem hiding this comment.
Correct. In addition, result.getExceptions() will contain the list of all gathered exceptions (if there are more than one).
|
@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 To clarify why I am even commenting here: I would like to take a look at some of the bugs / feature requests related to Edit: Just FYI - I tried to locally rebase on top of the latest master and all the tests are passing, so the change still looks good. |
|
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. |
|
Hi, I can help as well. I've done similar jobs for versions, enforcer and mrm already. |
|
Toss a coin then 🪙 I am currently unable to continue this PR, and I'd hate anyone being hindered by sitting and waiting for this PR to land... |
|
@psiroky let me know if you want to do it, otherwise I'll do it |
|
@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). |
…hs' dependencies * heavily based on work by @cstamas (PR apache#151) * added IT to verify the build fails if one the dependencies cannot be resolved (does not exist)
…hs' dependencies * heavily based on work by @cstamas (PR apache#151) * added IT to verify the build fails if one the dependencies cannot be resolved (does not exist)
…hs' dependencies * heavily based on work by @cstamas (PR apache#151) * added IT to verify the build fails if one the dependencies cannot be resolved (does not exist)
|
Superseded by #169 |
…hs' dependencies * heavily based on work by @cstamas (PR apache#151) * added IT to verify the build fails if one the dependencies cannot be resolved (does not exist)
And switch to resolver to resolver annotation processors with dependencies.