fix(bzlmod): allow users to specify extra targets to be added to hub …

…repos (#2369) Before this change, it was impossible for users to use the targets created with `additive_build_content` whl annotation unless they relied on the implementation detail of the naming of the spoke repositories and had `use_repo` statements in their `MODULE.bazel` files importing the spoke repos. With #2325 in the works, users will have to change their `use_repo` statements, which is going to be disruptive. In order to offer them an alternative for not relying on the names of the spokes, there has to be a way to expose the extra targets created and this PR implements a method. Incidentally, the same would have happened if we wanted to stabilize the #260 work and mark `experimental_index_url` as non-experimental anymore. I was hoping we could autodetect them by parsing the build content ourselves in the `pip` extension, but it turned out to be extremely tricky and I figured that it was better to have an API rather than not have it. Whilst at it, also relax the naming requirements for the `whl_modifications` attribute. Fixes #2187
bazelbuild · Nov 5, 2024 · 218f8e1 · 218f8e1
1 parent ae0aeff
commit 218f8e1
Show file tree

Hide file tree

Showing 9 changed files with 196 additions and 34 deletions.
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -65,15 +65,6 @@ buildifier:
 .reusable_build_test_all: &reusable_build_test_all
   build_targets: ["..."]
   test_targets: ["..."]
-.lockfile_mode_error: &lockfile_mode_error
-  # For testing lockfile support
-  skip_in_bazel_downstream_pipeline: "Lockfile depends on the bazel version"
-  build_flags:
-    - "--lockfile_mode=error"
-  test_flags:
-    - "--lockfile_mode=error"
-  coverage_flags:
-    - "--lockfile_mode=error"
 .coverage_targets_example_bzlmod: &coverage_targets_example_bzlmod
   coverage_targets: ["..."]
 .coverage_targets_example_bzlmod_build_file_generation: &coverage_targets_example_bzlmod_build_file_generation
@@ -268,17 +259,23 @@ tasks:
   integration_test_bzlmod_ubuntu_lockfile:
     <<: *reusable_build_test_all
     <<: *coverage_targets_example_bzlmod
-    <<: *lockfile_mode_error
     name: "examples/bzlmod: Ubuntu with lockfile"
     working_directory: examples/bzlmod
     platform: ubuntu2004
+    shell_commands:
+    # Update the lockfiles and fail if it is different.
+    - "../../tools/private/update_bzlmod_lockfiles.sh"
+    - "git diff --exit-code"
   integration_test_bzlmod_macos_lockfile:
     <<: *reusable_build_test_all
     <<: *coverage_targets_example_bzlmod
-    <<: *lockfile_mode_error
     name: "examples/bzlmod: macOS with lockfile"
     working_directory: examples/bzlmod
     platform: macos
+    shell_commands:
+    # Update the lockfiles and fail if it is different.
+    - "../../tools/private/update_bzlmod_lockfiles.sh"
+    - "git diff --exit-code"
 
   integration_test_bzlmod_generate_build_file_generation_ubuntu_min:
     <<: *minimum_supported_version

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,8 @@ A brief description of the categories of changes:
   by default. Users wishing to keep this argument and to enforce more hermetic
   builds can do so by passing the argument in
   [`pip.parse#extra_pip_args`](https://rules-python.readthedocs.io/en/latest/api/rules_python/python/extensions/pip.html#pip.parse.extra_pip_args)
+* (pip.parse) {attr}`pip.parse.whl_modifications` now normalizes the given whl names
+  and now `pyyaml` and `PyYAML` will both work.
 
 {#v0-0-0-fixed}
 ### Fixed
@@ -58,6 +60,9 @@ A brief description of the categories of changes:
   and one extra file `requirements_universal.txt` if you prefer a single file.
   The `requirements.txt` file may be removed in the future.
 * The rules_python version is now reported in `//python/features.bzl#features.version`
+* (pip.parse) {attr}`pip.parse.extra_hub_aliases` can now be used to expose extra
+  targets created by annotations in whl repositories.
+  Fixes [#2187](https://github.com/bazelbuild/rules_python/issues/2187).
 
 {#v0-0-0-removed}
 ### Removed

diff --git a/examples/bzlmod/BUILD.bazel b/examples/bzlmod/BUILD.bazel
@@ -69,16 +69,24 @@ py_test_with_transition(
 # to run some of the tests.
 # See: https://github.com/bazelbuild/bazel-skylib/blob/main/docs/build_test_doc.md
 build_test(
-    name = "all_wheels",
+    name = "all_wheels_build_test",
     targets = all_whl_requirements,
 )
 
 build_test(
-    name = "all_data_requirements",
+    name = "all_data_requirements_build_test",
     targets = all_data_requirements,
 )
 
 build_test(
-    name = "all_requirements",
+    name = "all_requirements_build_test",
     targets = all_requirements,
 )
+
+# Check the annotations API
+build_test(
+    name = "extra_annotation_targets_build_test",
+    targets = [
+        "@pip//wheel:generated_file",
+    ],
+)
diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel
@@ -183,6 +183,9 @@ pip.parse(
         "cp39_linux_*",
         "cp39_*",
     ],
+    extra_hub_aliases = {
+        "wheel": ["generated_file"],
+    },
     hub_name = "pip",
     python_version = "3.9",
     requirements_lock = "requirements_lock_3_9.txt",

diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
@@ -104,6 +104,10 @@ def _create_whl_repos(
     # containers to aggregate outputs from this function
     whl_map = {}
     exposed_packages = {}
+    extra_aliases = {
+        whl_name: {alias: True for alias in aliases}
+        for whl_name, aliases in pip_attr.extra_hub_aliases.items()
+    }
     whl_libraries = {}
 
     # if we do not have the python_interpreter set in the attributes
@@ -136,7 +140,7 @@ def _create_whl_repos(
     whl_modifications = {}
     if pip_attr.whl_modifications != None:
         for mod, whl_name in pip_attr.whl_modifications.items():
-            whl_modifications[whl_name] = mod
+            whl_modifications[normalize_name(whl_name)] = mod
 
     if pip_attr.experimental_requirement_cycles:
         requirement_cycles = {
@@ -214,10 +218,6 @@ def _create_whl_repos(
 
     repository_platform = host_platform(module_ctx)
     for whl_name, requirements in requirements_by_platform.items():
-        # We are not using the "sanitized name" because the user
-        # would need to guess what name we modified the whl name
-        # to.
-        annotation = whl_modifications.get(whl_name)
         whl_name = normalize_name(whl_name)
 
         group_name = whl_group_mapping.get(whl_name)
@@ -231,7 +231,7 @@ def _create_whl_repos(
         )
         maybe_args = dict(
             # The following values are safe to omit if they have false like values
-            annotation = annotation,
+            annotation = whl_modifications.get(whl_name),
             download_only = pip_attr.download_only,
             enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
             environment = pip_attr.environment,
@@ -353,6 +353,7 @@ def _create_whl_repos(
         is_reproducible = is_reproducible,
         whl_map = whl_map,
         exposed_packages = exposed_packages,
+        extra_aliases = extra_aliases,
         whl_libraries = whl_libraries,
     )
 
@@ -437,6 +438,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
     hub_whl_map = {}
     hub_group_map = {}
     exposed_packages = {}
+    extra_aliases = {}
     whl_libraries = {}
 
     is_reproducible = True
@@ -486,6 +488,9 @@ You cannot use both the additive_build_content and additive_build_content_file a
             hub_whl_map.setdefault(hub_name, {})
             for key, settings in out.whl_map.items():
                 hub_whl_map[hub_name].setdefault(key, []).extend(settings)
+            extra_aliases.setdefault(hub_name, {})
+            for whl_name, aliases in out.extra_aliases.items():
+                extra_aliases[hub_name].setdefault(whl_name, {}).update(aliases)
             exposed_packages.setdefault(hub_name, {}).update(out.exposed_packages)
             whl_libraries.update(out.whl_libraries)
             is_reproducible = is_reproducible and out.is_reproducible
@@ -514,6 +519,13 @@ You cannot use both the additive_build_content and additive_build_content_file a
             for hub_name, group_map in sorted(hub_group_map.items())
         },
         exposed_packages = {k: sorted(v) for k, v in sorted(exposed_packages.items())},
+        extra_aliases = {
+            hub_name: {
+                whl_name: sorted(aliases)
+                for whl_name, aliases in extra_whl_aliases.items()
+            }
+            for hub_name, extra_whl_aliases in extra_aliases.items()
+        },
         whl_libraries = dict(sorted(whl_libraries.items())),
         is_reproducible = is_reproducible,
     )
@@ -598,6 +610,7 @@ def _pip_impl(module_ctx):
         hub_repository(
             name = hub_name,
             repo_name = hub_name,
+            extra_hub_aliases = mods.extra_aliases.get(hub_name, {}),
             whl_map = {
                 key: json.encode(value)
                 for key, value in whl_map.items()
@@ -684,6 +697,16 @@ The indexes must support Simple API as described here:
 https://packaging.python.org/en/latest/specifications/simple-repository-api/
 """,
         ),
+        "extra_hub_aliases": attr.string_list_dict(
+            doc = """\
+Extra aliases to make for specific wheels in the hub repo. This is useful when
+paired with the {attr}`whl_modifications`.
+
+:::{versionadded} 0.38.0
+:::
+""",
+            mandatory = False,
+        ),
         "hub_name": attr.string(
             mandatory = True,
             doc = """

diff --git a/python/private/pypi/hub_repository.bzl b/python/private/pypi/hub_repository.bzl
@@ -35,6 +35,7 @@ def _impl(rctx):
             key: [whl_alias(**v) for v in json.decode(values)]
             for key, values in rctx.attr.whl_map.items()
         },
+        extra_hub_aliases = rctx.attr.extra_hub_aliases,
         requirement_cycles = rctx.attr.groups,
     )
     for path, contents in aliases.items():
@@ -65,6 +66,10 @@ def _impl(rctx):
 
 hub_repository = repository_rule(
     attrs = {
+        "extra_hub_aliases": attr.string_list_dict(
+            doc = "Extra aliases to make for specific wheels in the hub repo.",
+            mandatory = True,
+        ),
         "groups": attr.string_list_dict(
             mandatory = False,
         ),

diff --git a/python/private/pypi/render_pkg_aliases.bzl b/python/private/pypi/render_pkg_aliases.bzl
@@ -117,7 +117,7 @@ def _render_whl_library_alias(
         **kwargs
     )
 
-def _render_common_aliases(*, name, aliases, group_name = None):
+def _render_common_aliases(*, name, aliases, extra_aliases = [], group_name = None):
     lines = [
         """load("@bazel_skylib//lib:selects.bzl", "selects")""",
         """package(default_visibility = ["//visibility:public"])""",
@@ -153,12 +153,17 @@ def _render_common_aliases(*, name, aliases, group_name = None):
                 target_name = target_name,
                 visibility = ["//_groups:__subpackages__"] if name.startswith("_") else None,
             )
-            for target_name, name in {
-                PY_LIBRARY_PUBLIC_LABEL: PY_LIBRARY_IMPL_LABEL if group_name else PY_LIBRARY_PUBLIC_LABEL,
-                WHEEL_FILE_PUBLIC_LABEL: WHEEL_FILE_IMPL_LABEL if group_name else WHEEL_FILE_PUBLIC_LABEL,
-                DATA_LABEL: DATA_LABEL,
-                DIST_INFO_LABEL: DIST_INFO_LABEL,
-            }.items()
+            for target_name, name in (
+                {
+                    PY_LIBRARY_PUBLIC_LABEL: PY_LIBRARY_IMPL_LABEL if group_name else PY_LIBRARY_PUBLIC_LABEL,
+                    WHEEL_FILE_PUBLIC_LABEL: WHEEL_FILE_IMPL_LABEL if group_name else WHEEL_FILE_PUBLIC_LABEL,
+                    DATA_LABEL: DATA_LABEL,
+                    DIST_INFO_LABEL: DIST_INFO_LABEL,
+                } | {
+                    x: x
+                    for x in extra_aliases
+                }
+            ).items()
         ],
     )
     if group_name:
@@ -177,7 +182,7 @@ def _render_common_aliases(*, name, aliases, group_name = None):
 
     return "\n\n".join(lines)
 
-def render_pkg_aliases(*, aliases, requirement_cycles = None):
+def render_pkg_aliases(*, aliases, requirement_cycles = None, extra_hub_aliases = {}):
     """Create alias declarations for each PyPI package.
 
     The aliases should be appended to the pip_repository BUILD.bazel file. These aliases
@@ -188,6 +193,8 @@ def render_pkg_aliases(*, aliases, requirement_cycles = None):
         aliases: dict, the keys are normalized distribution names and values are the
             whl_alias instances.
         requirement_cycles: any package groups to also add.
+        extra_hub_aliases: The list of extra aliases for each whl to be added
+          in addition to the default ones.
 
     Returns:
         A dict of file paths and their contents.
@@ -215,6 +222,7 @@ def render_pkg_aliases(*, aliases, requirement_cycles = None):
         "{}/BUILD.bazel".format(normalize_name(name)): _render_common_aliases(
             name = normalize_name(name),
             aliases = pkg_aliases,
+            extra_aliases = extra_hub_aliases.get(name, []),
             group_name = whl_group_mapping.get(normalize_name(name)),
         ).strip()
         for name, pkg_aliases in aliases.items()