Merge pull request #127 from cgruber/eliminate_duplicate_deps

Fix a problem where a pom may specify a dependency more than once.

kramer/src/main/kotlin/kramer/GenerateMavenRepoCommand.kt

@@ -32,6 +32,17 @@ import com.squareup.tools.maven.resolution.FetchStatus.RepositoryFetchStatus.SUC
 import com.squareup.tools.maven.resolution.Repositories.Companion.DEFAULT
 import com.squareup.tools.maven.resolution.ResolutionResult
 import com.squareup.tools.maven.resolution.ResolvedArtifact
+import java.io.IOException
+import java.net.URI
+import java.nio.file.FileSystem
+import java.nio.file.FileSystems
+import java.nio.file.Files
+import java.nio.file.Path
+import java.util.Collections
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.atomic.AtomicInteger
+import kotlin.collections.Map.Entry
+import kotlin.system.measureTimeMillis
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.FlowPreview
@@ -49,17 +60,6 @@ import kramer.GenerateMavenRepo.ArtifactResolution.AarArtifactResolution
 import kramer.GenerateMavenRepo.ArtifactResolution.FileArtifactResolution
 import kramer.GenerateMavenRepo.ArtifactResolution.JarArtifactResolution
 import org.apache.maven.model.Dependency
-import java.io.IOException
-import java.net.URI
-import java.nio.file.FileSystem
-import java.nio.file.FileSystems
-import java.nio.file.Files
-import java.nio.file.Path
-import java.util.Collections
-import java.util.concurrent.ConcurrentHashMap
-import java.util.concurrent.atomic.AtomicInteger
-import kotlin.collections.Map.Entry
-import kotlin.system.measureTimeMillis
 
 class GenerateMavenRepo(
   fs: FileSystem = FileSystems.getDefault()
@@ -410,15 +410,16 @@ private val jvmPackagingTypes = setOf("jar", "aar", "bundle")
 private val bazelPackagePrefixes = setOf('@', '/', ':')
 /**
  * For a given resolved artifact, prepare the list of its dependencies, excluding unused scopes,
- * explicitly excluded deps, and fixing and formatting.
+ * explicitly excluded deps, and fixing and formatting.  Returns an ordered set, to handle cases
+ * where a dependency is included in multiple scopes, or multiple times in the .pom file.
  */
 private fun prepareDependencies(
   resolved: ResolvedArtifact,
   config: ArtifactConfig,
   seen: ConcurrentHashMap<String, GenerateMavenRepo.IndexEntry>,
   repoConfig: RepositorySpecification
-): Sequence<String> {
-  if (!config.snippet.isNullOrBlank()) return sequenceOf()
+): Set<String> {
+  if (!config.snippet.isNullOrBlank()) return setOf()
   val substitutes =
     repoConfig.targetSubstitutes.getOrElse(resolved.groupId) { mapOf() }
   return when {
@@ -427,6 +428,7 @@ private fun prepareDependencies(
           if (it[0] in bazelPackagePrefixes) it
           else unversionedDependency(it).targetFor(repoConfig.name, substitutes, resolved)
         }
+        .toSet()
 
     else -> resolved.model.dependencies.asSequence()
         .filter { dep -> !dep.isOptional }
@@ -457,6 +459,7 @@ private fun prepareDependencies(
         }
         .map { dep -> dep.targetFor(repoConfig.name, substitutes, resolved) }
         .plus(config.include.filter { it[0] in bazelPackagePrefixes }) // add bazel extra includes
+        .toSet()
   }
 }
 
@@ -489,7 +492,8 @@ internal enum class UnknownPackagingStrategy {
       exit: AtomicInteger,
       a: FileArtifactResolution
     ) = kontext.out {
-      "WARNING: ${a.resolved.coordinate} is not a handled package type, ${a.resolved.model.packaging}"
+      "WARNING: ${a.resolved.coordinate} is not a handled package type, " +
+        "${a.resolved.model.packaging}"
     }
   },
   FAIL {
@@ -499,7 +503,8 @@ internal enum class UnknownPackagingStrategy {
       a: FileArtifactResolution
     ) {
       kontext.out {
-        "\nERROR: ${a.resolved.coordinate} is not a supported packaging, ${a.resolved.model.packaging}"
+        "\nERROR: ${a.resolved.coordinate} is not a supported packaging, " +
+          "${a.resolved.model.packaging}"
       }
       exit.set(1)
     }

kramer/src/test/kotlin/kramer/integration/GenerateMavenRepoIntegrationTest.kt

@@ -19,7 +19,9 @@ import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.Truth.assertWithMessage
 import java.io.ByteArrayOutputStream
 import java.io.PrintStream
+import java.nio.file.FileSystems
 import java.nio.file.Files
+import java.nio.file.Path
 import java.nio.file.Paths
 import kramer.GenerateMavenRepo
 import kramer.Kontext
@@ -30,8 +32,6 @@ import kramer.parseJson
 import org.junit.After
 import org.junit.Ignore
 import org.junit.Test
-import java.nio.file.FileSystems
-import java.nio.file.Path
 
 /**
  * Integration tests for [GenerateMavenRepoCommand]. These are dependent on access to the network
@@ -88,7 +88,9 @@ class GenerateMavenRepoIntegrationTest {
     assertThat(output).contains("Generated 1 build files in ")
     assertThat(output).contains("Resolved 1 artifacts with 100 threads in")
     assertThat(output)
-        .contains("WARNING: com.squareup.sqldelight:runtime:1.4.0 is not a handled package type, pom")
+      .contains("WARNING: com.squareup.sqldelight:runtime:1.4.0 is not a handled package type")
+    assertThat(output)
+      .contains("com.squareup.sqldelight:runtime:1.4.0 is not a handled package type, pom")
     val build = mavenRepo.readBuildFile("com.squareup.sqldelight")
     assertThat(build).contains("com.squareup.sqldelight:runtime:1.4.0")
     assertThat(build).contains("filegroup(")
@@ -169,6 +171,18 @@ class GenerateMavenRepoIntegrationTest {
     assertThat(helpshift).contains("\"@maven//androidx/annotation\",")
   }
 
+  @Test fun duplicateDeps() {
+    val args = configFlags("duplicate-deps", "gen-maven-repo")
+    val output = cmd.test(args, baos)
+    assertThat(output).contains("Building workspace for 9 artifacts")
+    assertThat(output).contains("Generated 2 build files in ")
+    assertThat(output).contains("Resolved 9 artifacts with 100 threads in")
+
+    val jimfs = mavenRepo.readBuildFile("com.github.jnr")
+    assertThat(jimfs).contains("\":jffi\",")
+    assertThat(jimfs).doesNotContainMatch("\":jffi\",\n        \":jffi\",")
+  }
+
   @Test fun overconfiguredArtifacts() {
     val args = configFlags("overconfigured", "gen-maven-repo")
     val output = cmd.fail(args, baos)

kramer/src/test/kotlin/kramer/integration/test-duplicate-deps-config.json

@@ -0,0 +1,18 @@
+{
+  "artifacts": {
+    "com.github.jnr:jnr-ffi:2.1.9": { "insecure": true },
+    "org.ow2.asm:asm-tree:5.0.3": {"insecure": true },
+    "com.github.jnr:jnr-x86asm:1.0.2": {"insecure": true },
+    "com.github.jnr:jffi:1.2.17": {"insecure": true },
+    "com.github.jnr:jnr-a64asm:1.0.0": {"insecure": true },
+    "org.ow2.asm:asm-analysis:5.0.3": {"insecure": true },
+    "org.ow2.asm:asm-commons:5.0.3": {"insecure": true },
+    "org.ow2.asm:asm:5.0.3": {"insecure": true },
+    "org.ow2.asm:asm-util:5.0.3": {"insecure": true }
+  },
+  "jetifier_excludes": [],
+  "maven_rules_repository": "maven_repository_rules",
+  "name": "maven",
+  "target_substitutes": {},
+  "use_jetifier": false
+}