Expand robustness of isPlugin against unbuildable models (#699)

* Expand robustness of `isPlugin` against unbuildable models

* Javadoc mistake

* IT for `isPlugin`

pom.xml

@@ -343,6 +343,9 @@
             <!-- block anything set by a CI environment -->
             <JENKINS_HOME />
           </environmentVariables>
+          <setupIncludes>
+            <setupInclude>somedep-*/pom.xml</setupInclude>
+          </setupIncludes>
         </configuration>
       </plugin>
       <plugin>

src/it/override-test-dependencies-split-plugin/invoker.properties

@@ -0,0 +1 @@
+invoker.goals=-ntp -DoverrideVersions=io.jenkins.plugins:somedep:2 -Djenkins.version=2.489 test

src/it/override-test-dependencies-split-plugin/pom.xml

@@ -0,0 +1,38 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.jenkins-ci.plugins</groupId>
+    <artifactId>plugin</artifactId>
+    <version>5.3</version>
+    <relativePath />
+  </parent>
+  <groupId>org.jenkins-ci.tools.hpi.its</groupId>
+  <artifactId>override-test-dependencies-smokes</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <packaging>hpi</packaging>
+  <properties>
+    <jenkins.version>2.479.2</jenkins.version>
+    <hpi-plugin.version>@project.version@</hpi-plugin.version>
+    <maven.test.redirectTestOutputToFile>false</maven.test.redirectTestOutputToFile>
+  </properties>
+  <dependencies>
+    <dependency>
+      <groupId>io.jenkins.plugins</groupId>
+      <artifactId>somedep</artifactId>
+      <version>1</version>
+    </dependency>
+  </dependencies>
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
+  <pluginRepositories>
+    <pluginRepository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </pluginRepository>
+  </pluginRepositories>
+</project>

src/it/override-test-dependencies-split-plugin/src/main/resources/index.jelly

@@ -0,0 +1,2 @@
+<?jelly escape-by-default='true'?>
+<div/>

src/it/override-test-dependencies-split-plugin/src/test/java/sample/SampleTest.java

@@ -0,0 +1,16 @@
+package sample;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import io.jenkins.plugins.sample.Util;
+import org.junit.Test;
+
+public class SampleTest {
+
+    @Test
+    public void smokes() throws Exception {
+        // just load some org.apache.commons.compress types:
+        assertThat(Util.x(), is(259L));
+    }
+}

src/it/somedep-1/invoker.properties

@@ -0,0 +1 @@
+invoker.goals=-ntp -P-might-produce-incrementals -Pquick-build install

src/it/somedep-1/pom.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.jenkins-ci.plugins</groupId>
+    <artifactId>plugin</artifactId>
+    <version>5.3</version>
+    <relativePath/>
+  </parent>
+  <groupId>io.jenkins.plugins</groupId>
+  <artifactId>somedep</artifactId>
+  <version>1</version>
+  <packaging>hpi</packaging>
+  <properties>
+    <jenkins.version>2.479.2</jenkins.version>
+  </properties>
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-compress</artifactId>
+      <scope>provided</scope>
+    </dependency>
+  </dependencies>
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
+  <pluginRepositories>
+    <pluginRepository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </pluginRepository>
+  </pluginRepositories>
+</project>

src/it/somedep-1/src/main/java/io/jenkins/plugins/sample/Util.java

@@ -0,0 +1,13 @@
+package io.jenkins.plugins.sample;
+
+import org.apache.commons.compress.utils.ByteUtils;
+
+public class Util {
+
+    public static long x() {
+        return ByteUtils.fromLittleEndian(new byte[] {3, 1}); // ⇒ 259
+    }
+
+    private Util() {}
+
+}

src/it/somedep-1/src/main/resources/index.jelly

@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<div>
+    TODO
+</div>

src/it/somedep-2/invoker.properties

@@ -0,0 +1 @@
+invoker.goals=-ntp -P-might-produce-incrementals -Pquick-build install

src/it/somedep-2/pom.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.jenkins-ci.plugins</groupId>
+    <artifactId>plugin</artifactId>
+    <version>5.3</version>
+    <relativePath/>
+  </parent>
+  <groupId>io.jenkins.plugins</groupId>
+  <artifactId>somedep</artifactId>
+  <version>2</version>
+  <packaging>hpi</packaging>
+  <properties>
+    <jenkins.version>2.479.2</jenkins.version>
+  </properties>
+  <dependencies>
+    <dependency>
+      <groupId>io.jenkins.plugins</groupId>
+      <artifactId>commons-compress-api</artifactId>
+      <version>1.26.1-2</version>
+    </dependency>
+  </dependencies>
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
+  <pluginRepositories>
+    <pluginRepository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </pluginRepository>
+  </pluginRepositories>
+</project>

src/it/somedep-2/src/main/java/io/jenkins/plugins/sample/Util.java

@@ -0,0 +1,13 @@
+package io.jenkins.plugins.sample;
+
+import org.apache.commons.compress.utils.ByteUtils;
+
+public class Util {
+
+    public static long x() {
+        return ByteUtils.fromLittleEndian(new byte[] {3, 1}); // ⇒ 259
+    }
+
+    private Util() {}
+
+}

src/it/somedep-2/src/main/resources/index.jelly

@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<div>
+    TODO
+</div>

src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java

@@ -458,7 +458,7 @@ public void buildWebapp(MavenProject project, File webappDirectory) throws MojoE
         Set<String> jenkinsPlugins = new HashSet<>();
         Set<String> excludedArtifacts = new HashSet<>();
         for (MavenArtifact artifact : Utils.unionOf(artifacts, dependencyArtifacts)) {
-            if (artifact.isPluginBestEffort(getLog())) {
+            if (artifact.isPlugin(getLog())) {
                 jenkinsPlugins.add(artifact.getId());
             }
             // Exclude dependency if it comes from test or provided trail.

src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractJenkinsManifestMojo.java

@@ -263,7 +263,7 @@ protected void setAttributes(Manifest.ExistingSection mainSection)
     private String findDependencyPlugins() throws IOException, MojoExecutionException {
         StringBuilder buf = new StringBuilder();
         for (MavenArtifact a : getDirectDependencyArtfacts()) {
-            if (a.isPlugin() && scopeFilter.include(a.artifact) && !a.hasSameGAAs(project)) {
+            if (a.isPlugin(getLog()) && scopeFilter.include(a.artifact) && !a.hasSameGAAs(project)) {
                 if (buf.length() > 0) {
                     buf.append(',');
                 }
@@ -280,7 +280,7 @@ private String findDependencyPlugins() throws IOException, MojoExecutionExceptio
         // see
         // http://jenkins-ci.361315.n4.nabble.com/Classloading-problem-when-referencing-classes-from-another-plugin-during-the-initialization-phase-of-td394967.html
         for (Artifact a : project.getDependencyArtifacts()) {
-            if ("provided".equals(a.getScope()) && wrap(a).isPlugin()) {
+            if ("provided".equals(a.getScope()) && wrap(a).isPlugin(getLog())) {
                 throw new MojoExecutionException(
                         a.getId() + " is marked as 'provided' scope dependency, but it should be the 'compile' scope.");
             }

src/main/java/org/jenkinsci/maven/plugins/hpi/AssembleDependenciesMojo.java

@@ -68,13 +68,9 @@ protected boolean accept(DependencyNode g) {
             return false; // cut off optional dependencies
         }
 
-        try {
-            if (!a.isPlugin()) {
-                // only traverse chains of direct plugin dependencies, unless it's from the root
-                return g.getParent() == null;
-            }
-        } catch (IOException e) {
-            getLog().warn("Failed to process " + a, e);
+        if (!a.isPlugin(getLog())) {
+            // only traverse chains of direct plugin dependencies, unless it's from the root
+            return g.getParent() == null;
         }
 
         MavenArtifact v = hpis.get(a.getArtifactId());

src/main/java/org/jenkinsci/maven/plugins/hpi/HplMojo.java

@@ -119,7 +119,7 @@ private void buildLibraries(List<String> paths) throws IOException, MojoExecutio
         // List up IDs of Jenkins plugin dependencies
         Set<String> jenkinsPlugins = new HashSet<>();
         for (MavenArtifact artifact : artifacts) {
-            if (artifact.isPluginBestEffort(getLog())) {
+            if (artifact.isPlugin(getLog())) {
                 jenkinsPlugins.add(artifact.getId());
             }
         }

src/main/java/org/jenkinsci/maven/plugins/hpi/ListPluginDependenciesMojo.java

@@ -36,7 +36,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                 ? new NullWriter()
                 : new OutputStreamWriter(new FileOutputStream(outputFile), StandardCharsets.UTF_8)) {
             for (MavenArtifact a : getDirectDependencyArtfacts()) {
-                if (!a.isPlugin()) {
+                if (!a.isPlugin(getLog())) {
                     continue;
                 }
 

src/main/java/org/jenkinsci/maven/plugins/hpi/MavenArtifact.java

@@ -72,27 +72,40 @@ public MavenProject resolvePom() throws ProjectBuildingException {
 
     /**
      * Is this a Jenkins plugin?
+     * The preferred check is for {@code hpi} or {@code jpi} packaging.
+     * If the project model cannot be resolved
+     * (for example when an indirect dependency has a bogus {@code systemPath} that is only rejected in some environments)
+     * then the fallback logic is to look for a JAR manifest with entries typical of plugins.
      */
-    public boolean isPlugin() throws IOException {
-        String type = getResolvedType();
-        return type.equals("hpi") || type.equals("jpi");
-    }
-
-    /**
-     * Like {@link #isPlugin} but will not throw an exception if the project model cannot be resolved.
-     * Helpful for example when an indirect dependency has a bogus {@code systemPath} that is only rejected in some environments.
-     */
-    public boolean isPluginBestEffort(Log log) {
+    public boolean isPlugin(Log log) {
         try {
-            return isPlugin();
+            String type = getResolvedType();
+            return type.equals("hpi") || type.equals("jpi");
         } catch (IOException x) {
             if (log.isDebugEnabled()) {
                 log.debug(x);
             } else {
-                log.warn(x.getCause().getMessage());
+                log.warn("While inspecting " + artifact + ": " + x.getCause().getMessage());
             }
-            return false;
         }
+        var f = artifact.getFile();
+        if (f.getName().endsWith(".jar") && f.isFile()) {
+            try (var jf = new JarFile(f)) {
+                var mani = jf.getManifest();
+                if (mani != null) {
+                    var attr = mani.getMainAttributes();
+                    return attr.getValue("Jenkins-Version") != null && attr.getValue("Plugin-Version") != null;
+                }
+            } catch (IOException x) {
+                if (log.isDebugEnabled()) {
+                    log.debug(x);
+                } else {
+                    log.warn(
+                            "While inspecting " + artifact + ": " + x.getCause().getMessage());
+                }
+            }
+        }
+        return false;
     }
 
     public String getId() {

src/main/java/org/jenkinsci/maven/plugins/hpi/RunMojo.java

@@ -373,7 +373,7 @@ protected ClassLoader configureClassLoader(ClassLoader loader) {
         // copy other dependency Jenkins plugins
         try {
             for (MavenArtifact a : getProjectArtifacts()) {
-                if (!a.isPluginBestEffort(getLog())) {
+                if (!a.isPlugin(getLog())) {
                     continue;
                 }
 

src/main/java/org/jenkinsci/maven/plugins/hpi/TestDependencyMojo.java

@@ -411,7 +411,7 @@ private void copyTestDependencies(Set<MavenArtifact> mavenArtifacts) throws Mojo
 
         List<ArtifactRequest> artifactRequests = new ArrayList<>();
         for (MavenArtifact mavenArtifact : mavenArtifacts) {
-            if (!mavenArtifact.isPluginBestEffort(getLog())) {
+            if (!mavenArtifact.isPlugin(getLog())) {
                 continue;
             }
 

src/main/java/org/jenkinsci/maven/plugins/hpi/ValidateHpiMojo.java

@@ -29,7 +29,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
 
         for (MavenArtifact artifact : Utils.unionOf(getProjectArtfacts(), getDirectDependencyArtfacts())) {
             try {
-                if (artifact.isPluginBestEffort(getLog())) {
+                if (artifact.isPlugin(getLog())) {
                     VersionNumber dependencyCoreVersion = getDependencyCoreVersion(artifact);
                     if (dependencyCoreVersion.compareTo(maxCoreVersion) > 0) {
                         maxCoreVersionArtifact = artifact;