Skip to content

Commit

Permalink
Detect when pinned inputs have changed and stop the build
Browse files Browse the repository at this point in the history
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 `MAVEN_SKIP_CHECKS`
environment variable is checked. This allows one to update the inputs
without needing to modify the `WORKSPACE` file.
  • Loading branch information
shs96c committed Jan 8, 2021
1 parent 8ca81e0 commit 519b280
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 35 deletions.
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,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
Expand Down
50 changes: 48 additions & 2 deletions coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -301,6 +316,12 @@ def _add_outdated_files(repository_ctx, artifacts, repositories):
executable = True,
)

def _is_failing_build_if_inputs_change(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." +
Expand Down Expand Up @@ -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 = compute_dependency_inputs_signature(repository_ctx.attr.artifacts)
if computed != dep_tree.get("__INPUT_ARTIFACTS_HASH"):
if _is_failing_build_if_inputs_change(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 +
Expand Down Expand Up @@ -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..")
Expand Down Expand Up @@ -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,
)
Expand Down
5 changes: 4 additions & 1 deletion defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
10 changes: 7 additions & 3 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 |


Expand All @@ -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.
Expand All @@ -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> |
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion maven_install.json
Original file line number Diff line number Diff line change
@@ -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": [
{
Expand Down
1 change: 1 addition & 0 deletions repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Loading

0 comments on commit 519b280

Please sign in to comment.