Print a warning when there is a duplicate artifact in the artifact list and setup basic integration testing (bazel-contrib#579)

Author: cheister

.bazelci/presubmit.yml

@@ -9,7 +9,7 @@ tasks:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
       - bazel run @duplicate_artifacts_test//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_flags:
       - "--//settings:stamp_manifest=True"
     test_targets:
@@ -33,7 +33,7 @@ tasks:
     shell_commands:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_targets:
       - "--"
       - "//..."
@@ -43,7 +43,7 @@ tasks:
     shell_commands:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_targets:
       - "--"
       - "//..."
@@ -53,7 +53,7 @@ tasks:
     shell_commands:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_targets:
       - "--"
       - "//..."
@@ -68,7 +68,7 @@ tasks:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
       - bazel run @duplicate_artifacts_test//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_targets:
       - "--"
       - "//..."
@@ -81,7 +81,7 @@ tasks:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
       - bazel run @duplicate_artifacts_test//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_flags:
       - "--//settings:stamp_manifest=True"
     test_targets:
@@ -98,7 +98,7 @@ tasks:
       - bazel run @unpinned_regression_testing//:pin
       - bazel run @unpinned_maven_install_in_custom_location//:pin
       - bazel run @duplicate_artifacts_test//:pin
-      - bazel run @regression_testing//:outdated
+      - tests/bazel_run_tests.sh
     test_targets:
       - "--"
       - "//..."

README.md

@@ -36,6 +36,7 @@ Table of Contents
          * [Hiding transitive dependencies](#hiding-transitive-dependencies)
          * [Fetch and resolve timeout](#fetch-and-resolve-timeout)
          * [Jetifier](#jetifier)
+         * [Duplicate artifact warning](#duplicate-artifact-warning)
       * [Exporting and consuming artifacts from external repositories](#exporting-and-consuming-artifacts-from-external-repositories)
       * [Publishing to external repositories](#publishing-to-external-repositories)
       * [Demo](#demo)
@@ -893,6 +894,22 @@ maven_install(
 )
 ```
 
+### Duplicate artifact warning
+
+By default you will be warned if there are duplicate artifacts in your artifact list. The `duplicate_version_warning` setting can be used to change this behavior. Use "none" to disable the warning and "error" to fail the build instead of warn.
+
+```python
+maven_install(
+    artifacts = [
+        # ...
+    ],
+    repositories = [
+        # ...
+    ],
+    duplicate_version_warning = "error"
+)
+```
+
 ### Provide JVM options for Coursier with `COURSIER_OPTS`
 
 You can set up `COURSIER_OPTS` environment variable to provide some additional JVM options for Coursier.

WORKSPACE

@@ -363,6 +363,33 @@ maven_install(
     ],
 )
 
+maven_install(
+    name = "duplicate_version_warning",
+    artifacts = [
+        "com.fasterxml.jackson.core:jackson-annotations:2.10.1",
+        "com.fasterxml.jackson.core:jackson-annotations:2.12.1",
+        "com.fasterxml.jackson.core:jackson-annotations:2.10.1",
+        "com.fasterxml.jackson.core:jackson-annotations:2.11.2",
+        "com.github.jnr:jffi:1.3.4",
+        maven.artifact(
+            group = "com.github.jnr",
+            artifact = "jffi",
+            version = "1.3.3",
+            classifier = "native",
+        ),
+        maven.artifact(
+            group = "com.github.jnr",
+            artifact = "jffi",
+            version = "1.3.2",
+            classifier = "native",
+        ),
+    ],
+    repositories = [
+        "https://repo1.maven.org/maven2",
+        "https://maven.google.com",
+    ],
+)
+
 maven_install(
     name = "starlark_aar_import_test",
     artifacts = [

coursier.bzl

@@ -372,6 +372,8 @@ def _pinned_coursier_fetch_impl(repository_ctx):
     for artifact in repository_ctx.attr.artifacts:
         artifacts.append(json_parse(artifact))
 
+    _check_artifacts_are_unique(artifacts, repository_ctx.attr.duplicate_version_warning)
+
     # Read Coursier state from maven_install.json.
     repository_ctx.symlink(
         repository_ctx.path(repository_ctx.attr.maven_install_json),
@@ -606,6 +608,31 @@ def _deduplicate_artifacts(dep_tree):
     dep_tree.update({"dependencies": deduped_artifacts.values() + null_artifacts})
     return dep_tree
 
+def _check_artifacts_are_unique(artifacts, duplicate_version_warning):
+    if duplicate_version_warning == "none":
+        return
+    seen_artifacts = {}
+    duplicate_artifacts = {}
+    for artifact in artifacts:
+        artifact_coordinate = artifact["group"] + ":" + artifact["artifact"] + (":%s" % artifact["classifier"] if artifact.get("classifier") != None else "")
+        if artifact_coordinate in seen_artifacts:
+            if artifact_coordinate in duplicate_artifacts:
+                duplicate_artifacts[artifact_coordinate].append(artifact["version"])
+            else:
+                duplicate_artifacts[artifact_coordinate] = [artifact["version"]]
+        else:
+            seen_artifacts[artifact_coordinate] = artifact["version"]
+
+    if duplicate_artifacts:
+        msg_parts = ["Found duplicate artifact versions"]
+        for duplicate in duplicate_artifacts:
+            msg_parts.append("    {} has multiple versions {}".format(duplicate, ", ".join([seen_artifacts[duplicate]] + duplicate_artifacts[duplicate])))
+        msg_parts.append("Please remove duplicate artifacts from the artifact list so you do not get unexpected artifact versions")
+        if duplicate_version_warning == "error":
+            fail("\n".join(msg_parts))
+        else:
+            print("\n".join(msg_parts))
+
 # Get the path to the cache directory containing Coursier-downloaded artifacts.
 #
 # This method is public for testing.
@@ -779,6 +806,8 @@ def _coursier_fetch_impl(repository_ctx):
     for artifact in repository_ctx.attr.artifacts:
         artifacts.append(json_parse(artifact))
 
+    _check_artifacts_are_unique(artifacts, repository_ctx.attr.duplicate_version_warning)
+
     excluded_artifacts = []
     for artifact in repository_ctx.attr.excluded_artifacts:
         excluded_artifacts.append(json_parse(artifact))
@@ -1093,6 +1122,20 @@ pinned_coursier_fetch = repository_rule(
         "fail_if_repin_required": attr.bool(doc = "Whether to fail the build if the maven_artifact inputs have changed but the lock file has not been repinned.", default = False),
         "use_starlark_android_rules": attr.bool(default = False, doc = "Whether to use the native or Starlark version of the Android rules."),
         "aar_import_bzl_label": attr.string(default = DEFAULT_AAR_IMPORT_LABEL, doc = "The label (as a string) to use to import aar_import from"),
+        "duplicate_version_warning": attr.string(
+            doc = """What to do if there are duplicate artifacts
+
+            If "error", then print a message and fail the build.
+            If "warn", then print a warning and continue.
+            If "none", then do nothing.
+            """,
+            default = "warn",
+            values = [
+                "error",
+                "warn",
+                "none",
+            ],
+        ),
     },
     implementation = _pinned_coursier_fetch_impl,
 )
@@ -1138,6 +1181,20 @@ coursier_fetch = repository_rule(
         "jetify_include_list": attr.string_list(doc = "List of artifacts that need to be jetified in `groupId:artifactId` format. By default all artifacts are jetified if `jetify` is set to True.", default = JETIFY_INCLUDE_LIST_JETIFY_ALL),
         "use_starlark_android_rules": attr.bool(default = False, doc = "Whether to use the native or Starlark version of the Android rules."),
         "aar_import_bzl_label": attr.string(default = DEFAULT_AAR_IMPORT_LABEL, doc = "The label (as a string) to use to import aar_import from"),
+        "duplicate_version_warning": attr.string(
+            doc = """What to do if there are duplicate artifacts
+
+            If "error", then print a message and fail the build.
+            If "warn", then print a warning and continue.
+            If "none", then do nothing.
+            """,
+            default = "warn",
+            values = [
+                "error",
+                "warn",
+                "none",
+            ],
+        ),
     },
     environ = [
         "JAVA_HOME",

defs.bzl

@@ -42,7 +42,8 @@ def maven_install(
         additional_netrc_lines = [],
         fail_if_repin_required = False,
         use_starlark_android_rules = False,
-        aar_import_bzl_label = DEFAULT_AAR_IMPORT_LABEL):
+        aar_import_bzl_label = DEFAULT_AAR_IMPORT_LABEL,
+        duplicate_version_warning = "warn"):
     """Resolves and fetches artifacts transitively from Maven repositories.
 
     This macro runs a repository rule that invokes the Coursier CLI to resolve
@@ -86,6 +87,9 @@ def maven_install(
         not use the typical default repository name to import the Android
         Starlark rules. Default is
         "@build_bazel_rules_android//rules:rules.bzl".
+      duplicate_version_warning: What to do if an artifact is specified multiple times. If "error" then
+        fail the build, if "warn" then print a message and continue, if "none" then do nothing. The default
+        is "warn".
     """
     repositories_json_strings = []
     for repository in parse.parse_repository_spec_list(repositories):
@@ -136,6 +140,7 @@ def maven_install(
         jetify_include_list = jetify_include_list,
         use_starlark_android_rules = use_starlark_android_rules,
         aar_import_bzl_label = aar_import_bzl_label,
+        duplicate_version_warning = duplicate_version_warning,
     )
 
     if maven_install_json != None:
@@ -154,6 +159,7 @@ def maven_install(
             jetify_include_list = jetify_include_list,
             additional_netrc_lines = additional_netrc_lines,
             fail_if_repin_required = fail_if_repin_required,
+            duplicate_version_warning = duplicate_version_warning,
         )
 
 def artifact(a, repository_name = DEFAULT_REPOSITORY_NAME):

tests/bazel_run_tests.sh

@@ -0,0 +1,61 @@
+#!/bin/bash -e
+
+# A simple test framework to verify bazel output without setting up an entire WORKSPACE
+# in the bazel sandbox as is done in https://github.com/bazelbuild/bazel/blob/master/src/test/shell/unittest.bash
+#
+# Add a new test to the TESTS array and send all output to TEST_LOG
+
+function test_duplicate_version_warning() {
+  bazel run @duplicate_version_warning//:pin >> "$TEST_LOG" 2>&1
+    
+  expect_log "Found duplicate artifact versions"
+  expect_log "com.fasterxml.jackson.core:jackson-annotations has multiple versions"
+  expect_log "com.github.jnr:jffi:native has multiple versions"
+}
+
+function test_outdated() {
+  bazel run @regression_testing//:outdated >> "$TEST_LOG" 2>&1
+
+  expect_log "Checking for updates of .* artifacts against the following repositories"
+  expect_log "junit:junit \[4.12"
+}
+
+TESTS=("test_duplicate_version_warning" "test_outdated")
+
+function run_tests() {
+  printf "Running bazel run tests:\n"
+  for test in ${TESTS[@]}; do
+    printf "  ${test} "
+    TEST_LOG=/tmp/${test}_test.log
+    rm -f "$TEST_LOG"
+    DUMPED_TEST_LOG=0
+    ${test}
+    printf "PASSED\n"
+    rm -f "$TEST_LOG"
+  done
+}
+
+function expect_log() {
+  local pattern=$1
+  local message=${2:-Expected regexp "$pattern" not found}
+  grep -sq -- "$pattern" $TEST_LOG && return 0
+
+  printf "FAILED\n"
+  cat $TEST_LOG
+  DUMPED_TEST_LOG=1
+  printf "FAILURE: $message\n"
+  return 1
+}
+
+function exit_handler() {
+  local exit_code=$?
+  if [ $exit_code != "0" ] && [ $DUMPED_TEST_LOG == "0" ]; then
+    printf "ERROR\n"
+    cat $TEST_LOG
+  fi
+  return $exit_code
+}
+
+trap exit_handler EXIT
+
+run_tests