From 806ac7f059cff4849de1186d80c39094b2059912 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Tue, 23 Jul 2024 16:47:02 -0700 Subject: [PATCH 1/3] Factor detecting whether repo is unpinned into a shared helper --- private/rules/coursier.bzl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/private/rules/coursier.bzl b/private/rules/coursier.bzl index 47bcfe6b..2bca1543 100644 --- a/private/rules/coursier.bzl +++ b/private/rules/coursier.bzl @@ -137,6 +137,9 @@ def _is_file(repository_ctx, path): def _is_directory(repository_ctx, path): return repository_ctx.which("test") and repository_ctx.execute(["test", "-d", path]).return_code == 0 +def _is_unpinned(repository_ctx): + return repository_ctx.attr.name.startswith("unpinned_") + def _shell_quote(s): # Lifted from # https://github.com/bazelbuild/bazel-skylib/blob/6a17363a3c27dde70ab5002ad9f2e29aff1e1f4b/lib/shell.bzl#L49 @@ -875,7 +878,7 @@ def make_coursier_dep_tree( cmd.append("--default=true") environment = {} - if not repository_ctx.attr.name.startswith("unpinned_"): + if not _is_unpinned(repository_ctx): coursier_cache_location = get_coursier_cache_or_default( repository_ctx, False, @@ -1009,7 +1012,7 @@ def _coursier_fetch_impl(repository_ctx): # TODO(jin): allow custom cache locations coursier_cache_path = get_coursier_cache_or_default( repository_ctx, - repository_ctx.attr.name.startswith("unpinned_"), + _is_unpinned(repository_ctx), ).replace("//", "/") for artifact in dep_tree["dependencies"]: @@ -1032,13 +1035,13 @@ def _coursier_fetch_impl(repository_ctx): # to file within the repository rule workspace print("Assuming maven local for artifact: %s" % artifact["coord"]) artifact.update({"url": None}) - if not repository_ctx.attr.name.startswith("unpinned_"): + if not _is_unpinned(repository_ctx): artifact.update({"file": _relativize_and_symlink_file_in_maven_local(repository_ctx, artifact["file"])}) files_to_inspect.append(repository_ctx.path(artifact["file"])) continue - if repository_ctx.attr.name.startswith("unpinned_"): + if _is_unpinned(repository_ctx): artifact.update({"file": _relativize_and_symlink_file_in_coursier_cache(repository_ctx, artifact["file"], coursier_cache_path)}) # Coursier saves the artifacts into a subdirectory structure @@ -1231,12 +1234,12 @@ def _coursier_fetch_impl(repository_ctx): }, override_targets = repository_ctx.attr.override_targets, # Skip maven local dependencies if generating the unpinned repository - skip_maven_local_dependencies = repository_ctx.attr.name.startswith("unpinned_"), + skip_maven_local_dependencies = _is_unpinned(repository_ctx), ) # This repository rule can be either in the pinned or unpinned state, depending on when # the user invokes artifact pinning. Normalize the repository name here. - if repository_ctx.name.startswith("unpinned_"): + if _is_unpinned(repository_ctx): repository_name = repository_ctx.name[len("unpinned_"):] outdated_build_file_content = "" else: From 1afeb6a3348f80fab04525223bc5968b12fab758 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Tue, 23 Jul 2024 22:40:26 -0700 Subject: [PATCH 2/3] Track whether a repo is pinned or unpinned by explicit attr Doing this by performing string interpretation on the repo name is a bit brittle. And it works less well under Bzlmod where the unpinned repo name is going to look something like `rules_jvm_external~~maven~unpinned_maven`. After this change, the lockfile pinning script, `@maven//:pin` will use the local machine's Coursier cache (`~/Library/Caches/Coursier` on MacOS) even under Bzlmod. A populated cache makes version resolutions substantially faster to obtain. --- private/extensions/maven.bzl | 1 + private/rules/coursier.bzl | 8 ++++++-- private/rules/maven_install.bzl | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl index 0720d37f..5b63f9a0 100644 --- a/private/extensions/maven.bzl +++ b/private/extensions/maven.bzl @@ -412,6 +412,7 @@ def _maven_impl(mctx): # created from the maven_install.json file in the coursier_fetch # invocation after this. name = "unpinned_" + name if repo.get("lock_file") else name, + pinned_repo_name = name if repo.get("lock_file") else None, user_provided_name = name, repositories = repo.get("repositories"), artifacts = artifacts_json, diff --git a/private/rules/coursier.bzl b/private/rules/coursier.bzl index 2bca1543..9594062f 100644 --- a/private/rules/coursier.bzl +++ b/private/rules/coursier.bzl @@ -138,7 +138,7 @@ def _is_directory(repository_ctx, path): return repository_ctx.which("test") and repository_ctx.execute(["test", "-d", path]).return_code == 0 def _is_unpinned(repository_ctx): - return repository_ctx.attr.name.startswith("unpinned_") + return repository_ctx.attr.pinned_repo_name != "" def _shell_quote(s): # Lifted from @@ -1240,7 +1240,7 @@ def _coursier_fetch_impl(repository_ctx): # This repository rule can be either in the pinned or unpinned state, depending on when # the user invokes artifact pinning. Normalize the repository name here. if _is_unpinned(repository_ctx): - repository_name = repository_ctx.name[len("unpinned_"):] + repository_name = repository_ctx.attr.pinned_repo_name outdated_build_file_content = "" else: repository_name = repository_ctx.name @@ -1442,6 +1442,10 @@ coursier_fetch = repository_rule( ), "ignore_empty_files": attr.bool(default = False, doc = "Treat jars that are empty as if they were not found."), "additional_coursier_options": attr.string_list(doc = "Additional options that will be passed to coursier."), + "pinned_repo_name": attr.string( + doc = "Name of the corresponding pinned repo for this repo. Presence implies that this is an unpinned repo.", + mandatory = False, + ), }, environ = [ "JAVA_HOME", diff --git a/private/rules/maven_install.bzl b/private/rules/maven_install.bzl index b94e04a2..3378e0c8 100644 --- a/private/rules/maven_install.bzl +++ b/private/rules/maven_install.bzl @@ -123,6 +123,7 @@ def maven_install( # created from the maven_install.json file in the coursier_fetch # invocation after this. name = name if maven_install_json == None else "unpinned_" + name, + pinned_repo_name = None if maven_install_json == None else name, repositories = repositories_json_strings, artifacts = artifacts_json_strings, fail_on_missing_checksum = fail_on_missing_checksum, From 58b716ff1aef9a641a48738ffa3219fdd9119588 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Wed, 24 Jul 2024 15:40:03 -0700 Subject: [PATCH 3/3] Fix errant tab in indentation Tab added by misconfigured editor. --- private/extensions/maven.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl index 5b63f9a0..a97b1913 100644 --- a/private/extensions/maven.bzl +++ b/private/extensions/maven.bzl @@ -412,7 +412,7 @@ def _maven_impl(mctx): # created from the maven_install.json file in the coursier_fetch # invocation after this. name = "unpinned_" + name if repo.get("lock_file") else name, - pinned_repo_name = name if repo.get("lock_file") else None, + pinned_repo_name = name if repo.get("lock_file") else None, user_provided_name = name, repositories = repo.get("repositories"), artifacts = artifacts_json,