Include excluded_artifacts in lockfile input hash (bazelbuild#954)

coursier.bzl

@@ -283,22 +283,39 @@ def _windows_check(repository_ctx):
                  "This is typically `c:\\msys64\\usr\\bin\\bash.exe`. For more information, read " +
                  "https://docs.bazel.build/versions/master/install-windows.html#getting-bazel")
 
+def _stable_artifact(artifact):
+    parsed = json.decode(artifact)
+
+    # Sort the keys to provide a stable order
+    keys = sorted(parsed.keys())
+    return ":".join(["%s=%s" % (key, parsed[key]) for key in keys])
+
 # Compute a signature of the list of artifacts that will be used to build
 # the dependency tree. This is used as a check to see whether the dependency
 # tree needs to be repinned.
+# Returns a tuple where the first element is the currently used hash, and the
+# second element is a list of hashes in previous formats. This is to allow for
+# upgrading rules_jvm_external when the hash inputs change.
 #
 # Visible for testing
-def compute_dependency_inputs_signature(artifacts, repositories):
+def compute_dependency_inputs_signature(artifacts, repositories, excluded_artifacts):
     artifact_inputs = []
+    excluded_artifact_inputs = []
 
     for artifact in sorted(artifacts):
-        parsed = json.decode(artifact)
+        artifact_inputs.append(_stable_artifact(artifact))
+
+    for artifacts in sorted(excluded_artifacts):
+        excluded_artifact_inputs.append(_stable_artifact(artifacts))
+
+    v1_sig = hash(repr(sorted(artifact_inputs))) ^ hash(repr(sorted(repositories)))
+
+    hash_parts = [sorted(artifact_inputs), sorted(repositories), sorted(excluded_artifact_inputs)]
+    current_version_sig = 0
+    for part in hash_parts:
+        current_version_sig ^= hash(repr(part))
 
-        # Sort the keys to provide a stable order
-        keys = sorted(parsed.keys())
-        flattened = ":".join(["%s=%s" % (key, parsed[key]) for key in keys])
-        artifact_inputs.append(flattened)
-    return hash(repr(sorted(artifact_inputs))) ^ hash(repr(sorted(repositories)))
+    return (current_version_sig, [v1_sig])
 
 def get_netrc_lines_from_entries(netrc_entries):
     netrc_lines = []
@@ -412,18 +429,30 @@ def _pinned_coursier_fetch_impl(repository_ctx):
               "This feature ensures that the build does not use stale dependencies when the inputs " +
               "have changed. To generate this signature, run 'bazel run %s'." % pin_target)
     else:
-        computed_artifacts_hash = compute_dependency_inputs_signature(repository_ctx.attr.artifacts, repository_ctx.attr.repositories)
-        if computed_artifacts_hash != input_artifacts_hash:
+        computed_artifacts_hash, old_hashes = compute_dependency_inputs_signature(
+            repository_ctx.attr.artifacts,
+            repository_ctx.attr.repositories,
+            repository_ctx.attr.excluded_artifacts,
+        )
+        repin_instructions = (
+            " REPIN=1 bazel run %s\n" % pin_target +
+            "or:\n" +
+            " 1) Set 'fail_if_repin_required' to 'False' in 'maven_install'\n" +
+            " 2) Run 'bazel run %s'\n" % pin_target +
+            " 3) Reset 'fail_if_repin_required' to 'True' in 'maven_install'\n\n"
+        )
+        if input_artifacts_hash in old_hashes:
+            print(
+                "WARNING: %s_install.json contains an outdated input signature. " % (user_provided_name) +
+                "It is recommended that you regenerate it by running either:\n" + repin_instructions,
+            )
+        elif computed_artifacts_hash != input_artifacts_hash:
             if _fail_if_repin_required(repository_ctx):
                 fail("%s_install.json contains an invalid input signature and must be regenerated. " % (user_provided_name) +
                      "This typically happens when the maven_install artifacts have been changed but not repinned. " +
                      "PLEASE DO NOT MODIFY THIS FILE DIRECTLY! To generate a new " +
                      "%s_install.json and re-pin the artifacts, either run:\n" % user_provided_name +
-                     " REPIN=1 bazel run %s\n" % pin_target +
-                     "or:\n" +
-                     " 1) Set 'fail_if_repin_required' to 'False' in 'maven_install'\n" +
-                     " 2) Run 'bazel run %s'\n" % pin_target +
-                     " 3) Reset 'fail_if_repin_required' to 'True' in 'maven_install'\n\n")
+                     repin_instructions)
             else:
                 print("The inputs to %s_install.json have changed, but the lock file has not been regenerated. " % user_provided_name +
                       "Consider running 'bazel run %s'" % pin_target)
@@ -1051,11 +1080,17 @@ def _coursier_fetch_impl(repository_ctx):
 
     lock_file_contents = json.decode(result.stdout)
 
+    inputs_hash, _ = compute_dependency_inputs_signature(
+        repository_ctx.attr.artifacts,
+        repository_ctx.attr.repositories,
+        repository_ctx.attr.excluded_artifacts,
+    )
+
     repository_ctx.file(
         "unsorted_deps.json",
         content = v2_lock_file.render_lock_file(
             lock_file_contents,
-            compute_dependency_inputs_signature(repository_ctx.attr.artifacts, repository_ctx.attr.repositories),
+            inputs_hash,
         ),
     )
 
@@ -1219,6 +1254,7 @@ pinned_coursier_fetch = repository_rule(
                 "none",
             ],
         ),
+        "excluded_artifacts": attr.string_list(default = []),  # only used for hash generation
     },
     implementation = _pinned_coursier_fetch_impl,
 )

maven_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": -2113228313,
+  "__INPUT_ARTIFACTS_HASH": -2113226107,
   "__RESOLVED_ARTIFACTS_HASH": 540623723,
   "artifacts": {
     "com.google.code.findbugs:jsr305": {

private/extensions/maven.bzl

@@ -376,6 +376,7 @@ def _maven_impl(mctx):
                 use_starlark_android_rules = repo.get("use_starlark_android_rules"),
                 aar_import_bzl_label = repo.get("aar_import_bzl_label"),
                 duplicate_version_warning = repo.get("duplicate_version_warning"),
+                excluded_artifacts = repo.get("excluded_artifacts"),
             )
 
 maven = module_extension(

private/rules/maven_install.bzl

@@ -152,4 +152,6 @@ def maven_install(
             use_credentials_from_home_netrc_file = use_credentials_from_home_netrc_file,
             use_starlark_android_rules = use_starlark_android_rules,
             aar_import_bzl_label = aar_import_bzl_label,
+            # Extra arguments only used for hash generation
+            excluded_artifacts = excluded_artifacts_json_strings,
         )

tests/custom_maven_install/java_export_exclusion_testing_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": 1917486889,
+  "__INPUT_ARTIFACTS_HASH": 1917488203,
   "__RESOLVED_ARTIFACTS_HASH": 1233788803,
   "artifacts": {
     "com.google.protobuf:protobuf-java": {

tests/custom_maven_install/json_artifacts_testing_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": 332726454,
+  "__INPUT_ARTIFACTS_HASH": 332729300,
   "__RESOLVED_ARTIFACTS_HASH": 1650741950,
   "artifacts": {
     "aopalliance:aopalliance": {

tests/custom_maven_install/manifest_stamp_testing_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": -1782480348,
+  "__INPUT_ARTIFACTS_HASH": -1782478522,
   "__RESOLVED_ARTIFACTS_HASH": 99567389,
   "artifacts": {
     "com.fasterxml.jackson.core:jackson-annotations": {

tests/custom_maven_install/maven_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": -315283510,
+  "__INPUT_ARTIFACTS_HASH": -315282264,
   "__RESOLVED_ARTIFACTS_HASH": 97194859,
   "artifacts": {
     "com.google.code.findbugs:jsr305": {

tests/custom_maven_install/policy_pinned_testing_install.json

@@ -1,6 +1,6 @@
 {
   "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
-  "__INPUT_ARTIFACTS_HASH": -92184963,
+  "__INPUT_ARTIFACTS_HASH": -92187361,
   "__RESOLVED_ARTIFACTS_HASH": -814737391,
   "artifacts": {
     "com.fasterxml.jackson.core:jackson-core": {

tests/unit/coursier_test.bzl

@@ -497,8 +497,8 @@ def _calculate_inputs_hash_does_not_care_about_input_order_test(ctx):
     ]
 
     # Order of artifacts is switched in each hash
-    hash1 = compute_dependency_inputs_signature(artifacts1, repositories1)
-    hash2 = compute_dependency_inputs_signature(artifacts2, repositories2)
+    hash1, _ = compute_dependency_inputs_signature(artifacts1, repositories1, artifacts2)
+    hash2, _ = compute_dependency_inputs_signature(artifacts2, repositories2, artifacts1)
 
     asserts.equals(env, hash1, hash2)
 
@@ -529,15 +529,44 @@ def _calculate_inputs_hash_is_different_for_different_repositories_test(ctx):
     ]
 
     # Order of artifacts is switched in each hash
-    hash1 = compute_dependency_inputs_signature(artifacts1, repositories1)
-    hash2 = compute_dependency_inputs_signature(artifacts2, repositories2)
+    hash1, _ = compute_dependency_inputs_signature(artifacts1, repositories1, [])
+    hash2, _ = compute_dependency_inputs_signature(artifacts2, repositories2, [])
 
     asserts.false(env, hash1 == hash2)
 
     return unittest.end(env)
 
 calculate_inputs_hash_is_different_for_different_repositories_test = add_test(_calculate_inputs_hash_is_different_for_different_repositories_test)
 
+def _calculate_inputs_hash_uses_excluded_artifacts_test(ctx):
+    env = unittest.begin(ctx)
+
+    artifacts1 = [
+        """{"group": "first", "artifact": "artifact", "version": "version"}""",
+        """{"group": "second", "artifact": "artifact", "version": "version"}""",
+    ]
+    repositories1 = [
+        "https://maven.google.com",
+        "https://repo1.maven.org/maven2",
+    ]
+
+    artifacts2 = [
+        """{"group": "second", "artifact": "artifact", "version": "version"}""",
+        """{"group": "first", "artifact": "artifact", "version": "version"}""",
+    ]
+
+    excluded1 = ["""{"group": "first", "artifact": "artifact", "version": "version1"}"""]
+    excluded2 = ["""{"group": "first", "artifact": "artifact", "version": "version2"}"""]
+    hash1, old_hashes1 = compute_dependency_inputs_signature(artifacts1, repositories1, excluded1)
+    hash2, old_hashes2 = compute_dependency_inputs_signature(artifacts1, repositories1, excluded2)
+
+    asserts.false(env, hash1 == hash2)
+    asserts.true(env, old_hashes1[0] == old_hashes2[0])
+
+    return unittest.end(env)
+
+calculate_inputs_hash_uses_excluded_artifacts_test = add_test(_calculate_inputs_hash_uses_excluded_artifacts_test)
+
 def coursier_test_suite():
     unittest.suite(
         "coursier_tests",