[MINSTALL-196] Consider packaging in install-file mojo (#81)

Bug fix and adjust some fluke ITs. Changes: * honor packaging to calculate classifier (optionally) and extension * add param "extension" to override Maven calculated extension explicitly * switch to SLF4J logging --- https://issues.apache.org/jira/browse/MINSTALL-196
apache · Aug 15, 2024 · 49abeeb · 49abeeb
1 parent 30ca28e
commit 49abeeb
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 26 deletions.
diff --git a/pom.xml b/pom.xml
@@ -104,7 +104,6 @@
       <version>${mavenVersion}</version>
       <scope>provided</scope>
     </dependency>
-    <!-- This is here to override 3.0 coming with maven-artifact-transfer -->
     <dependency>
       <groupId>org.apache.maven</groupId>
       <artifactId>maven-core</artifactId>
@@ -130,6 +129,12 @@
       <!-- To work in Maven versions older than 3.9.0 -->
       <scope>compile</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4jVersion}</version>
+      <scope>provided</scope>
+    </dependency>
     <dependency>
       <groupId>org.codehaus.plexus</groupId>
       <artifactId>plexus-utils</artifactId>
@@ -171,12 +176,6 @@
       <version>4.8.1</version>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-api</artifactId>
-      <version>${slf4jVersion}</version>
-      <scope>provided</scope>
-    </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-nop</artifactId>

diff --git a/src/it/install-file-minstall-121-bundle/test.properties b/src/it/install-file-minstall-121-bundle/test.properties
@@ -18,3 +18,4 @@
 file = test-0.1.jar
 pomFile = test-0.1.pom
 packaging = bundle
+extension = jar
diff --git a/src/it/install-file-minstall-121-bundle/verify.bsh b/src/it/install-file-minstall-121-bundle/verify.bsh
@@ -24,7 +24,7 @@ String[] paths =
 {
     "org/apache/maven/its/install/121/maven-bundle/maven-metadata-local.xml",
     "org/apache/maven/its/install/121/maven-bundle/1.0/maven-bundle-1.0.pom",
-    "org/apache/maven/its/install/121/maven-bundle/1.0/maven-bundle-1.0.jar",
+    "org/apache/maven/its/install/121/maven-bundle/1.0/maven-bundle-1.0.jar", // packaging bundle is NOT in core! (explicitly set with -Dextension=jar)
 };
 
 for ( String path : paths )

diff --git a/src/it/install-file-minstall-121-java-source/verify.bsh b/src/it/install-file-minstall-121-java-source/verify.bsh
@@ -24,7 +24,7 @@ String[] paths =
 {
     "org/apache/maven/its/install/121/test-java-source/maven-metadata-local.xml",
     "org/apache/maven/its/install/121/test-java-source/1.0/test-java-source-1.0.pom",
-    "org/apache/maven/its/install/121/test-java-source/1.0/test-java-source-1.0-sources.jar",
+    "org/apache/maven/its/install/121/test-java-source/1.0/test-java-source-1.0-sources.jar", // packaging java-source IS in core
 };
 
 for ( String path : paths )

diff --git a/src/it/install-file-minstall-121-maven-archetype/test.properties b/src/it/install-file-minstall-121-maven-archetype/test.properties
@@ -18,3 +18,4 @@
 file = test-0.1.jar
 pomFile = test-0.1.pom
 packaging = maven-archetype
+extension = jar
diff --git a/src/it/install-file-minstall-121-maven-archetype/verify.bsh b/src/it/install-file-minstall-121-maven-archetype/verify.bsh
@@ -24,7 +24,7 @@ String[] paths =
 {
     "org/apache/maven/its/install/121/maven-archetype/maven-metadata-local.xml",
     "org/apache/maven/its/install/121/maven-archetype/1.0/maven-archetype-1.0.pom",
-    "org/apache/maven/its/install/121/maven-archetype/1.0/maven-archetype-1.0.jar",
+    "org/apache/maven/its/install/121/maven-archetype/1.0/maven-archetype-1.0.jar",  // packaging maven-archetype is NOT in core! (explicitly set with -Dextension=jar)
 };
 
 for ( String path : paths )

diff --git a/src/it/install-file-minstall-121-targz/test.properties b/src/it/install-file-minstall-121-targz/test.properties
@@ -18,3 +18,4 @@
 file = test-0.1.tar.gz
 pomFile = test-0.1.pom
 packaging = war
+extension = tar.gz
diff --git a/src/it/install-file-minstall-121-targz/verify.bsh b/src/it/install-file-minstall-121-targz/verify.bsh
@@ -24,7 +24,7 @@ String[] paths =
 {
     "org/apache/maven/its/install/121/test-targz/maven-metadata-local.xml",
     "org/apache/maven/its/install/121/test-targz/1.0/test-targz-1.0.pom",
-    "org/apache/maven/its/install/121/test-targz/1.0/test-targz-1.0.tar.gz",
+    "org/apache/maven/its/install/121/test-targz/1.0/test-targz-1.0.tar.gz", // this test is totally fluke: deploy POM.packaging=war and main artifact tar.gz
 };
 
 for ( String path : paths )

diff --git a/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java
@@ -26,6 +26,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
@@ -43,6 +44,7 @@
 import org.apache.maven.plugins.annotations.Mojo;
 import org.apache.maven.plugins.annotations.Parameter;
 import org.codehaus.plexus.util.FileUtils;
+import org.codehaus.plexus.util.StringUtils;
 import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
 import org.eclipse.aether.DefaultRepositoryCache;
 import org.eclipse.aether.DefaultRepositorySystemSession;
@@ -56,9 +58,10 @@
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.repository.LocalRepositoryManager;
 import org.eclipse.aether.util.artifact.SubArtifact;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import static java.util.Objects.isNull;
-import static java.util.Objects.nonNull;
 
 /**
  * Installs a file in the local repository.
@@ -68,6 +71,7 @@
 @Mojo(name = "install-file", requiresProject = false, aggregator = true, threadSafe = true)
 public class InstallFileMojo extends AbstractMojo {
     private static final String LS = System.lineSeparator();
+    private final Logger log = LoggerFactory.getLogger(getClass());
 
     @Component
     private RepositorySystem repositorySystem;
@@ -112,6 +116,15 @@ public class InstallFileMojo extends AbstractMojo {
     @Parameter(property = "classifier")
     private String classifier;
 
+    /**
+     * Extension of the artifact to be installed. If set, will override plugin own logic to detect extension. If not set,
+     * as Maven expected, packaging determines the artifact extension.
+     *
+     * @since 3.1.3
+     */
+    @Parameter(property = "extension")
+    private String extension;
+
     /**
      * The file to be installed in the local repository.
      */
@@ -169,7 +182,7 @@ public class InstallFileMojo extends AbstractMojo {
     public void execute() throws MojoExecutionException, MojoFailureException {
         if (!file.exists()) {
             String message = "The specified file '" + file.getPath() + "' does not exist";
-            getLog().error(message);
+            log.error(message);
             throw new MojoFailureException(message);
         }
 
@@ -189,8 +202,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                     newSession, new LocalRepository(localRepositoryPath, contentType));
             newSession.setLocalRepositoryManager(localRepositoryManager);
             repositorySystemSession = newSession;
-            getLog().debug("localRepoPath: "
-                    + localRepositoryManager.getRepository().getBasedir());
+            log.debug(
+                    "localRepoPath: {}", localRepositoryManager.getRepository().getBasedir());
         }
 
         File temporaryPom = null;
@@ -199,7 +212,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
             temporaryPom = readingPomFromJarFile();
             if (!Boolean.TRUE.equals(generatePom)) {
                 pomFile = temporaryPom;
-                getLog().debug("Using JAR embedded POM as pomFile");
+                log.debug("Using JAR embedded POM as pomFile");
             }
         } else {
             processModel(readModel(pomFile));
@@ -216,16 +229,29 @@ public void execute() throws MojoExecutionException, MojoFailureException {
 
         InstallRequest installRequest = new InstallRequest();
 
-        boolean isFilePom = isNull(classifier) && IS_POM_PACKAGING.test(packaging);
-        if (!isFilePom) {
+        String mainArtifactExtension;
+        if (classifier == null && "pom".equals(packaging)) {
+            mainArtifactExtension = "pom";
+        } else {
             ArtifactType artifactType =
-                    repositorySystemSession.getArtifactTypeRegistry().get(packaging);
-            if (nonNull(artifactType) && IS_EMPTY.test(classifier) && !IS_EMPTY.test(artifactType.getClassifier())) {
-                classifier = artifactType.getClassifier();
+                    session.getRepositorySession().getArtifactTypeRegistry().get(packaging);
+            if (artifactType != null) {
+                if (StringUtils.isEmpty(classifier) && !StringUtils.isEmpty(artifactType.getClassifier())) {
+                    classifier = artifactType.getClassifier();
+                }
+                mainArtifactExtension = artifactType.getExtension();
+            } else {
+                mainArtifactExtension = packaging;
             }
         }
+        if (extension != null && !Objects.equals(extension, mainArtifactExtension)) {
+            log.warn(
+                    "Main artifact extension should be '{}' but was overridden to '{}'",
+                    mainArtifactExtension,
+                    extension);
+        }
         Artifact mainArtifact = new DefaultArtifact(
-                        groupId, artifactId, classifier, isFilePom ? "pom" : getExtension(file), version)
+                        groupId, artifactId, classifier, extension != null ? extension : mainArtifactExtension, version)
                 .setFile(file);
         installRequest.addArtifact(mainArtifact);
 
@@ -241,10 +267,10 @@ public void execute() throws MojoExecutionException, MojoFailureException {
             if (isNull(pomFile)) {
                 if (Boolean.TRUE.equals(generatePom) || (generatePom == null && !pomLocalFile.exists())) {
                     temporaryPom = generatePomFile();
-                    getLog().debug("Installing generated POM");
+                    log.debug("Installing generated POM");
                     installRequest.addArtifact(new SubArtifact(mainArtifact, "", "pom", temporaryPom));
                 } else if (generatePom == null) {
-                    getLog().debug("Skipping installation of generated POM, already present in local repository");
+                    log.debug("Skipping installation of generated POM, already present in local repository");
                 }
             } else {
                 installRequest.addArtifact(new SubArtifact(mainArtifact, "", "pom", pomFile));
@@ -289,15 +315,15 @@ private File readingPomFromJarFile() throws MojoExecutionException {
 
             if (isNull(pomEntry)) {
                 // This means there is no entry which matches the "pom.xml"...(or in other words: not packaged by Maven)
-                getLog().info("pom.xml not found in " + file.getName());
+                log.info("pom.xml not found in {}", file.getName());
                 return null;
             }
 
             Path tempPomFile = Files.createTempFile(base, ".pom");
 
             Files.copy(jarFile.getInputStream(pomEntry), tempPomFile, StandardCopyOption.REPLACE_EXISTING);
 
-            getLog().debug("Loading " + pomEntry.getName());
+            log.debug("Loading {}", pomEntry.getName());
             processModel(readModel(tempPomFile.toFile()));
             return tempPomFile.toFile();