Detect when pinned inputs have changed and stop the build (#509)

Optionally force people to repin if the inputs to maven_install have
changed. This is done by adding a new `fail_if_repin_required`
attribute to `maven_install`. In order to preserve existing behaviour,
this is currently set to `False`, but users can update to `True` if
required.

If a repin is not required, a warning is printed to the console and
the build continues as expected. If a repin is required, the build is
failed and steps to rectify the situation are printed.

In addition to the `maven_install` attribute the `REPIN` environment
variable is checked. This allows one to update the inputs without
needing to modify the `WORKSPACE` file.

README.md

@@ -186,6 +186,11 @@ your project's root directory. If you do not have one, create an empty BUILD
 file to fix issues you may see. See
 [#242](https://github.com/bazelbuild/rules_jvm_external/issues/242)
 
+**Note:** If you're using an older version of `rules_jvm_external` and 
+haven't repinned your dependencies, you may see a warning that you lock
+file "does not contain a signature of the required artifacts" then don't
+worry: either ignore the warning or repin the dependencies.
+
 ### Updating `maven_install.json`
 
 Whenever you make a change to the list of `artifacts` or `repositories` and want
@@ -206,6 +211,25 @@ prefix (e.g.`@unpinned_maven` or `@unpinned_<your_maven_install_name>`). For
 example, if your `maven_install` is named `@foo`, `@unpinned_foo` will be
 created.
 
+### Requiring lock file repinning when the list of artifacts changes
+
+It can be easy to forget to update the `maven_install.json` lock file
+when updating artifacts in a `maven_install`. Normally,
+rules_jvm_external will print a warning to the console and continue
+the build when this happens, but by setting the
+`fail_if_repin_required` attribute to `True`, this will be treated as
+a build error, causing the build to fail. When this attribute is set,
+it is possible to update the `maven_install.json` file using:
+
+```shell
+$ REPIN=1 bazel run @unpinned_maven//:pin
+```
+
+Alternatively, it is also possible to modify the
+`fail_if_repin_required` attribute in your `WORKSPACE` file, run
+`bazel run @unpinned_maven//:pin` and then reset the
+`fail_if_repin_required` attribute.
+
 ### Custom location for `maven_install.json`
 
 You can specify a custom location for `maven_install.json` by changing the

coursier.bzl

@@ -183,6 +183,21 @@ def _compute_dependency_tree_signature(artifacts):
         signature_inputs.append(":".join(artifact_group))
     return hash(repr(sorted(signature_inputs)))
 
+# 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.
+#
+# Visible for testing
+def compute_dependency_inputs_signature(artifacts):
+    artifact_inputs = []
+    for artifact in artifacts:
+        parsed = json_parse(artifact)
+        # 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)))
+
 def extract_netrc_from_auth_url(url):
     """Return a dict showing the netrc machine, login, and password extracted from a url.
 
@@ -301,6 +316,12 @@ def _add_outdated_files(repository_ctx, artifacts, repositories):
         executable = True,
     )
 
+def _fail_if_repin_required(repository_ctx):
+    if not repository_ctx.attr.fail_if_repin_required:
+        return False
+
+    return "REPIN" not in repository_ctx.os.environ.keys()
+
 def _pinned_coursier_fetch_impl(repository_ctx):
     if not repository_ctx.attr.maven_install_json:
         fail("Please specify the file label to maven_install.json (e.g." +
@@ -346,7 +367,29 @@ def _pinned_coursier_fetch_impl(repository_ctx):
 
     dep_tree = maven_install_json_content["dependency_tree"]
 
-    dep_tree_signature = dep_tree.get("__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY")
+    # Then, check to see if we need to repin our deps because inputs have changed
+    if dep_tree.get("__INPUT_ARTIFACTS_HASH") == None:
+        print("NOTE: %s_install.json does not contain a signature of the required artifacts. " % repository_ctx.name +
+              "This feature ensures that the build does not use stale dependencies when the inputs " +
+              "have changed. To generate this signature, run 'bazel run @unpinned_%s//:pin'." % repository_ctx.name)
+    else:
+        computed_artifacts_hash = compute_dependency_inputs_signature(repository_ctx.attr.artifacts)
+        if computed_artifacts_hash != dep_tree.get("__INPUT_ARTIFACTS_HASH"):
+            if _fail_if_repin_required(repository_ctx):
+                fail("%s_install.json contains an invalid input signature and must be regenerated. " % (repository_ctx.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" % repository_ctx.name +
+                     " REPIN=1 bazel run @unpinned_%s//:pin\n" % repository_ctx.name +
+                     "or:\n" +
+                     " 1) Set 'fail_if_repin_required' to 'False' in 'maven_install'\n" +
+                     " 2) Run 'bazel run @unpinned_%s//:pin'\n" % repository_ctx.name +
+                    " 3) Reset 'fail_if_repin_required' to 'True' in 'maven_install'\n\n");
+            else:
+                print("The inputs to %s_install.json have changed, but the lock file has not been regenerated. " % repository_ctx.name +
+                      "Consider running 'bazel run @unpinned_%s//:pin'" % repository_ctx.name)
+
+    dep_tree_signature = dep_tree.get("__RESOLVED_ARTIFACTS_HASH")
 
     if dep_tree_signature == None:
         print("NOTE: %s_install.json does not contain a signature entry of the dependency tree. " % repository_ctx.name +
@@ -862,7 +905,9 @@ def _coursier_fetch_impl(repository_ctx):
         artifact.update({"sha256": shas[str(repository_ctx.path(file))]})
 
     dep_tree.update({
-        "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": _compute_dependency_tree_signature(dep_tree["dependencies"]),
+        "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
+        "__RESOLVED_ARTIFACTS_HASH": _compute_dependency_tree_signature(dep_tree["dependencies"]),
+        "__INPUT_ARTIFACTS_HASH": compute_dependency_inputs_signature(repository_ctx.attr.artifacts),
     })
 
     repository_ctx.report_progress("Generating BUILD targets..")
@@ -1002,6 +1047,7 @@ pinned_coursier_fetch = repository_rule(
         "jetify": attr.bool(doc = "Runs the AndroidX [Jetifier](https://developer.android.com/studio/command-line/jetifier) tool on artifacts specified in jetify_include_list. If jetify_include_list is not specified, run Jetifier on all artifacts.", default = False),
         "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),
         "additional_netrc_lines": attr.string_list(doc = "Additional lines prepended to the netrc file used by `http_file` (with `maven_install_json` only).", default = []),
+        "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),
     },
     implementation = _pinned_coursier_fetch_impl,
 )

defs.bzl

@@ -39,7 +39,8 @@ def maven_install(
         resolve_timeout = 600,
         jetify = False,
         jetify_include_list = JETIFY_INCLUDE_LIST_JETIFY_ALL,
-        additional_netrc_lines = []):
+        additional_netrc_lines = [],
+        fail_if_repin_required = False):
     """Resolves and fetches artifacts transitively from Maven repositories.
 
     This macro runs a repository rule that invokes the Coursier CLI to resolve
@@ -75,6 +76,7 @@ def maven_install(
       jetify: Runs the AndroidX [Jetifier](https://developer.android.com/studio/command-line/jetifier) tool on artifacts specified in jetify_include_list. If jetify_include_list is not specified, run Jetifier on all artifacts.
       jetify_include_list: List of artifacts that need to be jetified in `groupId:artifactId` format. By default all artifacts are jetified if `jetify` is set to True.
       additional_netrc_lines: Additional lines prepended to the netrc file used by `http_file` (with `maven_install_json` only).
+      fail_if_repin_required: Whether to fail the build if the required maven artifacts have been changed but not repinned. Requires the `maven_install_json` to have been set.
     """
     repositories_json_strings = []
     for repository in parse.parse_repository_spec_list(repositories):
@@ -140,6 +142,7 @@ def maven_install(
             jetify = jetify,
             jetify_include_list = jetify_include_list,
             additional_netrc_lines = additional_netrc_lines,
+            fail_if_repin_required = fail_if_repin_required,
         )
 
 def artifact(a, repository_name = DEFAULT_REPOSITORY_NAME):

docs/api.md

@@ -48,7 +48,7 @@ Generate a javadoc from all the `deps`
 ## java_export
 
 <pre>
-java_export(<a href="#java_export-name">name</a>, <a href="#java_export-maven_coordinates">maven_coordinates</a>, <a href="#java_export-pom_template">pom_template</a>, <a href="#java_export-visibility">visibility</a>, <a href="#java_export-kwargs">kwargs</a>)
+java_export(<a href="#java_export-name">name</a>, <a href="#java_export-maven_coordinates">maven_coordinates</a>, <a href="#java_export-pom_template">pom_template</a>, <a href="#java_export-visibility">visibility</a>, <a href="#java_export-tags">tags</a>, <a href="#java_export-kwargs">kwargs</a>)
 </pre>
 
 Extends `java_library` to allow maven artifacts to be uploaded.
@@ -99,6 +99,7 @@ Generated rules:
 | maven_coordinates |  The maven coordinates for this target.   |  none |
 | pom_template |  The template to be used for the pom.xml file.   |  <code>None</code> |
 | visibility |  The visibility of the target   |  <code>None</code> |
+| tags |  <p align="center"> - </p>   |  <code>[]</code> |
 | kwargs |  These are passed to [<code>java_library</code>](https://docs.bazel.build/versions/master/be/java.html#java_library),   and so may contain any valid parameter for that rule.   |  none |
 
 
@@ -107,10 +108,11 @@ Generated rules:
 ## maven_install
 
 <pre>
-maven_install(<a href="#maven_install-name">name</a>, <a href="#maven_install-repositories">repositories</a>, <a href="#maven_install-artifacts">artifacts</a>, <a href="#maven_install-fail_on_missing_checksum">fail_on_missing_checksum</a>, <a href="#maven_install-fetch_sources">fetch_sources</a>,
+maven_install(<a href="#maven_install-name">name</a>, <a href="#maven_install-repositories">repositories</a>, <a href="#maven_install-artifacts">artifacts</a>, <a href="#maven_install-fail_on_missing_checksum">fail_on_missing_checksum</a>, <a href="#maven_install-fetch_sources">fetch_sources</a>, <a href="#maven_install-fetch_javadoc">fetch_javadoc</a>,
               <a href="#maven_install-use_unsafe_shared_cache">use_unsafe_shared_cache</a>, <a href="#maven_install-excluded_artifacts">excluded_artifacts</a>, <a href="#maven_install-generate_compat_repositories">generate_compat_repositories</a>,
               <a href="#maven_install-version_conflict_policy">version_conflict_policy</a>, <a href="#maven_install-maven_install_json">maven_install_json</a>, <a href="#maven_install-override_targets">override_targets</a>, <a href="#maven_install-strict_visibility">strict_visibility</a>,
-              <a href="#maven_install-resolve_timeout">resolve_timeout</a>, <a href="#maven_install-jetify">jetify</a>, <a href="#maven_install-jetify_include_list">jetify_include_list</a>, <a href="#maven_install-additional_netrc_lines">additional_netrc_lines</a>)
+              <a href="#maven_install-resolve_timeout">resolve_timeout</a>, <a href="#maven_install-jetify">jetify</a>, <a href="#maven_install-jetify_include_list">jetify_include_list</a>, <a href="#maven_install-additional_netrc_lines">additional_netrc_lines</a>,
+              <a href="#maven_install-fail_if_repin_required">fail_if_repin_required</a>)
 </pre>
 
 Resolves and fetches artifacts transitively from Maven repositories.
@@ -129,6 +131,7 @@ and fetch Maven artifacts transitively.
 | artifacts |  A list of Maven artifact coordinates in the form of <code>group:artifact:version</code>.   |  <code>[]</code> |
 | fail_on_missing_checksum |  <p align="center"> - </p>   |  <code>True</code> |
 | fetch_sources |  Additionally fetch source JARs.   |  <code>False</code> |
+| fetch_javadoc |  Additionally fetch javadoc JARs.   |  <code>False</code> |
 | use_unsafe_shared_cache |  Download artifacts into a persistent shared cache on disk. Unsafe as Bazel is   currently unable to detect modifications to the cache.   |  <code>False</code> |
 | excluded_artifacts |  A list of Maven artifact coordinates in the form of <code>group:artifact</code> to be   excluded from the transitive dependencies.   |  <code>[]</code> |
 | generate_compat_repositories |  Additionally generate repository aliases in a .bzl file for all JAR   artifacts. For example, <code>@maven//:com_google_guava_guava</code> can also be referenced as   <code>@com_google_guava_guava//jar</code>.   |  <code>False</code> |
@@ -140,6 +143,7 @@ and fetch Maven artifacts transitively.
 | jetify |  Runs the AndroidX [Jetifier](https://developer.android.com/studio/command-line/jetifier) tool on artifacts specified in jetify_include_list. If jetify_include_list is not specified, run Jetifier on all artifacts.   |  <code>False</code> |
 | jetify_include_list |  List of artifacts that need to be jetified in <code>groupId:artifactId</code> format. By default all artifacts are jetified if <code>jetify</code> is set to True.   |  <code>["*"]</code> |
 | additional_netrc_lines |  Additional lines prepended to the netrc file used by <code>http_file</code> (with <code>maven_install_json</code> only).   |  <code>[]</code> |
+| fail_if_repin_required |  Whether to fail the build if the required maven artifacts have been changed but not repinned. Requires the <code>maven_install_json</code> to have been set.   |  <code>False</code> |
 
 
 # Maven specification functions

maven_install.json

@@ -1,6 +1,8 @@
 {
     "dependency_tree": {
-        "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": -13350534,
+        "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
+        "__INPUT_ARTIFACTS_HASH": 750470154,
+        "__RESOLVED_ARTIFACTS_HASH": -13350534,
         "conflict_resolution": {},
         "dependencies": [
             {

repositories.bzl

@@ -13,5 +13,6 @@ def rules_jvm_external_deps(repositories = _DEFAULT_REPOSITORIES):
             "com.google.cloud:google-cloud-storage:1.113.4",
         ],
         maven_install_json = "@rules_jvm_external//:rules_jvm_external_deps_install.json",
+        fail_if_repin_required = True,
         repositories = repositories,
     )