fix(toolchain): restrict coverage tool visibility under bzlmod (#1252)
Before this PR the `coverage_tool` automatically registered by
`rules_python`
was visible outside the toolchain repository. This fixes it to be
consistent
with `non-bzlmod` setups and ensures that the default `coverage_tool` is
not
visible outside the toolchain repos.
This means that the `MODULE.bazel` file can be cleaned-up at the expense
of
relaxing the `coverage_tool` attribute for the `python_repository` to be
a
simple string as the label would be evaluated within the context of
`rules_python` which may not necessarily resolve correctly without the
`use_repo` statement in our `MODULE.bazel`.
diff --git a/MODULE.bazel b/MODULE.bazel
index 6729d09..b7a0411 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -29,23 +29,6 @@
"pypi__tomli",
"pypi__wheel",
"pypi__zipp",
- # coverage_deps managed by running ./tools/update_coverage_deps.py <version>
- "pypi__coverage_cp310_aarch64-apple-darwin",
- "pypi__coverage_cp310_aarch64-unknown-linux-gnu",
- "pypi__coverage_cp310_x86_64-apple-darwin",
- "pypi__coverage_cp310_x86_64-unknown-linux-gnu",
- "pypi__coverage_cp311_aarch64-apple-darwin",
- "pypi__coverage_cp311_aarch64-unknown-linux-gnu",
- "pypi__coverage_cp311_x86_64-apple-darwin",
- "pypi__coverage_cp311_x86_64-unknown-linux-gnu",
- "pypi__coverage_cp38_aarch64-apple-darwin",
- "pypi__coverage_cp38_aarch64-unknown-linux-gnu",
- "pypi__coverage_cp38_x86_64-apple-darwin",
- "pypi__coverage_cp38_x86_64-unknown-linux-gnu",
- "pypi__coverage_cp39_aarch64-apple-darwin",
- "pypi__coverage_cp39_aarch64-unknown-linux-gnu",
- "pypi__coverage_cp39_x86_64-apple-darwin",
- "pypi__coverage_cp39_x86_64-unknown-linux-gnu",
)
# We need to do another use_extension call to expose the "pythons_hub"
diff --git a/examples/bzlmod/description.md b/examples/bzlmod/description.md
new file mode 100644
index 0000000..a5e5fba
--- /dev/null
+++ b/examples/bzlmod/description.md
@@ -0,0 +1,10 @@
+Before this PR the `coverage_tool` automatically registered by `rules_python`
+was visible outside the toolchain repository. This fixes it to be consistent
+with `non-bzlmod` setups and ensures that the default `coverage_tool` is not
+visible outside the toolchain repos.
+
+This means that the `MODULE.bazel` file can be cleaned-up at the expense of
+relaxing the `coverage_tool` attribute for the `python_repository` to be a
+simple string as the label would be evaluated within the context of
+`rules_python` which may not necessarily resolve correctly without the
+`use_repo` statement in our `MODULE.bazel`.
diff --git a/examples/bzlmod/test.py b/examples/bzlmod/test.py
index 0486916..80cd027 100644
--- a/examples/bzlmod/test.py
+++ b/examples/bzlmod/test.py
@@ -66,7 +66,7 @@
if os.environ.get("COVERAGE_MANIFEST"):
# we are running under the 'bazel coverage :test'
self.assertTrue(
- "pypi__coverage_cp" in last_item,
+ "_coverage" in last_item,
f"Expected {last_item} to be related to coverage",
)
self.assertEqual(pathlib.Path(last_item).name, "coverage")
diff --git a/python/extensions/private/internal_deps.bzl b/python/extensions/private/internal_deps.bzl
index dfa3e26..27e290c 100644
--- a/python/extensions/private/internal_deps.bzl
+++ b/python/extensions/private/internal_deps.bzl
@@ -9,12 +9,10 @@
"Python toolchain module extension for internal rule use"
load("@rules_python//python/pip_install:repositories.bzl", "pip_install_dependencies")
-load("@rules_python//python/private:coverage_deps.bzl", "install_coverage_deps")
# buildifier: disable=unused-variable
def _internal_deps_impl(module_ctx):
pip_install_dependencies()
- install_coverage_deps()
internal_deps = module_extension(
doc = "This extension to register internal rules_python dependecies.",
diff --git a/python/private/coverage_deps.bzl b/python/private/coverage_deps.bzl
index a4801ca..8d1e5f4 100644
--- a/python/private/coverage_deps.bzl
+++ b/python/private/coverage_deps.bzl
@@ -17,11 +17,6 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
-load(
- "//python:versions.bzl",
- "MINOR_MAPPING",
- "PLATFORMS",
-)
# Update with './tools/update_coverage_deps.py <version>'
#START: managed by update_coverage_deps.py script
@@ -103,7 +98,7 @@
_coverage_patch = Label("//python/private:coverage.patch")
-def coverage_dep(name, python_version, platform, visibility, install = True):
+def coverage_dep(name, python_version, platform, visibility):
"""Register a singe coverage dependency based on the python version and platform.
Args:
@@ -111,8 +106,6 @@
python_version: The full python version.
platform: The platform, which can be found in //python:versions.bzl PLATFORMS dict.
visibility: The visibility of the coverage tool.
- install: should we install the dependency with a given name or generate the label
- of the bzlmod dependency fallback, which is hard-coded in MODULE.bazel?
Returns:
The label of the coverage tool if the platform is supported, otherwise - None.
@@ -131,17 +124,6 @@
# Some wheels are not present for some builds, so let's silently ignore those.
return None
- if not install:
- # FIXME @aignas 2023-01-19: right now we use globally installed coverage
- # which has visibility set to public, but is hidden due to repo remapping.
- #
- # The name of the toolchain is not known when registering the coverage tooling,
- # so we use this as a workaround for now.
- return Label("@pypi__coverage_{abi}_{platform}//:coverage".format(
- abi = abi,
- platform = platform,
- ))
-
maybe(
http_archive,
name = name,
@@ -162,26 +144,4 @@
urls = [url],
)
- return Label("@@{name}//:coverage".format(name = name))
-
-def install_coverage_deps():
- """Register the dependency for the coverage dep.
-
- This is only used under bzlmod.
- """
-
- for python_version in MINOR_MAPPING.values():
- for platform in PLATFORMS.keys():
- if "windows" in platform:
- continue
-
- coverage_dep(
- name = "pypi__coverage_cp{version_no_dot}_{platform}".format(
- version_no_dot = python_version.rpartition(".")[0].replace(".", ""),
- platform = platform,
- ),
- python_version = python_version,
- platform = platform,
- visibility = ["//visibility:public"],
- install = True,
- )
+ return "@{name}//:coverage".format(name = name)
diff --git a/python/repositories.bzl b/python/repositories.bzl
index 2352e22..39182af 100644
--- a/python/repositories.bzl
+++ b/python/repositories.bzl
@@ -374,10 +374,9 @@
_python_repository_impl,
doc = "Fetches the external tools needed for the Python toolchain.",
attrs = {
- "coverage_tool": attr.label(
+ "coverage_tool": attr.string(
# Mirrors the definition at
# https://github.com/bazelbuild/bazel/blob/master/src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl
- allow_files = False,
doc = """
This is a target to use for collecting code coverage information from `py_binary`
and `py_test` targets.
@@ -392,6 +391,9 @@
of coverage.py (https://coverage.readthedocs.io), at least including
the `run` and `lcov` subcommands.
+The target is accepted as a string by the python_repository and evaluated within
+the context of the toolchain repository.
+
For more information see the official bazel docs
(https://bazel.build/reference/be/python#py_runtime.coverage_tool).
""",
@@ -535,11 +537,10 @@
),
python_version = python_version,
platform = platform,
- visibility = ["@@{name}_{platform}//:__subpackages__".format(
+ visibility = ["@{name}_{platform}//:__subpackages__".format(
name = name,
platform = platform,
)],
- install = not bzlmod,
)
python_repository(
diff --git a/tools/update_coverage_deps.py b/tools/update_coverage_deps.py
index 4cf1e94..57b7850 100755
--- a/tools/update_coverage_deps.py
+++ b/tools/update_coverage_deps.py
@@ -241,16 +241,6 @@
dry_run=args.dry_run,
)
- # Update the MODULE.bazel, which needs to expose the dependencies to the toolchain
- # repositories
- _update_file(
- path=rules_python / "MODULE.bazel",
- snippet="".join(sorted([f' "{u.repo_name}",\n' for u in urls])),
- start_marker=" # coverage_deps managed by running",
- end_marker=")",
- dry_run=args.dry_run,
- )
-
return
