Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
shs96c marked this conversation as resolved.
Show resolved Hide resolved
shs96c marked this conversation as resolved.
Show resolved Hide resolved
shs96c marked this conversation as resolved.
Show resolved Hide resolved
```

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 _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." +
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_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 +
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"]),
shs96c marked this conversation as resolved.
Show resolved Hide resolved
"__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