Skip to content

Commit

Permalink
Add workspace_prefix_for_pinned_deps
Browse files Browse the repository at this point in the history
When artifacts are pinned, `maven_install` would create individual
external repos for each dependency. The name of such an external repo is
simply the maven coordinate with all punctuation replaced by `_`. This
can cause name conflicts for code bases that are already using
`http_archive` to download thousands of dependencies through some other
mechanism but would like to migrate to use `maven_install`.

This PR tries to fix this by specifying a prefix for intermediate
external repos so that name collision becomes manageable.
  • Loading branch information
tgeng committed Sep 20, 2022
1 parent 182cd67 commit 242ddfb
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
10 changes: 9 additions & 1 deletion coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ def _pinned_coursier_fetch_impl(repository_ctx):

for artifact in dep_tree["dependencies"]:
if artifact.get("url") != None:
http_file_repository_name = escape(artifact["coord"])
http_file_repository_name = repository_ctx.attr.workspace_prefix_for_pinned_deps + escape(artifact["coord"])
maven_artifacts.extend([artifact["coord"]])
http_files.extend([
" http_file(",
Expand Down Expand Up @@ -1259,6 +1259,10 @@ pinned_coursier_fetch = repository_rule(
"none",
],
),
"workspace_prefix_for_pinned_deps": attr.string(
doc = """Prefix to use for names of http_file if using pinned dependencies. For example, with the default empty string, `maven_install` creates intermediate dependencies like `@junit_junit`. With a prefix "my_deps_", the intermediate workspace names becomes `@my_deps_junit_junit`. This is useful to avoid workspace name pollution during migration.""",
default = "",
),
},
implementation = _pinned_coursier_fetch_impl,
)
Expand Down Expand Up @@ -1320,6 +1324,10 @@ coursier_fetch = repository_rule(
"none",
],
),
"workspace_prefix_for_pinned_deps": attr.string(
doc = """Prefix to use for names of http_file if using pinned dependencies. For example, with the default empty string, `maven_install` creates intermediate dependencies like `@junit_junit`. With a prefix "my_deps_", the intermediate workspace names becomes `@my_deps_junit_junit`. This is useful to avoid workspace name pollution during migration.""",
default = "",
),
},
environ = [
"JAVA_HOME",
Expand Down
10 changes: 5 additions & 5 deletions private/dependency_tree_parser.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ load(

JETIFY_INCLUDE_LIST_JETIFY_ALL = ["*"]

def _genrule_copy_artifact_from_http_file(artifact, visibilities):
def _genrule_copy_artifact_from_http_file(artifact, visibilities, workspace_prefix_for_pinned_deps):
# skip artifacts without any urls (ie: maven local artifacts)
if not artifact.get("url"):
return ""
http_file_repository = escape(artifact["coord"])
http_file_repository = workspace_prefix_for_pinned_deps + escape(artifact["coord"])
return "\n".join([
"genrule(",
" name = \"%s_extension\"," % http_file_repository,
Expand Down Expand Up @@ -97,7 +97,7 @@ def _generate_imports(repository_ctx, dep_tree, explicit_artifacts, neverlink_ar
target_label = escape(strip_packaging_and_classifier_and_version(artifact["coord"]))
srcjar_paths[target_label] = artifact_path
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities, repository_ctx.attr.workspace_prefix_for_pinned_deps))

jetify_all = repository_ctx.attr.jetify and repository_ctx.attr.jetify_include_list == JETIFY_INCLUDE_LIST_JETIFY_ALL

Expand Down Expand Up @@ -141,7 +141,7 @@ def _generate_imports(repository_ctx, dep_tree, explicit_artifacts, neverlink_ar
)
if repository_ctx.attr.maven_install_json:
# Provide the downloaded artifact as a file target.
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities, repository_ctx.attr.workspace_prefix_for_pinned_deps))
elif artifact_path != None:
seen_imports[target_label] = True

Expand Down Expand Up @@ -356,7 +356,7 @@ def _generate_imports(repository_ctx, dep_tree, explicit_artifacts, neverlink_ar
# cmd = "cp $< $@",
# )
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities, repository_ctx.attr.workspace_prefix_for_pinned_deps))

else: # artifact_path == None:
# Special case for certain artifacts that only come with a POM file.
Expand Down
9 changes: 8 additions & 1 deletion private/rules/maven_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ def maven_install(
fail_if_repin_required = False,
use_starlark_android_rules = False,
aar_import_bzl_label = DEFAULT_AAR_IMPORT_LABEL,
duplicate_version_warning = "warn"):
duplicate_version_warning = "warn",
workspace_prefix_for_pinned_deps = "",
):
"""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 @@ -74,6 +76,10 @@ def maven_install(
duplicate_version_warning: What to do if an artifact is specified multiple times. If "error" then
fail the build, if "warn" then print a message and continue, if "none" then do nothing. The default
is "warn".
workspace_prefix_for_pinned_deps: Prefix to use for names of http_file if using pinned dependencies.
For example, with the default empty string, `maven_install` creates intermediate dependencies like
`@junit_junit`. With a prefix "my_deps_", the intermediate workspace names becomes
`@my_deps_junit_junit`. This is useful to avoid workspace name pollution during migration.
"""
repositories_json_strings = []
for repository in parse.parse_repository_spec_list(repositories):
Expand Down Expand Up @@ -148,4 +154,5 @@ def maven_install(
duplicate_version_warning = duplicate_version_warning,
use_starlark_android_rules = use_starlark_android_rules,
aar_import_bzl_label = aar_import_bzl_label,
workspace_prefix_for_pinned_deps = workspace_prefix_for_pinned_deps,
)
1 change: 1 addition & 0 deletions repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ def rules_jvm_external_deps(repositories = _DEFAULT_REPOSITORIES):
fail_if_repin_required = True,
strict_visibility = True,
repositories = repositories,
workspace_prefix_for_pinned_deps = "_rules_jvm_external_deps",
)

0 comments on commit 242ddfb

Please sign in to comment.