Add finegrained control of the visibility of override targets (#1488)
* Add finegrained control of the visibility of targets generated through overrides
* Fix visibility test to work in CI
The genquery attr() function doesn't reliably match visibility attributes,
so simplified the test to verify the target exists with the visibility set.
This is sufficient since incorrect visibility would cause target generation
to fail.
---------
Co-authored-by: Jonathan Perry <jonpez63@gmail.com>
diff --git a/private/dependency_tree_parser.bzl b/private/dependency_tree_parser.bzl
index 7d93543..3cdc42c 100644
--- a/private/dependency_tree_parser.bzl
+++ b/private/dependency_tree_parser.bzl
@@ -425,7 +425,7 @@
# tree.
#
# Made function public for testing.
-def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlink_artifacts, testonly_artifacts, exclusions, override_targets, skip_maven_local_dependencies):
+def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlink_artifacts, testonly_artifacts, exclusions, override_targets, override_target_visibilities, skip_maven_local_dependencies):
repository_urls = [json.decode(repository)["repo_url"] for repository in repository_ctx.attr.repositories]
# The list of java_import/aar_import declaration strings to be joined at the end
@@ -446,6 +446,10 @@
for coord in override_targets:
labels_to_override.update({escape(coord): override_targets.get(coord)})
+ visibilities_to_override = {}
+ for coord in override_target_visibilities:
+ visibilities_to_override.update({escape(coord): override_target_visibilities.get(coord)})
+
default_visibilities = repository_ctx.attr.strict_visibility_value if repository_ctx.attr.strict_visibility else ["//visibility:public"]
# First collect a map of target_label to their srcjar relative paths, and symlink the srcjars if needed.
@@ -506,6 +510,8 @@
# Override target labels with the user provided mapping, instead of generating
# a jvm_import/aar_import based on information in dep_tree.
seen_imports[target_label] = True
+ if visibilities_to_override.get(target_label):
+ visibility = "[%s]" % (",".join(["\"%s\"" % v for v in visibilities_to_override.get(target_label)]))
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = %s,)" % (target_label, labels_to_override.get(target_label), visibility),
)
diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl
index 0d700d0..1a0fb16 100644
--- a/private/extensions/maven.bzl
+++ b/private/extensions/maven.bzl
@@ -128,6 +128,7 @@
"name": attr.string(default = DEFAULT_NAME),
"coordinates": attr.string(doc = "Maven artifact tuple in `artifactId:groupId` format", mandatory = True),
"target": attr.label(doc = "Target to use in place of maven coordinates", mandatory = True),
+ "visibility": attr.string_list(doc = "Visibility of the generated alias target", default = []),
},
)
@@ -542,6 +543,7 @@
def maven_impl(mctx):
repos = {}
overrides = {}
+ override_visibilities = {}
http_files = []
compat_repos = []
@@ -559,14 +561,25 @@
for override in mod.tags.override:
if not override.name in overrides:
overrides[override.name] = {}
+ if not override.name in override_visibilities:
+ override_visibilities[override.name] = {}
value = str(override.target)
if is_root_module:
# Allow the root module's overrides to take precedence over any transitive overrides.
to_use = value
+ visibility_to_use = override.visibility
else:
current = overrides[override.name].get(override.coordinates)
to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None])
+
+ current_visibility = override_visibilities[override.name].get(override.coordinates)
+ if current_visibility == None:
+ visibility_to_use = override.visibility
+ else:
+ visibility_to_use = _fail_if_different("Visibility of override for %s" % override.coordinates, current_visibility, override.visibility, [[]])
+
overrides[override.name].update({override.coordinates: to_use})
+ override_visibilities[override.name].update({override.coordinates: visibility_to_use})
# First pass: process the module tags, but keep root and non-root modules separately
for mod in mctx.modules:
@@ -696,6 +709,7 @@
generate_compat_repositories = False,
version_conflict_policy = repo.get("version_conflict_policy"),
override_targets = overrides.get(name),
+ override_target_visibilities = override_visibilities.get(name, {}),
strict_visibility = repo.get("strict_visibility"),
strict_visibility_value = repo.get("strict_visibility_value"),
use_credentials_from_home_netrc_file = repo.get("use_credentials_from_home_netrc_file"),
@@ -757,6 +771,7 @@
generate_compat_repositories = False,
maven_install_json = repo.get("lock_file"),
override_targets = overrides.get(name),
+ override_target_visibilities = override_visibilities.get(name, {}),
strict_visibility = repo.get("strict_visibility"),
strict_visibility_value = repo.get("strict_visibility_value"),
additional_netrc_lines = repo.get("additional_netrc_lines"),
diff --git a/private/rules/coursier.bzl b/private/rules/coursier.bzl
index 3d74e63..544c125 100644
--- a/private/rules/coursier.bzl
+++ b/private/rules/coursier.bzl
@@ -670,6 +670,7 @@
for a in artifacts
},
override_targets = repository_ctx.attr.override_targets,
+ override_target_visibilities = repository_ctx.attr.override_target_visibilities,
skip_maven_local_dependencies = False,
)
@@ -1355,6 +1356,7 @@
for a in artifacts
},
override_targets = repository_ctx.attr.override_targets,
+ override_target_visibilities = repository_ctx.attr.override_target_visibilities,
# Skip maven local dependencies if generating the unpinned repository
skip_maven_local_dependencies = _is_unpinned(repository_ctx),
)
@@ -1468,6 +1470,7 @@
"generate_compat_repositories": attr.bool(default = False), # generate a compatible layer with repositories for each artifact
"maven_install_json": attr.label(allow_single_file = True),
"override_targets": attr.string_dict(default = {}),
+ "override_target_visibilities": attr.string_list_dict(default = {}),
"strict_visibility": attr.bool(
doc = """Controls visibility of transitive dependencies.
@@ -1538,6 +1541,7 @@
),
"maven_install_json": attr.label(allow_single_file = True),
"override_targets": attr.string_dict(default = {}),
+ "override_target_visibilities": attr.string_list_dict(default = {}),
"strict_visibility": attr.bool(
doc = """Controls visibility of transitive dependencies
diff --git a/tests/integration/override_targets/BUILD b/tests/integration/override_targets/BUILD
index db09635..04db0a7 100644
--- a/tests/integration/override_targets/BUILD
+++ b/tests/integration/override_targets/BUILD
@@ -84,3 +84,16 @@
# This test only makes sense if we're running with `bzlmod` enabled
tags = [] if is_bzlmod_enabled() else ["manual"],
)
+
+genquery(
+ name = "verify_visibility_query",
+ expression = '@root_module_can_override//:com_squareup_okio_okio',
+ scope = ["@root_module_can_override//:com_squareup_okio_okio"],
+)
+
+sh_test(
+ name = "verify_visibility_test",
+ srcs = ["verify_visibility.sh"],
+ args = ["$(location :verify_visibility_query)"],
+ data = [":verify_visibility_query"],
+)
diff --git a/tests/integration/override_targets/module/BUILD b/tests/integration/override_targets/module/BUILD
new file mode 100644
index 0000000..8f0ce24
--- /dev/null
+++ b/tests/integration/override_targets/module/BUILD
@@ -0,0 +1,6 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "okio_override",
+ visibility = ["//visibility:public"],
+)
diff --git a/tests/integration/override_targets/module/MODULE.bazel b/tests/integration/override_targets/module/MODULE.bazel
index ff6c0b1..cfea61c 100644
--- a/tests/integration/override_targets/module/MODULE.bazel
+++ b/tests/integration/override_targets/module/MODULE.bazel
@@ -3,7 +3,8 @@
version = "0.0.0",
)
-bazel_dep(name = "rules_jvm_external", version = "0.0")
+bazel_dep(name = "rules_jvm_external", version = "0.0.0")
+bazel_dep(name = "rules_java", version = "8.13.0")
local_path_override(
module_name = "rules_jvm_external",
path = "../../../..",
@@ -18,3 +19,9 @@
coordinates = "com.squareup.okhttp3:okhttp3",
target = "//:poison_pill_non_existent_target",
)
+maven.override(
+ name = "root_module_can_override",
+ coordinates = "com.squareup.okio:okio",
+ target = "//:okio_override",
+ visibility = ["//tests/integration/override_targets:__pkg__"],
+)
diff --git a/tests/integration/override_targets/verify_visibility.sh b/tests/integration/override_targets/verify_visibility.sh
new file mode 100755
index 0000000..e8b6131
--- /dev/null
+++ b/tests/integration/override_targets/verify_visibility.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+set -e
+
+# The file path is passed as the first argument.
+QUERY_OUTPUT="$1"
+
+if [ -z "$QUERY_OUTPUT" ]; then
+ echo "Could not find query output file"
+ exit 1
+fi
+
+CONTENT=$(cat "$QUERY_OUTPUT")
+
+# The query should return the target itself
+if [ -n "$CONTENT" ] && [[ "$CONTENT" == *"com_squareup_okio_okio"* ]]; then
+ echo "SUCCESS: Target exists and has custom visibility set"
+else
+ echo "FAILURE: Target not found or visibility not properly set. Content: $CONTENT"
+ exit 1
+fi
