From 99f552ba1ebb29d4d2e3344c106958666f7c113f Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 6 Feb 2024 10:58:11 +0100 Subject: [PATCH] Incomplete runtime classpath fix Similar fix to m-assembly-p: do not rely on Maven Core to provide "runtime" resolution scope as it have issues, see: https://issues.apache.org/jira/browse/MNG-8041 Instead, use Resolver APIs directly to get what is needed. Contains several minor code improvements across Provisio maven-plugin module. Contains IT from #70 Fixes #71 --- provisio-maven-plugin/pom.xml | 5 + .../maven/plugins/provisio/BaseMojo.java | 94 ++++++++++--------- .../maven/plugins/provisio/GeneratorMojo.java | 12 +-- .../maven/plugins/provisio/Provisio.java | 12 +-- .../plugins/provisio/ProvisioningMojo.java | 2 +- .../maven/plugins/provisio/ValidatorMojo.java | 1 + .../provisio/GeneratorIntegrationTest.java | 2 +- .../provisio/ProvisioningIntegrationTest.java | 60 ++++++++++++ .../provisio/ValidatorIntegrationTest.java | 2 +- .../src/test/projects/transitive-test/pom.xml | 37 ++++++++ .../src/main/provisio/provisio.xml | 3 + 11 files changed, 166 insertions(+), 64 deletions(-) create mode 100644 provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ProvisioningIntegrationTest.java create mode 100644 provisio-maven-plugin/src/test/projects/transitive-test/pom.xml create mode 100644 provisio-maven-plugin/src/test/projects/transitive-test/src/main/provisio/provisio.xml diff --git a/provisio-maven-plugin/pom.xml b/provisio-maven-plugin/pom.xml index fa9934d..7f2a6b7 100644 --- a/provisio-maven-plugin/pom.xml +++ b/provisio-maven-plugin/pom.xml @@ -108,6 +108,11 @@ maven-resolver-impl provided + + org.apache.maven.resolver + maven-resolver-util + + io.takari.maven.plugins diff --git a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/BaseMojo.java b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/BaseMojo.java index fe21afc..a662fe1 100644 --- a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/BaseMojo.java +++ b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/BaseMojo.java @@ -21,27 +21,24 @@ import ca.vanzyl.provisio.model.Runtime; import io.takari.incrementalbuild.Incremental; import io.takari.incrementalbuild.Incremental.Configuration; -import org.apache.maven.artifact.handler.ArtifactHandler; -import org.apache.maven.execution.MavenSession; +import org.apache.maven.RepositoryUtils; import org.apache.maven.model.Dependency; import org.apache.maven.model.Model; -import org.apache.maven.model.io.xpp3.MavenXpp3Reader; import org.apache.maven.plugin.AbstractMojo; -import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.project.MavenProject; import org.apache.maven.project.MavenProjectHelper; -import org.codehaus.plexus.util.IOUtil; -import org.codehaus.plexus.util.ReaderFactory; -import org.codehaus.plexus.util.xml.pull.XmlPullParserException; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; -import org.eclipse.aether.artifact.ArtifactProperties; -import org.eclipse.aether.artifact.ArtifactType; -import org.eclipse.aether.artifact.DefaultArtifact; -import org.eclipse.aether.artifact.DefaultArtifactType; +import org.eclipse.aether.collection.CollectRequest; +import org.eclipse.aether.graph.DefaultDependencyNode; +import org.eclipse.aether.graph.DependencyFilter; +import org.eclipse.aether.resolution.ArtifactResult; +import org.eclipse.aether.resolution.DependencyRequest; +import org.eclipse.aether.resolution.DependencyResolutionException; +import org.eclipse.aether.util.filter.ScopeDependencyFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,20 +80,22 @@ public abstract class BaseMojo @Parameter(required = true, defaultValue = "${basedir}/src/main/provisio") protected File descriptorDirectory; - @Parameter(defaultValue = "${session}") - protected MavenSession session; - protected ProvisioArtifact projectArtifact() { ProvisioArtifact jarArtifact = null; // // We also need to definitively know what others types of runtime artifacts have been created. Right now there // is no real protocol for knowing what something like, say, the JAR plugin did to drop off a file somewhere. We // need to improve this but for now we'll look. + // === + // While this above is still true, this check below is better, in a sense it allows JAR plugin to drop off file + // anywhere. // - File jar = new File(project.getBuild().getDirectory(), project.getArtifactId() + "-" + project.getVersion() + ".jar"); - if (jar.exists()) { + Artifact projectArtifact = RepositoryUtils.toArtifact(project.getArtifact()); + if (projectArtifact.getFile() != null + && projectArtifact.getFile().getName().endsWith(".jar") + && projectArtifact.getFile().exists()) { jarArtifact = new ProvisioArtifact(project.getGroupId() + ":" + project.getArtifactId() + ":" + project.getVersion()); - jarArtifact.setFile(jar); + jarArtifact.setFile(projectArtifact.getFile()); } return jarArtifact; } @@ -113,39 +112,44 @@ protected ArtifactSet getRuntimeClasspathAsArtifactSet() { // for this Mojo. I think this will be sufficient for anything related to creating a runtime. // ArtifactSet artifactSet = new ArtifactSet(); - for (org.apache.maven.artifact.Artifact mavenArtifact : project.getArtifacts()) { - if (!mavenArtifact.getScope().equals("system") && !mavenArtifact.getScope().equals("provided")) { - artifactSet.addArtifact(new ProvisioArtifact(toArtifact(mavenArtifact))); - } + for (Artifact mavenArtifact : resolveRuntimeScopeTransitively()) { + artifactSet.addArtifact(new ProvisioArtifact(mavenArtifact)); } return artifactSet; } - private static Artifact toArtifact(org.apache.maven.artifact.Artifact artifact) { - if (artifact == null) { - return null; - } - String version = artifact.getVersion(); - if (version == null && artifact.getVersionRange() != null) { - version = artifact.getVersionRange().toString(); + /** + * This method is in use instead of project offering mojo asked resolution scope due presence of: + * MNG-8041 + */ + private List resolveRuntimeScopeTransitively() { + DependencyFilter runtimeFilter = new ScopeDependencyFilter("system", "provided", "test"); + List dependencies = project.getDependencies().stream() + .map(d -> RepositoryUtils.toDependency(d, repositorySystemSession.getArtifactTypeRegistry())) + .filter(d -> runtimeFilter.accept(new DefaultDependencyNode(d), Collections.emptyList())) + .collect(Collectors.toList()); + List managedDependencies = Collections.emptyList(); + if (project.getDependencyManagement() != null) { + managedDependencies = project.getDependencyManagement().getDependencies().stream() + .map(d -> RepositoryUtils.toDependency(d, repositorySystemSession.getArtifactTypeRegistry())) + .collect(Collectors.toList()); } - Map props = null; - if (org.apache.maven.artifact.Artifact.SCOPE_SYSTEM.equals(artifact.getScope())) { - String localPath = (artifact.getFile() != null) ? artifact.getFile().getPath() : ""; - props = Collections.singletonMap(ArtifactProperties.LOCAL_PATH, localPath); + CollectRequest collectRequest = new CollectRequest(); + collectRequest.setRootArtifact(RepositoryUtils.toArtifact(project.getArtifact())); + collectRequest.setRepositories(project.getRemoteProjectRepositories()); + collectRequest.setDependencies(dependencies); + collectRequest.setManagedDependencies(managedDependencies); + DependencyRequest request = new DependencyRequest(collectRequest, runtimeFilter); + try { + return repositorySystem.resolveDependencies(repositorySystemSession, request).getArtifactResults().stream() + .map(ArtifactResult::getArtifact) + .collect(Collectors.toList()); + } catch (DependencyResolutionException e) { + logger.error("Failed to resolve runtime dependencies", e); + throw new RuntimeException(e); } - - Artifact result = new DefaultArtifact(artifact.getGroupId(), artifact.getArtifactId(), artifact.getClassifier(), artifact.getArtifactHandler().getExtension(), version, props, - newArtifactType(artifact.getType(), artifact.getArtifactHandler())); - result = result.setFile(artifact.getFile()); - - return result; - } - - private static ArtifactType newArtifactType(String id, ArtifactHandler handler) { - return new DefaultArtifactType(id, handler.getExtension(), handler.getClassifier(), handler.getLanguage(), handler.isAddedToClasspath(), handler.isIncludesDependencies()); } protected ProvisioningRequest getRequest(Runtime runtime) @@ -173,7 +177,7 @@ protected void checkDuplicates(List artifacts) .filter(strings -> strings.size() > 1) .map(strings -> String.join(", ", strings)) .collect(Collectors.toList()); - if (duplicates.size() != 0) { + if (!duplicates.isEmpty()) { throw new MojoFailureException("Found different versions of the same dependency: " + String.join(", ", duplicates)); } } @@ -186,10 +190,10 @@ protected List getDependencies(List artifacts) dependency.setGroupId(artifact.getGroupId()); dependency.setArtifactId(artifact.getArtifactId()); dependency.setVersion(artifact.getVersion()); - if (artifact.getClassifier() != null && artifact.getClassifier().length() != 0) { + if (artifact.getClassifier() != null && !artifact.getClassifier().isEmpty()) { dependency.setClassifier(artifact.getClassifier()); } - if (artifact.getExtension() != null && artifact.getExtension().length() != 0 && !artifact.getExtension().equals("jar")) { + if (artifact.getExtension() != null && !artifact.getExtension().isEmpty() && !artifact.getExtension().equals("jar")) { dependency.setType(artifact.getExtension()); } dependencies.add(dependency); diff --git a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/GeneratorMojo.java b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/GeneratorMojo.java index 7f83862..520a262 100644 --- a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/GeneratorMojo.java +++ b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/GeneratorMojo.java @@ -28,7 +28,6 @@ import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; import org.apache.maven.plugins.annotations.ResolutionScope; -import org.codehaus.plexus.util.IOUtil; import org.codehaus.plexus.util.WriterFactory; import java.io.File; @@ -48,6 +47,7 @@ public class GeneratorMojo @Parameter(property = "dependencyExtendedPomLocation", defaultValue = "${project.build.directory}/generated/provisio/dependency-extended-pom.xml") private File dependencyExtendedPomLocation; + @Override public void execute() throws MojoExecutionException, MojoFailureException { @@ -65,7 +65,7 @@ public void execute() throw new MojoExecutionException("Error resolving artifacts.", e); } } - if (artifacts.size() == 0) { + if (artifacts.isEmpty()) { return; } checkDuplicates(artifacts); @@ -85,17 +85,11 @@ private void writeModel(Model model) } catch (IOException e) { throw new MojoExecutionException("Error creating parent directories for the POM file: " + e.getMessage(), e); } - Writer writer = null; - try { - writer = WriterFactory.newXmlWriter(dependencyExtendedPomLocation); + try (Writer writer = WriterFactory.newXmlWriter(dependencyExtendedPomLocation)) { new MavenXpp3Writer().write(writer, model); - writer.close(); } catch (IOException e) { throw new MojoExecutionException("Error writing POM file: " + e.getMessage(), e); } - finally { - IOUtil.close(writer); - } } } diff --git a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/Provisio.java b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/Provisio.java index acd083b..42330ca 100644 --- a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/Provisio.java +++ b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/Provisio.java @@ -147,8 +147,8 @@ public List getManagedDependencies(MavenProject project) { } public String toCoordinate(Dependency d) { - StringBuffer sb = new StringBuffer().append(d.getGroupId()).append(":").append(d.getArtifactId()).append(":").append(d.getType()); - if (d.getClassifier() != null && d.getClassifier().isEmpty() == false) { + StringBuilder sb = new StringBuilder().append(d.getGroupId()).append(":").append(d.getArtifactId()).append(":").append(d.getType()); + if (d.getClassifier() != null && !d.getClassifier().isEmpty()) { sb.append(":").append(d.getClassifier()); } sb.append(":").append(d.getVersion()); @@ -156,8 +156,8 @@ public String toCoordinate(Dependency d) { } public String toVersionlessCoordinate(Dependency d) { - StringBuffer sb = new StringBuffer().append(d.getGroupId()).append(":").append(d.getArtifactId()).append(":").append(d.getType()); - if (d.getClassifier() != null && d.getClassifier().isEmpty() == false) { + StringBuilder sb = new StringBuilder().append(d.getGroupId()).append(":").append(d.getArtifactId()).append(":").append(d.getType()); + if (d.getClassifier() != null && !d.getClassifier().isEmpty()) { sb.append(":").append(d.getClassifier()); } return sb.toString(); @@ -165,8 +165,6 @@ public String toVersionlessCoordinate(Dependency d) { public String toVersionlessCoordinate(MavenProject project) { String extension = artifactHandlerManager.getArtifactHandler(project.getPackaging()).getExtension(); - StringBuffer sb = new StringBuffer().append(project.getGroupId()).append(":").append(project.getArtifactId()).append(":").append(extension); - return sb.toString(); + return project.getGroupId() + ":" + project.getArtifactId() + ":" + extension; } - } diff --git a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ProvisioningMojo.java b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ProvisioningMojo.java index a42ae2e..5387232 100644 --- a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ProvisioningMojo.java +++ b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ProvisioningMojo.java @@ -24,7 +24,6 @@ import ca.vanzyl.provisio.model.ProvisioningResult; import ca.vanzyl.provisio.model.Runtime; import org.apache.maven.plugin.MojoExecutionException; -import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugins.annotations.LifecyclePhase; import org.apache.maven.plugins.annotations.Mojo; import org.apache.maven.plugins.annotations.Parameter; @@ -41,6 +40,7 @@ public class ProvisioningMojo extends BaseMojo { @Parameter(defaultValue = "${project.build.directory}/${project.artifactId}-${project.version}") private File outputDirectory; + @Override public void execute() throws MojoExecutionException { if (skipProvision) { getLog().info("Skipping provision"); diff --git a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ValidatorMojo.java b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ValidatorMojo.java index 4cef34a..94e662a 100644 --- a/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ValidatorMojo.java +++ b/provisio-maven-plugin/src/main/java/ca/vanzyl/maven/plugins/provisio/ValidatorMojo.java @@ -44,6 +44,7 @@ public class ValidatorMojo @Parameter(required = true, property = "pomFile", defaultValue = "${basedir}/pom.xml") private File pomFile; + @Override public void execute() throws MojoExecutionException, MojoFailureException { diff --git a/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/GeneratorIntegrationTest.java b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/GeneratorIntegrationTest.java index 0a8a01b..a151e54 100644 --- a/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/GeneratorIntegrationTest.java +++ b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/GeneratorIntegrationTest.java @@ -41,7 +41,7 @@ import static org.junit.Assert.assertArrayEquals; @RunWith(MavenJUnitTestRunner.class) -@MavenVersions({"3.6.3", "3.8.4"}) +@MavenVersions({"3.6.3", "3.8.8", "3.9.6"}) @SuppressWarnings({"JUnitTestNG", "PublicField"}) public class GeneratorIntegrationTest { diff --git a/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ProvisioningIntegrationTest.java b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ProvisioningIntegrationTest.java new file mode 100644 index 0000000..7091152 --- /dev/null +++ b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ProvisioningIntegrationTest.java @@ -0,0 +1,60 @@ +/** + * Copyright (C) 2015-2020 Jason van Zyl + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ca.vanzyl.maven.plugins.provisio; + +import io.takari.maven.testing.TestResources; +import io.takari.maven.testing.executor.MavenRuntime; +import io.takari.maven.testing.executor.MavenRuntime.MavenRuntimeBuilder; +import io.takari.maven.testing.executor.MavenVersions; +import io.takari.maven.testing.executor.junit.MavenJUnitTestRunner; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.File; + +import static org.junit.Assert.assertTrue; + +@RunWith(MavenJUnitTestRunner.class) +@MavenVersions({"3.6.3", "3.8.8", "3.9.6"}) +@SuppressWarnings({"JUnitTestNG", "PublicField"}) +public class ProvisioningIntegrationTest +{ + @Rule + public final TestResources resources = new TestResources(); + + public final MavenRuntime maven; + + public ProvisioningIntegrationTest(MavenRuntimeBuilder mavenBuilder) + throws Exception + { + this.maven = mavenBuilder.withCliOptions("-B", "-U").build(); + } + + @Test + public void testTransitiveWithLocalTestScope() + throws Exception + { + File basedir = resources.getBasedir("transitive-test"); + maven.forProject(basedir) + .execute("provisio:provision") + .assertErrorFreeLog(); + + File libdir = new File(basedir, "target/test-1.0/lib"); + assertTrue("guice exists", new File(libdir, "guice-7.0.0.jar").isFile()); + assertTrue("guava exists", new File(libdir, "guava-31.0.1-jre.jar").isFile()); + } +} diff --git a/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ValidatorIntegrationTest.java b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ValidatorIntegrationTest.java index 399d946..003da68 100644 --- a/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ValidatorIntegrationTest.java +++ b/provisio-maven-plugin/src/test/java/ca/vanzyl/maven/plugins/provisio/ValidatorIntegrationTest.java @@ -27,7 +27,7 @@ import java.io.File; @RunWith(MavenJUnitTestRunner.class) -@MavenVersions({"3.6.3", "3.8.4"}) +@MavenVersions({"3.6.3", "3.8.8", "3.9.6"}) @SuppressWarnings({"JUnitTestNG", "PublicField"}) public class ValidatorIntegrationTest { diff --git a/provisio-maven-plugin/src/test/projects/transitive-test/pom.xml b/provisio-maven-plugin/src/test/projects/transitive-test/pom.xml new file mode 100644 index 0000000..05cad66 --- /dev/null +++ b/provisio-maven-plugin/src/test/projects/transitive-test/pom.xml @@ -0,0 +1,37 @@ + + + 4.0.0 + ca.vanzyl.provisio.maven.plugins.its + test + 1.0 + provisio + + + UTF-8 + + + + + com.google.inject + guice + 7.0.0 + + + com.google.guava + guava + 31.0.1-jre + test + + + + + + + ca.vanzyl.provisio.maven.plugins + provisio-maven-plugin + ${it-plugin.version} + true + + + + diff --git a/provisio-maven-plugin/src/test/projects/transitive-test/src/main/provisio/provisio.xml b/provisio-maven-plugin/src/test/projects/transitive-test/src/main/provisio/provisio.xml new file mode 100644 index 0000000..05d7faa --- /dev/null +++ b/provisio-maven-plugin/src/test/projects/transitive-test/src/main/provisio/provisio.xml @@ -0,0 +1,3 @@ + + +