Merge pull request #48800 from reaver585/sync-file-dep-jar-naming-for-gradle

Sync naming convention for file dependency JARs

Author: aloubyansky

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java

@@ -920,10 +920,7 @@ private void copyDependency(Set<ArtifactKey> parentFirstArtifacts, OutputTargetB
             return;
         }
         for (Path resolvedDep : appDep.getResolvedPaths()) {
-            final boolean isDirectory = Files.isDirectory(resolvedDep);
-            // we don't use getFileName() for directories, since directories would often be "classes" ending up merging content from multiple dependencies in the same package
-            final String fileName = isDirectory ? getFileNameForDirectory(appDep)
-                    : appDep.getGroupId() + "." + resolvedDep.getFileName();
+            final String fileName = getJarFileName(appDep, resolvedDep);
             final Path targetPath;
 
             if (allowParentFirst && parentFirstArtifacts.contains(appDep.getKey())) {
@@ -935,7 +932,7 @@ private void copyDependency(Set<ArtifactKey> parentFirstArtifacts, OutputTargetB
             }
             runtimeArtifacts.computeIfAbsent(appDep.getKey(), (s) -> new ArrayList<>(1)).add(targetPath);
 
-            if (isDirectory) {
+            if (Files.isDirectory(resolvedDep)) {
                 // This case can happen when we are building a jar from inside the Quarkus repository
                 // and Quarkus Bootstrap's localProjectDiscovery has been set to true. In such a case
                 // the non-jar dependencies are the Quarkus dependencies picked up on the file system
@@ -976,12 +973,20 @@ private void copyDependency(Set<ArtifactKey> parentFirstArtifacts, OutputTargetB
     }
 
     /**
-     * Returns a JAR file name to be used for a content of a dependency that is in a directory.
+     * Returns a JAR file name to be used for a content of a dependency, depending on whether the resolved path
+     * is a directory or not.
+     * We don't use getFileName() for directories, since directories would often be "classes" ending up merging
+     * content from multiple dependencies in the same package
      *
      * @param dep dependency
+     * @param resolvedPath path of the resolved dependency
      * @return JAR file name
      */
-    private static String getFileNameForDirectory(ResolvedDependency dep) {
+    public static String getJarFileName(ResolvedDependency dep, Path resolvedPath) {
+        boolean isDirectory = Files.isDirectory(resolvedPath);
+        if (!isDirectory) {
+            return dep.getGroupId() + "." + resolvedPath.getFileName();
+        }
         final StringBuilder sb = new StringBuilder();
         sb.append(dep.getGroupId()).append(".").append(dep.getArtifactId()).append("-");
         if (!dep.getClassifier().isEmpty()) {

devtools/gradle/gradle-application-plugin/src/main/java/io/quarkus/gradle/tasks/QuarkusBuildDependencies.java

@@ -19,6 +19,7 @@
 
 import io.quarkus.bootstrap.model.ApplicationModel;
 import io.quarkus.deployment.pkg.PackageConfig;
+import io.quarkus.deployment.pkg.steps.JarResultBuildStep;
 import io.quarkus.fs.util.ZipUtils;
 import io.quarkus.gradle.QuarkusPlugin;
 import io.quarkus.maven.dependency.ArtifactKey;
@@ -184,7 +185,7 @@ private void jarDependencies(Path libBoot, Path libMain) {
                     ResolvedDependency dep = depAndTarget.getValue();
                     Path targetDir = depAndTarget.getKey();
                     dep.getResolvedPaths().forEach(p -> {
-                        String file = dep.getGroupId() + '.' + p.getFileName();
+                        String file = JarResultBuildStep.getJarFileName(dep, p);
                         Path target = targetDir.resolve(file);
                         if (!Files.exists(target)) {
                             getLogger().debug("Dependency {} : copying {} to {}",

integration-tests/gradle/src/main/resources/additional-source-set-as-dependency/build.gradle

@@ -0,0 +1,33 @@
+plugins {
+    id 'java'
+    id 'io.quarkus'
+}
+
+repositories {
+    mavenLocal {
+        content {
+            includeGroupByRegex 'io.quarkus.*'
+            includeGroup 'org.hibernate.orm'
+        }
+    }
+    mavenCentral()
+    gradlePluginPortal()
+}
+
+sourceSets {
+    additional {
+        java {
+            srcDir "src/gen/java"
+        }
+    }
+}
+
+dependencies {
+    implementation enforcedPlatform("${quarkusPlatformGroupId}:${quarkusPlatformArtifactId}:${quarkusPlatformVersion}")
+    implementation sourceSets.additional.output
+    implementation 'io.quarkus:quarkus-arc'
+    implementation 'io.quarkus:quarkus-rest'
+}
+
+group 'org.acme'
+version '1.0.0-SNAPSHOT'

integration-tests/gradle/src/main/resources/additional-source-set-as-dependency/gradle.properties

@@ -0,0 +1,2 @@
+quarkusPlatformArtifactId=quarkus-bom
+quarkusPlatformGroupId=io.quarkus

integration-tests/gradle/src/main/resources/additional-source-set-as-dependency/settings.gradle

@@ -0,0 +1,16 @@
+pluginManagement {
+    repositories {
+        mavenLocal {
+            content {
+                includeGroupByRegex 'io.quarkus.*'
+                includeGroup 'org.hibernate.orm'
+            }
+        }
+        mavenCentral()
+        gradlePluginPortal()
+    }
+    plugins {
+      id 'io.quarkus' version "${quarkusPluginVersion}"
+    }
+}
+rootProject.name = 'additional-source-set-as-dependency'

integration-tests/gradle/src/main/resources/additional-source-set-as-dependency/src/gen/java/io/reproducer/GreetApi.java

@@ -0,0 +1,6 @@
+package io.reproducer;
+
+public interface GreetApi {
+
+  public String hello();
+}

integration-tests/gradle/src/main/resources/additional-source-set-as-dependency/src/main/java/org/acme/GreetingResource.java

@@ -0,0 +1,17 @@
+package org.acme;
+
+import io.reproducer.GreetApi;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+
+@Path("/hello")
+public class GreetingResource implements GreetApi {
+
+  @GET
+  @Produces(MediaType.TEXT_PLAIN)
+  public String hello() {
+    return "Hello from Quarkus REST...";
+  }
+}

integration-tests/gradle/src/test/java/io/quarkus/gradle/run/AdditionalSourceSetAsDependencyTest.java

@@ -0,0 +1,28 @@
+package io.quarkus.gradle.run;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.quarkus.gradle.devmode.QuarkusDevGradleTestBase;
+
+// Reproduces https://github.com/quarkusio/quarkus/issues/48768.
+// Despite extending QuarkusDevGradleTestBase, this test does not use the dev mode,
+// but rather runs the application in a production mode using `quarkusRun` task.
+// At this point it's the only test that does so, but if more such tests are added,
+// perhaps it would make sense to create a separate base class for them.
+public class AdditionalSourceSetAsDependencyTest extends QuarkusDevGradleTestBase {
+
+    @Override
+    protected String projectDirectoryName() {
+        return "additional-source-set-as-dependency";
+    }
+
+    @Override
+    protected String[] buildArguments() {
+        return new String[] { "clean", "quarkusRun" };
+    }
+
+    @Override
+    protected void testDevMode() throws Exception {
+        assertThat(getHttpResponse("/hello")).contains("Hello from Quarkus REST...");
+    }
+}