diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMerger.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMerger.kt index 510d72bc6..2e549f89d 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMerger.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMerger.kt @@ -90,9 +90,23 @@ class JdepsMerger { } if (reportUnusedDeps != "off") { - val unusedLabels = dependencyMap.values - .filter { it.kind == Deps.Dependency.Kind.UNUSED } - .mapNotNull { readJarOwnerFromManifest(Paths.get(it.path)).label } + val kindMap = mutableMapOf() + + // A target might produce multiple jars (Android produces _resources.jar) + // so we need to make sure wedon't mart the dependency as unused + // unless all of the jars are unused. + dependencyMap.values.forEach { + val label = readJarOwnerFromManifest(Paths.get(it.path)).label + if (label != null && + kindMap.getOrDefault(label, Deps.Dependency.Kind.UNUSED) >= it.kind + ) { + kindMap.put(label, it.kind) + } + } + + val unusedLabels = kindMap.entries + .filter { it.value == Deps.Dependency.Kind.UNUSED } + .map { it.key } .filter { it != label } if (unusedLabels.isNotEmpty()) { diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMergerTest.kt b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMergerTest.kt index db2eb3e69..2f4e2b187 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMergerTest.kt +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMergerTest.kt @@ -25,7 +25,7 @@ class JdepsMergerTest { private fun jdeps( path: String, - buildDeps: (Deps.Dependencies.Builder.() -> Deps.Dependencies.Builder) + buildDeps: (Deps.Dependencies.Builder.() -> Deps.Dependencies.Builder), ): Path { val jdepsPath = Files.createDirectories(wrkDir.resolve("jdeps")).resolve(path) @@ -40,12 +40,12 @@ class JdepsMergerTest { return Files.createDirectories(wrkDir.resolve("out")).resolve(name) } - fun ktJvmLibrary(label: String): String { - val path = Files.createDirectories(wrkDir.resolve("out")).resolve("lib$label.jar") + fun ktJvmLibrary(label: String, suffix: String = ""): String { + val path = Files.createDirectories(wrkDir.resolve("out")).resolve("lib${label}$suffix.jar") JarCreator( path = path, normalize = true, - verbose = false + verbose = false, ).also { it.setJarOwner(label, "kt_jvm_library") it.execute() @@ -63,7 +63,7 @@ class JdepsMergerTest { kind = Dependency.Kind.EXPLICIT path = "/path/to/kt_dep.jar" build() - } + }, ) } @@ -73,7 +73,7 @@ class JdepsMergerTest { kind = Dependency.Kind.EXPLICIT path = "/path/to/java_dep.jar" build() - } + }, ) } @@ -89,7 +89,7 @@ class JdepsMergerTest { input(javaJdeps) flag(JdepsMergerFlags.OUTPUT, mergedJdeps) flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "off") - } + }, ) } } @@ -100,7 +100,7 @@ class JdepsMergerTest { assertThat(depsProto.ruleLabel).isEqualTo("//foo/bar:baz") assertThat(depsProto.dependencyList.map { it.path }).containsExactly( "/path/to/kt_dep.jar", - "/path/to/java_dep.jar" + "/path/to/java_dep.jar", ) } @@ -114,7 +114,7 @@ class JdepsMergerTest { kind = Dependency.Kind.UNUSED path = "/path/to/shared_dep.jar" build() - } + }, ) } @@ -124,7 +124,7 @@ class JdepsMergerTest { kind = Dependency.Kind.EXPLICIT path = "/path/to/shared_dep.jar" build() - } + }, ) } @@ -140,7 +140,7 @@ class JdepsMergerTest { input(javaJdeps) flag(JdepsMergerFlags.OUTPUT, mergedJdeps) flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "off") - } + }, ) } } @@ -163,7 +163,7 @@ class JdepsMergerTest { kind = Dependency.Kind.UNUSED path = unusedKotlinDep build() - } + }, ) } @@ -173,7 +173,7 @@ class JdepsMergerTest { kind = Dependency.Kind.EXPLICIT path = ktJvmLibrary("java_dep") build() - } + }, ) } @@ -189,7 +189,7 @@ class JdepsMergerTest { flag(JdepsMergerFlags.TARGET_LABEL, "//foo/bar:baz") flag(JdepsMergerFlags.OUTPUT, mergedJdeps) flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "warn") - } + }, ) } } @@ -200,7 +200,7 @@ class JdepsMergerTest { assertThat( depsProto.dependencyList .filter { it.kind == Dependency.Kind.UNUSED } - .map { it.path } + .map { it.path }, ).containsExactly(unusedKotlinDep) } @@ -215,7 +215,7 @@ class JdepsMergerTest { kind = Dependency.Kind.UNUSED path = unusedKotlinDep build() - } + }, ) } @@ -225,7 +225,7 @@ class JdepsMergerTest { kind = Dependency.Kind.EXPLICIT path = ktJvmLibrary("java_dep") build() - } + }, ) } @@ -242,7 +242,7 @@ class JdepsMergerTest { flag(JdepsMergerFlags.TARGET_LABEL, "//foo/bar:baz") flag(JdepsMergerFlags.OUTPUT, mergedJdeps) flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "error") - } + }, ) } } @@ -253,10 +253,100 @@ class JdepsMergerTest { assertThat( depsProto.dependencyList .filter { it.kind == Dependency.Kind.UNUSED } - .map { it.path } + .map { it.path }, ).containsExactly(unusedKotlinDep) } + @Test + fun `unused deps multiple jars for label`() { + val merger = DaggerJdepsMergerTestComponent.builder().build().jdepsMerger() + + val unusedKotlinDepA = ktJvmLibrary("kotlin_dep", "_a") + val unusedKotlinDepB = ktJvmLibrary("kotlin_dep", "_b") + val kotlinJdeps = jdeps("kt.jdeps") { + addDependency( + with(Dependency.newBuilder()) { + kind = Dependency.Kind.UNUSED + path = unusedKotlinDepA + build() + }, + ) + addDependency( + with(Dependency.newBuilder()) { + kind = Dependency.Kind.UNUSED + path = unusedKotlinDepB + build() + }, + ) + } + + val mergedJdeps = out("merged.jdeps") + + val result = WorkerContext.run { + doTask("jdepsmerge") { taskCtx -> + MergeJdeps(merger = merger).invoke( + taskCtx, + args { + input(kotlinJdeps) + flag(JdepsMergerFlags.TARGET_LABEL, "//foo/bar:baz") + flag(JdepsMergerFlags.OUTPUT, mergedJdeps) + flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "warn") + }, + ) + } + } + assertThat(result.status).isEqualTo(SUCCESS) + assertThat(result.log.out.toString()).contains("'remove deps kotlin_dep' //foo/bar:baz") + } + + @Test + fun `used deps multiple jars for label`() { + val merger = DaggerJdepsMergerTestComponent.builder().build().jdepsMerger() + + val unusedKotlinDep = ktJvmLibrary("kotlin_dep", "_a") + val usedKotlinDep = ktJvmLibrary("kotlin_dep", "_b") + val kotlinJdeps = jdeps("kt.jdeps") { + addDependency( + with(Dependency.newBuilder()) { + kind = Dependency.Kind.UNUSED + path = unusedKotlinDep + build() + }, + ) + addDependency( + with(Dependency.newBuilder()) { + kind = Dependency.Kind.EXPLICIT + path = usedKotlinDep + build() + }, + ) + } + + val mergedJdeps = out("merged.jdeps") + + val result = WorkerContext.run { + doTask("jdepsmerge") { taskCtx -> + MergeJdeps(merger = merger).invoke( + taskCtx, + args { + input(kotlinJdeps) + flag(JdepsMergerFlags.TARGET_LABEL, "//foo/bar:baz") + flag(JdepsMergerFlags.OUTPUT, mergedJdeps) + flag(JdepsMergerFlags.REPORT_UNUSED_DEPS, "warn") + }, + ) + } + } + assertThat(result.status).isEqualTo(SUCCESS) + + val depsProto = depsProto(mergedJdeps) + assertThat( + depsProto.dependencyList + .filter { it.kind == Dependency.Kind.EXPLICIT } + .map { it.path }, + ).containsExactly(usedKotlinDep) + } + private fun depsProto(mergedJdeps: Path) = Deps.Dependencies.parseFrom(BufferedInputStream(Files.newInputStream(mergedJdeps)))