fix(toolchain): restrict coverage tool visibility under bzlmod (bazel…

…build#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`.

MODULE.bazel

@@ -29,23 +29,6 @@ use_repo(
  "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"

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`.

examples/bzlmod/test.py

@@ -66,7 +66,7 @@ def test_coverage_sys_path(self):
  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")

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.",

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,16 +98,14 @@ _coverage_deps = {
 
 _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:
  name: The name of the registered repository.
  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 @@ def coverage_dep(name, python_version, platform, visibility, install = True):
  # 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 @@ filegroup(
  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)

python/repositories.bzl

@@ -374,10 +374,9 @@ python_repository = repository_rule(
  _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 @@ The entry point for the tool must be loadable by a Python interpreter (e.g. a
 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 @@ def python_register_toolchains(
  ),
  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(

tools/update_coverage_deps.py

@@ -241,16 +241,6 @@ def main():
  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