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