Merge branch 'main' into fix/lockfile-inconsistency

Author: aignas

.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

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.
 * (bzlmod) `pip.parse` spoke repository naming will be changed in an upcoming
   release in places where the users specify different package versions per
   platform in the same hub repository. The naming of the spoke repos is considered
@@ -53,6 +55,8 @@ A brief description of the categories of changes:
   ([617](https://github.com/bazelbuild/rules_python/issues/617)).
 * (pypi) When {attr}`pip.parse.experimental_index_url` is set, we need to still
   pass the `extra_pip_args` value when building an `sdist`.
+* (pypi) The patched wheel filenames from now on are using local version specifiers
+  which fixes usage of the said wheels using standard package managers.
 * (bzlmod) The extension evaluation has been adjusted to always generate the
   same lock file irrespective if `experimental_index_url` is set by any module
   or not. Fixes
@@ -68,6 +72,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).
 * (bzlmod) `pip.parse` now supports `whl-only` setup using
   `download_only = True` where users can specify multiple requirements files
   and use the `pip` backend to do the downloading. This was only available for

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",
+    ],
+)

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

examples/bzlmod/MODULE.bazel.lock

@@ -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,
@@ -389,6 +389,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,
     )
 
@@ -473,6 +474,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
@@ -522,6 +524,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
@@ -554,6 +559,13 @@ You cannot use both the additive_build_content and additive_build_content_file a
             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 = {
             k: dict(sorted(args.items()))
             for k, args in sorted(whl_libraries.items())
@@ -639,6 +651,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()
@@ -725,6 +738,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 = """

python/private/pypi/extension.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,
         ),

python/private/pypi/hub_repository.bzl

@@ -32,6 +32,39 @@ load(":parse_whl_name.bzl", "parse_whl_name")
 
 _rules_python_root = Label("//:BUILD.bazel")
 
+def patched_whl_name(original_whl_name):
+    """Return the new filename to output the patched wheel.
+
+    Args:
+        original_whl_name: {type}`str` the whl name of the original file.
+
+    Returns:
+        {type}`str` an output name to write the patched wheel to.
+    """
+    parsed_whl = parse_whl_name(original_whl_name)
+    version = parsed_whl.version
+    suffix = "patched"
+    if "+" in version:
+        # This already has some local version, so we just append one more
+        # identifier here. We comply with the spec and mark the file as patched
+        # by adding a local version identifier at the end.
+        #
+        # By doing this we can still install the package using most of the package
+        # managers
+        #
+        # See https://packaging.python.org/en/latest/specifications/version-specifiers/#local-version-identifiers
+        version = "{}.{}".format(version, suffix)
+    else:
+        version = "{}+{}".format(version, suffix)
+
+    return "{distribution}-{version}-{python_tag}-{abi_tag}-{platform_tag}.whl".format(
+        distribution = parsed_whl.distribution,
+        version = version,
+        python_tag = parsed_whl.python_tag,
+        abi_tag = parsed_whl.abi_tag,
+        platform_tag = parsed_whl.platform_tag,
+    )
+
 def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
     """Patch a whl file and repack it to ensure that the RECORD metadata stays correct.
 
@@ -66,18 +99,8 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
     for patch_file, patch_strip in patches.items():
         rctx.patch(patch_file, strip = patch_strip)
 
-    # Generate an output filename, which we will be returning
-    parsed_whl = parse_whl_name(whl_input.basename)
-    whl_patched = "{}.whl".format("-".join([
-        parsed_whl.distribution,
-        parsed_whl.version,
-        (parsed_whl.build_tag or "") + "patched",
-        parsed_whl.python_tag,
-        parsed_whl.abi_tag,
-        parsed_whl.platform_tag,
-    ]))
-
     record_patch = rctx.path("RECORD.patch")
+    whl_patched = patched_whl_name(whl_input.basename)
 
     repo_utils.execute_checked(
         rctx,

python/private/pypi/patch_whl.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()