Prefer artifacts with dependencies when de-duplicating coursier dependencies (#801)
diff --git a/private/artifact_utilities.bzl b/private/artifact_utilities.bzl index 3c7f64f..4ae8e89 100644 --- a/private/artifact_utilities.bzl +++ b/private/artifact_utilities.bzl
@@ -5,11 +5,17 @@ load("//:specs.bzl", "utils") def deduplicate_and_sort_artifacts(dep_tree, artifacts, excluded_artifacts, verbose): + # The deps json returned from coursier can have duplicate artifacts with + # different dependencies and exclusions. We want to de-duplicate the + # artifacts and chose the ones that most closely match the exclusions + # specified in the maven_install declaration and not chose ones with + # empty dependencies if possible + # First we find all of the artifacts that have exclusions artifacts_with_exclusions = {} for a in artifacts: coordinate = utils.artifact_coordinate(a) - if "exclusions" in a: + if "exclusions" in a and len(a["exclusions"]) > 0: deduped_exclusions = {} for e in excluded_artifacts: deduped_exclusions["{}:{}".format(e["group"], e["artifact"])] = True @@ -22,6 +28,7 @@ # As we de-duplicate the list keep the duplicate artifacts with exclusions separate # so we can look at them and select the one that has the same exclusions + # Also prefer the duplicates with non-empty dependency lists duplicate_artifacts_with_exclusions = {} deduped_artifacts = {} null_artifacts = [] @@ -34,20 +41,25 @@ duplicate_artifacts_with_exclusions[artifact["coord"]].append(artifact) else: duplicate_artifacts_with_exclusions[artifact["coord"]] = [artifact] + elif artifact["file"] in deduped_artifacts: + if len(artifact["dependencies"]) > 0 and len(deduped_artifacts[artifact["file"]]["dependencies"]) == 0: + deduped_artifacts[artifact["file"]] = artifact else: - if artifact["file"] in deduped_artifacts: - continue deduped_artifacts[artifact["file"]] = artifact # Look through the duplicates with exclusions and try to select the artifact - # that has the same exclusions as specified in the artifact + # that has the same exclusions as specified in the artifact and + # prefer the one with non-empty dependencies for duplicate_coord in duplicate_artifacts_with_exclusions: deduped_artifact_with_exclusion = duplicate_artifacts_with_exclusions[duplicate_coord][0] found_artifact_with_exclusion = False for duplicate_artifact in duplicate_artifacts_with_exclusions[duplicate_coord]: if "exclusions" in duplicate_artifact and sorted(duplicate_artifact["exclusions"]) == sorted(artifacts_with_exclusions[duplicate_coord]): - found_artifact_with_exclusion = True - deduped_artifact_with_exclusion = duplicate_artifact + if not found_artifact_with_exclusion: + found_artifact_with_exclusion = True + deduped_artifact_with_exclusion = duplicate_artifact + elif len(duplicate_artifact["dependencies"]) > 0 and len(deduped_artifact_with_exclusion["dependencies"]) == 0: + deduped_artifact_with_exclusion = duplicate_artifact if verbose and not found_artifact_with_exclusion: print("Could not find duplicate artifact with matching exclusions for {} when de-duplicating the dependency tree. Using exclusions {}".format(deduped_artifact_with_exclusion)) deduped_artifacts[deduped_artifact_with_exclusion["file"]] = deduped_artifact_with_exclusion
diff --git a/rules_jvm_external_deps_install.json b/rules_jvm_external_deps_install.json index 6367dfb..4413e5c 100644 --- a/rules_jvm_external_deps_install.json +++ b/rules_jvm_external_deps_install.json
@@ -2,7 +2,7 @@ "dependency_tree": { "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", "__INPUT_ARTIFACTS_HASH": 431054571, - "__RESOLVED_ARTIFACTS_HASH": 2138969449, + "__RESOLVED_ARTIFACTS_HASH": -2089731, "conflict_resolution": {}, "dependencies": [ { @@ -1179,10 +1179,11 @@ }, { "coord": "io.opencensus:opencensus-api:0.24.0", - "dependencies": [], - "directDependencies": [], - "exclusions": [ - "io.grpc:grpc-context" + "dependencies": [ + "io.grpc:grpc-context:1.33.1" + ], + "directDependencies": [ + "io.grpc:grpc-context:1.33.1" ], "file": "v1/https/repo1.maven.org/maven2/io/opencensus/opencensus-api/0.24.0/opencensus-api-0.24.0.jar", "mirror_urls": [
diff --git a/tests/unit/artifact_utilities_test.bzl b/tests/unit/artifact_utilities_test.bzl index 67db34c..440c8a5 100644 --- a/tests/unit/artifact_utilities_test.bzl +++ b/tests/unit/artifact_utilities_test.bzl
@@ -34,9 +34,9 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") - asserts.equals(env, sorted_dep_tree["dependencies"][0]["exclusions"], []) + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) + asserts.equals(env, [], sorted_dep_tree["dependencies"][0]["exclusions"]) return unittest.end(env) @@ -75,10 +75,10 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 2) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") - asserts.equals(env, sorted_dep_tree["dependencies"][0]["exclusions"], []) - asserts.equals(env, sorted_dep_tree["dependencies"][1]["coord"], "org.checkerframework:checker-qual:2.5.2") + asserts.equals(env, 2, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) + asserts.equals(env, [], sorted_dep_tree["dependencies"][0]["exclusions"]) + asserts.equals(env, "org.checkerframework:checker-qual:2.5.2", sorted_dep_tree["dependencies"][1]["coord"]) return unittest.end(env) @@ -117,9 +117,9 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") - asserts.equals(env, sorted_dep_tree["dependencies"][0]["exclusions"], []) + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) + asserts.equals(env, [], sorted_dep_tree["dependencies"][0]["exclusions"]) return unittest.end(env) @@ -172,9 +172,9 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") - asserts.equals(env, sorted_dep_tree["dependencies"][0]["exclusions"], ["*:*"]) + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) + asserts.equals(env, ["*:*"], sorted_dep_tree["dependencies"][0]["exclusions"]) dep_tree = { "conflict_resolution": {}, @@ -221,8 +221,8 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) asserts.equals( env, sorted_dep_tree["dependencies"][0]["exclusions"], @@ -286,9 +286,9 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, excluded_artifacts, False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") - asserts.equals(env, sorted_dep_tree["dependencies"][0]["exclusions"], ["*:*"]) + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) + asserts.equals(env, ["*:*"], sorted_dep_tree["dependencies"][0]["exclusions"]) dep_tree = { "conflict_resolution": {}, @@ -336,22 +336,170 @@ sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, excluded_artifacts, False) - asserts.equals(env, len(sorted_dep_tree["dependencies"]), 1) - asserts.equals(env, sorted_dep_tree["dependencies"][0]["coord"], "com.google.guava:guava:27.0-jre") + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:27.0-jre", sorted_dep_tree["dependencies"][0]["coord"]) asserts.equals( env, - sorted_dep_tree["dependencies"][0]["exclusions"], [ "org.codehaus.mojo:animal-sniffer-annotations", "com.google.j2objc:j2objc-annotations", "org.checkerframework:checker-qual", ], + sorted_dep_tree["dependencies"][0]["exclusions"], ) return unittest.end(env) one_artifact_duplicate_with_global_exclusions_test = unittest.make(_one_artifact_duplicate_with_global_exclusions_test_impl) +def _duplicate_with_and_without_dependencies_test_impl(ctx): + env = unittest.begin(ctx) + + dep_tree = { + "conflict_resolution": {}, + "dependencies": [ + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [], + "exclusions": [], + }, + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [ + "com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava", + "com.google.j2objc:j2objc-annotations:1.3", + "com.google.code.findbugs:jsr305:3.0.2", + "com.google.errorprone:error_prone_annotations:2.10.0", + "org.checkerframework:checker-qual:2.10.0", + "com.google.guava:failureaccess:1.0.1", + ], + "exclusions": [], + }, + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [], + "exclusions": [], + }, + ], + "version": "0.1.0", + } + + artifacts = [{ + "group": "com.google.guava", + "artifact": "guava", + "version": "31.1-jre", + "exclusions": [], + }] + + sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) + + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:31.1-jre", sorted_dep_tree["dependencies"][0]["coord"]) + + # We should select the duplicate artifact that has non-empty dependencies + asserts.equals( + env, + [ + "com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava", + "com.google.j2objc:j2objc-annotations:1.3", + "com.google.code.findbugs:jsr305:3.0.2", + "com.google.errorprone:error_prone_annotations:2.10.0", + "org.checkerframework:checker-qual:2.10.0", + "com.google.guava:failureaccess:1.0.1", + ], + sorted_dep_tree["dependencies"][0]["dependencies"], + ) + + return unittest.end(env) + +duplicate_with_and_without_dependencies_test = unittest.make(_duplicate_with_and_without_dependencies_test_impl) + +def _duplicate_with_and_without_dependencies_and_exclusions_test_impl(ctx): + env = unittest.begin(ctx) + + dep_tree = { + "conflict_resolution": {}, + "dependencies": [ + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [], + "exclusions": [ + "org.codehaus.mojo:animal-sniffer-annotations", + "com.google.j2objc:j2objc-annotations", + ], + }, + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [ + "com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava", + "com.google.j2objc:j2objc-annotations:1.3", + "com.google.code.findbugs:jsr305:3.0.2", + "com.google.errorprone:error_prone_annotations:2.10.0", + "org.checkerframework:checker-qual:2.10.0", + "com.google.guava:failureaccess:1.0.1", + ], + "exclusions": [ + "org.codehaus.mojo:animal-sniffer-annotations", + "com.google.j2objc:j2objc-annotations", + ], + }, + { + "coord": "com.google.guava:guava:31.1-jre", + "file": "v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar", + "directDependencies": [], + "dependencies": [], + "exclusions": [ + "org.codehaus.mojo:animal-sniffer-annotations", + "com.google.j2objc:j2objc-annotations", + ], + }, + ], + "version": "0.1.0", + } + + artifacts = [{ + "group": "com.google.guava", + "artifact": "guava", + "version": "31.1-jre", + "exclusions": [ + {"group": "org.codehaus.mojo", "artifact": "animal-sniffer-annotations"}, + {"group": "com.google.j2objc", "artifact": "j2objc-annotations"}, + ], + }] + + sorted_dep_tree = deduplicate_and_sort_artifacts(dep_tree, artifacts, [], False) + + asserts.equals(env, 1, len(sorted_dep_tree["dependencies"])) + asserts.equals(env, "com.google.guava:guava:31.1-jre", sorted_dep_tree["dependencies"][0]["coord"]) + + # We should select the duplicate artifact that has non-empty dependencies + asserts.equals( + env, + [ + "com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava", + "com.google.j2objc:j2objc-annotations:1.3", + "com.google.code.findbugs:jsr305:3.0.2", + "com.google.errorprone:error_prone_annotations:2.10.0", + "org.checkerframework:checker-qual:2.10.0", + "com.google.guava:failureaccess:1.0.1", + ], + sorted_dep_tree["dependencies"][0]["dependencies"], + ) + + return unittest.end(env) + +duplicate_with_and_without_dependencies_and_exclusions_test = unittest.make(_duplicate_with_and_without_dependencies_and_exclusions_test_impl) + def artifact_utilities_test_suite(): unittest.suite( "artifact_utilities_tests", @@ -361,4 +509,6 @@ one_artifact_duplicate_no_exclusions_test, one_artifact_duplicate_matches_exclusions_test, one_artifact_duplicate_with_global_exclusions_test, + duplicate_with_and_without_dependencies_test, + duplicate_with_and_without_dependencies_and_exclusions_test, )