Merge pull request #31747 from holly-cummins/read-scm-from-pom

gsmet · web-flow · commit d40800d9af58 · 2023-03-14T20:55:11.000+01:00
Read scm-url information from pom, where environment information is missing
diff --git a/devtools/gradle/gradle-extension-plugin/src/main/java/io/quarkus/extension/gradle/tasks/ExtensionDescriptorTask.java b/devtools/gradle/gradle-extension-plugin/src/main/java/io/quarkus/extension/gradle/tasks/ExtensionDescriptorTask.java
@@ -311,7 +311,7 @@ private void computeArtifactCoords(ObjectNode extObject) {
     }
 
     private void computeSourceLocation(ObjectNode extObject) {
-        Map<String, String> repo = ScmInfoProvider.getSourceRepo();
+        Map<String, String> repo = ScmInfoProvider.getSourceRepo(null);
         if (repo != null) {
             ObjectNode metadata = getMetadataNode(extObject);
 
diff --git a/independent-projects/extension-maven-plugin/pom.xml b/independent-projects/extension-maven-plugin/pom.xml
@@ -128,9 +128,6 @@
                         <!-- set tmpdir as early as possible because failsafe sets it too late for JDK16 -->
                         <argLine>-Djava.io.tmpdir="${project.build.directory}"</argLine>
                         <excludedEnvironmentVariables>MAVEN_OPTS</excludedEnvironmentVariables>
-                        <environmentVariables>
-                            <GITHUB_REPOSITORY>some/repo</GITHUB_REPOSITORY>
-                        </environmentVariables>
                     </configuration>
                 </plugin>
                 <plugin>
@@ -300,6 +297,19 @@
             <version>3.8.7</version>
             <scope>test</scope>
         </dependency>
+        <!-- Needed for system stubs -->
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>5.1.1</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>uk.org.webcompere</groupId>
+            <artifactId>system-stubs-jupiter</artifactId>
+            <version>2.0.1</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.apache.maven.shared</groupId>
             <artifactId>maven-invoker</artifactId>
diff --git a/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/BootstrapWorkspaceProvider.java b/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/BootstrapWorkspaceProvider.java
@@ -17,18 +17,30 @@ public class BootstrapWorkspaceProvider {
     private final Path base;
     private boolean initialized;
     private LocalProject origin;
+    private final boolean honourAlternatePomFile;
 
     public BootstrapWorkspaceProvider() {
+        this("", true);
+    }
+
+    // Mostly for use by tests
+    BootstrapWorkspaceProvider(String dirName) {
+        // If we get passed in a directory explicitly, don't try and tack the "-f" value onto it
+        this(dirName, false);
+    }
+
+    private BootstrapWorkspaceProvider(String dirName, boolean honourAlternatePomFile) {
+        this.honourAlternatePomFile = honourAlternatePomFile;
         // load the workspace lazily on request, in case the component is injected but the logic using it is skipped
-        base = Paths.get("").normalize().toAbsolutePath();
+        base = Paths.get(dirName).normalize().toAbsolutePath();
     }
 
     public LocalProject origin() {
         if (!initialized) {
             Path modulePath = base;
             final String alternatePomParam = BootstrapMavenOptions.newInstance()
                     .getOptionValue(BootstrapMavenOptions.ALTERNATE_POM_FILE);
-            if (alternatePomParam != null) {
+            if (alternatePomParam != null && honourAlternatePomFile) {
                 final Path path = Paths.get(alternatePomParam);
                 if (path.isAbsolute()) {
                     modulePath = path;
diff --git a/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java b/independent-projects/extension-maven-plugin/src/main/java/io/quarkus/maven/ExtensionDescriptorMojo.java
@@ -21,6 +21,7 @@
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Scm;
 import org.apache.maven.plugin.AbstractMojo;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.logging.Log;
@@ -678,17 +679,47 @@ public boolean visitLeave(DependencyNode node) {
     }
 
     private void addSource(ObjectNode extObject) throws MojoExecutionException {
-        Map<String, String> repo = ScmInfoProvider.getSourceRepo();
+        Scm scm = getScm();
+        String scmUrl = scm != null ? scm.getUrl() : null;
+
+        Map<String, String> repo = new ScmInfoProvider().getSourceRepo(scmUrl);
         if (repo != null) {
             ObjectNode metadata = getMetadataNode(extObject);
             for (Map.Entry<String, String> e : repo.entrySet()) {
-                // Tools may not be able to handle nesting in metadata, so do fake-nesting
-                metadata.put("scm-" + e.getKey(), e.getValue());
+                // Ignore if already set
+                String value = e.getValue();
+                String fieldName = "scm-" + e.getKey();
+                if (!metadata.has(fieldName) && value != null) {
+                    // Tools may not be able to handle nesting in metadata, so do fake-nesting
+                    metadata.put(fieldName, value);
+                }
 
             }
         }
     }
 
+    private Scm getScm() {
+        // We have three ways to do this; project.getScm() will query the derived model. Sadly, inherited <scm> entries are usually wrong, unless the parent is in the same project
+        // We can use getOriginalModel and getParent to walk the tree, but this will miss parents in poms outside the current execution, which might include a local reactor that we'd actually want to query
+        // Or we can use the bootstrap provider
+        Scm scm = null;
+        final Artifact artifact = project.getArtifact();
+        LocalProject localProject = workspaceProvider.getProject(artifact.getGroupId(), artifact.getArtifactId());
+
+        if (localProject == null) {
+            final Log log = getLog();
+            log.warn("Workspace provider could not resolve local project for " + artifact.getGroupId() + ":"
+                    + artifact.getArtifactId());
+        }
+
+        while (scm == null && localProject != null) {
+            scm = localProject.getRawModel().getScm();
+            localProject = localProject.getLocalParent();
+        }
+        return scm;
+
+    }
+
     public void addJavaVersion(ObjectNode extObject) {
         ObjectNode metadataNode = getMetadataNode(extObject);
         // Ignore if already set
diff --git a/independent-projects/extension-maven-plugin/src/test/java/io/quarkus/maven/ExtensionDescriptorMojoTest.java b/independent-projects/extension-maven-plugin/src/test/java/io/quarkus/maven/ExtensionDescriptorMojoTest.java
@@ -33,19 +33,29 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 import io.quarkus.bootstrap.resolver.maven.BootstrapMavenContext;
 import io.quarkus.bootstrap.resolver.maven.MavenArtifactResolver;
+import uk.org.webcompere.systemstubs.environment.EnvironmentVariables;
+import uk.org.webcompere.systemstubs.jupiter.SystemStub;
+import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension;
 
+@ExtendWith(SystemStubsExtension.class)
 class ExtensionDescriptorMojoTest extends AbstractMojoTestCase {
 
+    @SystemStub
+    private EnvironmentVariables environment;
+
     private static final boolean RESOLVE_OFFLINE = true;
     // Test resources end up in target/test-classes after filtering
     public static final String TEST_RESOURCES = "target/test-classes/";
 
     @BeforeEach
     public void setup() throws Exception {
         super.setUp();
+        // Make sure that we don't have the GITHUB_REPOSITORY environment variable masking what this mojo does
+        environment.set("GITHUB_REPOSITORY", null);
     }
 
     @AfterEach
@@ -82,6 +92,7 @@ public void shouldCreateExtensionProperties()
     @Test
     public void shouldCreateMetadata()
             throws Exception {
+
         ExtensionDescriptorMojo mojo = makeMojo("simple-pom-with-checks-disabled");
         File yamlFile = getGeneratedExtensionMetadataFile(mojo.project.getBasedir(),
                 "target/classes/META-INF/quarkus-extension.yaml");
@@ -97,11 +108,29 @@ public void shouldCreateMetadata()
         assertYamlContains(fileContents, "name", "an arbitrary name");
         assertYamlContains(fileContents, "artifact", "io.quackiverse:test-artifact::jar:1.4.2-SNAPSHOT");
 
-        // From maven this property should be set, running in an IDE it won't be unless specially configured
-        if (System.getenv("GITHUB_REPOSITORY") != null) {
-            // Lazily test that the scm is there but is an object
-            assertYamlContains(fileContents, "scm-url", "https://github.com/some/repo");
+        assertYamlContains(fileContents, "scm-url", "https://github.com/from/pom");
+
+    }
+
+    @Test
+    public void shouldReadLocalParentsForScmInfo()
+            throws Exception {
+
+        ExtensionDescriptorMojo mojo = makeMojo("simple-pom-with-checks-disabled-and-local-parent/child");
+        File yamlFile = getGeneratedExtensionMetadataFile(mojo.project.getBasedir(),
+                "target/classes/META-INF/quarkus-extension.yaml");
+
+        // Tidy up any artifacts from previous runs
+        if (yamlFile.exists()) {
+            Files.delete(yamlFile.toPath());
         }
+        mojo.execute();
+        assertTrue(yamlFile.exists());
+
+        String fileContents = readFileAsString(yamlFile);
+        assertYamlContains(fileContents, "artifact", "io.quackiverse:test-artifact-child::jar:1.4.2-SNAPSHOT");
+
+        assertYamlContains(fileContents, "scm-url", "https://github.com/from/parent");
 
     }
 
@@ -222,6 +251,7 @@ private ExtensionDescriptorMojo makeMojo(String dirName) throws Exception {
                 newMojoExecution("extension-descriptor"));
         mojo.repoSystem = mvn.getSystem();
         mojo.repoSession = mvn.getSession();
+        mojo.workspaceProvider = new BootstrapWorkspaceProvider(basedir.getAbsolutePath());
         return mojo;
     }
 
diff --git a/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled-and-local-parent/child/pom.xml b/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled-and-local-parent/child/pom.xml
@@ -0,0 +1,21 @@
+<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>
+  <groupId>io.quackiverse</groupId>
+  <artifactId>test-artifact-child</artifactId>
+  <version>1.4.2-SNAPSHOT</version>
+  <name>an arbitrary name</name>
+
+  <parent>
+    <groupId>io.quackiverse</groupId>
+    <artifactId>test-artifact-parent</artifactId>
+    <version>1.4.2-SNAPSHOT</version>
+    <relativePath>../</relativePath>
+  </parent>
+
+  <properties>
+    <skipExtensionValidation>true</skipExtensionValidation>
+    <ignoreNotDetectedQuarkusCoreVersion>true</ignoreNotDetectedQuarkusCoreVersion>
+  </properties>
+
+</project>
diff --git a/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled-and-local-parent/pom.xml b/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled-and-local-parent/pom.xml
@@ -0,0 +1,19 @@
+<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>
+  <groupId>io.quackiverse</groupId>
+  <artifactId>test-artifact-parent</artifactId>
+  <version>1.4.2-SNAPSHOT</version>
+  <name>an arbitrary parent</name>
+  <packaging>pom</packaging>
+
+
+  <scm>
+    <url>https://github.com/from/parent</url>
+  </scm>
+
+  <properties>
+    <skipExtensionValidation>true</skipExtensionValidation>
+    <ignoreNotDetectedQuarkusCoreVersion>true</ignoreNotDetectedQuarkusCoreVersion>
+  </properties>
+</project>
diff --git a/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled/pom.xml b/independent-projects/extension-maven-plugin/src/test/resources/simple-pom-with-checks-disabled/pom.xml
@@ -11,6 +11,10 @@
     <ignoreNotDetectedQuarkusCoreVersion>true</ignoreNotDetectedQuarkusCoreVersion>
   </properties>
 
+  <scm>
+    <url>https://github.com/from/pom</url>
+  </scm>
+
   <build>
     <plugins>
       <plugin>
diff --git a/independent-projects/tools/devtools-common/pom.xml b/independent-projects/tools/devtools-common/pom.xml
@@ -116,8 +116,8 @@
         <dependency>
             <groupId>uk.org.webcompere</groupId>
             <artifactId>system-stubs-jupiter</artifactId>
-            <scope>test</scope>
             <version>2.0.1</version>
+            <scope>test</scope>
         </dependency>
     </dependencies>
 </project>
diff --git a/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/extensions/ScmInfoProvider.java b/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/project/extensions/ScmInfoProvider.java
@@ -5,20 +5,23 @@
 
 public class ScmInfoProvider {
 
-    public static Map<String, String> getSourceRepo() {
+    public static Map<String, String> getSourceRepo(String valueFromBuildFile) {
         // We could try and parse the .git/config file, but that will be fragile
-        // Let's assume we only care about the repo for official-ish builds produced via github actions
+        // We could use JGit, something like https://wiki.eclipse.org/JGit/User_Guide#Repository but it seems a lot for our needs
         String repo = System.getenv("GITHUB_REPOSITORY");
+        Map info = null;
         if (repo != null) {
-            Map info = new HashMap();
+            info = new HashMap();
             String qualifiedRepo = "https://github.com/" + repo;
             // Don't try and guess where slashes will be, just deal with any double slashes by brute force
             qualifiedRepo = qualifiedRepo.replace("github.com//", "github.com/");
-
             info.put("url", qualifiedRepo);
-            return info;
+
+        } else if (valueFromBuildFile != null) {
+            info = new HashMap();
+            info.put("url", valueFromBuildFile);
         }
-        return null;
+        return info;
     }
 
 }
diff --git a/independent-projects/tools/devtools-common/src/test/java/io/quarkus/devtools/project/extensions/ScmInfoProviderTest.java b/independent-projects/tools/devtools-common/src/test/java/io/quarkus/devtools/project/extensions/ScmInfoProviderTest.java
@@ -28,17 +28,39 @@ public void setUp() {
 
     @Test
     public void shouldReturnNullWhenNoEnvironmentOrBuildConfigIsPresent() {
-        Map scm = getSourceRepo();
+        Map scm = getSourceRepo(null);
         // We shouldn't throw an exception or get upset, we should just quietly return null
         assertNull(scm);
     }
 
+    // Easy case - we just read the info from the environment
     @Test
-    void testGetSourceControlCoordinates() {
+    void testGetSourceControlCoordinatesWhenOnlyEnvironmentIsSet() {
         String repoName = "org/place";
         environment.set("GITHUB_REPOSITORY", repoName);
-        Map repo = getSourceRepo();
+        Map repo = getSourceRepo(null);
         assertNotNull(repo);
         assertEquals(repo.get("url").toString(), "https://github.com/org/place");
     }
+
+    // Easy case - we just read the info from the pom
+    @Test
+    void testGetSourceControlCoordinatesWhenOnlyPomIsSet() {
+        final String scmUrl = "https://github.com/org/frompom";
+        Map repo = getSourceRepo(scmUrl);
+        assertNotNull(repo);
+        assertEquals(repo.get("url").toString(), scmUrl);
+    }
+
+    // Case where the pom info conflicts with the environment info; honour the environment info
+    void testGetSourceControlCoordinatesWhenEnvironmentAndPomDisagree() {
+        final String scmUrl = "https://github.com/org/frompom";
+        String repoName = "org/place";
+        environment.set("GITHUB_REPOSITORY", repoName);
+        Map repo = getSourceRepo(null);
+        assertNotNull(repo);
+        // We should honour the environment info; out of the box, all quarkiverse extensions generated by the quarkus tooling will report a scm url of quarkiverse-parent/<project-name> in their model
+        assertEquals(repo.get("url").toString(), "https://github.com/org/place");
+    }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -311,7 +311,7 @@ private void computeArtifactCoords(ObjectNode extObject) {`
`311`	`311`	`}`
`312`	`312`
`313`	`313`	`private void computeSourceLocation(ObjectNode extObject) {`
`314`		`- Map<String, String> repo = ScmInfoProvider.getSourceRepo();`
	`314`	`+ Map<String, String> repo = ScmInfoProvider.getSourceRepo(null);`
`315`	`315`	`if (repo != null) {`
`316`	`316`	`ObjectNode metadata = getMetadataNode(extObject);`
`317`	`317`