From 0a2cf0586230956751b35df0f97f0746c4de23b1 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 16 Apr 2025 18:36:47 +0900 Subject: [PATCH 1/9] refactor: load target_platforms through the hub This PR moves the parsing of `Requires-Dist` to the analysis phase within the `whl_library_targets_from_requires` macro. The original `whl_library_targets` macro has been left unchanged so that I don't have to reinvent the unit tests - it is well covered under tests. Before this PR we had to wire the `target_platforms` via the `experimental_target_platforms` attr in the `whl_library`, which means that whenever this would change (e.g. the minor Python version changes), the wheel would be re-extracted even though the final result may be the same. This also cleans up the code by removing left over TODO notes or code that no longer make sense. Work towards #260, #2319 --- config.bzl.tmpl.bzlmod | 0 python/private/pypi/BUILD.bazel | 14 +-- python/private/pypi/config.bzl.tmpl.bzlmod | 9 ++ python/private/pypi/extension.bzl | 41 ++++---- .../pypi/generate_whl_library_build_bazel.bzl | 23 ++++- python/private/pypi/hub_repository.bzl | 18 +++- python/private/pypi/pep508.bzl | 23 ----- python/private/pypi/pep508_deps.bzl | 59 ++++++----- python/private/pypi/whl_library.bzl | 97 ++++++------------- python/private/pypi/whl_library_targets.bzl | 79 ++++++++++++++- tests/pypi/extension/extension_tests.bzl | 10 -- ...generate_whl_library_build_bazel_tests.bzl | 92 +++++++++++++----- tests/pypi/pep508/deps_tests.bzl | 31 +++--- .../whl_library_targets_tests.bzl | 67 ++++++++++++- 14 files changed, 358 insertions(+), 205 deletions(-) create mode 100644 config.bzl.tmpl.bzlmod create mode 100644 python/private/pypi/config.bzl.tmpl.bzlmod delete mode 100644 python/private/pypi/pep508.bzl diff --git a/config.bzl.tmpl.bzlmod b/config.bzl.tmpl.bzlmod new file mode 100644 index 0000000000..e69de29bb2 diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index 7297238cb4..a758b3f153 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -212,15 +212,6 @@ bzl_library( ], ) -bzl_library( - name = "pep508_bzl", - srcs = ["pep508.bzl"], - deps = [ - ":pep508_env_bzl", - ":pep508_evaluate_bzl", - ], -) - bzl_library( name = "pep508_deps_bzl", srcs = ["pep508_deps.bzl"], @@ -378,13 +369,12 @@ bzl_library( ":attrs_bzl", ":deps_bzl", ":generate_whl_library_build_bazel_bzl", - ":parse_whl_name_bzl", ":patch_whl_bzl", - ":pep508_deps_bzl", + ":pep508_requirement_bzl", ":pypi_repo_utils_bzl", ":whl_metadata_bzl", - ":whl_target_platforms_bzl", "//python/private:auth_bzl", + "//python/private:bzlmod_enabled_bzl", "//python/private:envsubst_bzl", "//python/private:is_standalone_interpreter_bzl", "//python/private:repo_utils_bzl", diff --git a/python/private/pypi/config.bzl.tmpl.bzlmod b/python/private/pypi/config.bzl.tmpl.bzlmod new file mode 100644 index 0000000000..deb53631d1 --- /dev/null +++ b/python/private/pypi/config.bzl.tmpl.bzlmod @@ -0,0 +1,9 @@ +"""Extra configuration values that are exposed from the hub repository for spoke repositories to access. + +NOTE: This is internal `rules_python` API and if you would like to depend on it, please raise an issue +with your usecase. This may change in between rules_python versions without any notice. + +@generated by rules_python pip.parse bzlmod extension. +""" + +target_platforms = %%TARGET_PLATFORMS%% diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index d2ae132741..e053ce4932 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -32,7 +32,6 @@ load(":simpleapi_download.bzl", "simpleapi_download") load(":whl_config_setting.bzl", "whl_config_setting") load(":whl_library.bzl", "whl_library") load(":whl_repo_name.bzl", "pypi_repo_name", "whl_repo_name") -load(":whl_target_platforms.bzl", "whl_target_platforms") def _major_minor_version(version): version = semver(version) @@ -68,7 +67,6 @@ def _create_whl_repos( *, pip_attr, whl_overrides, - evaluate_markers = evaluate_markers, available_interpreters = INTERPRETER_LABELS, get_index_urls = None): """create all of the whl repositories @@ -77,7 +75,6 @@ def _create_whl_repos( module_ctx: {type}`module_ctx`. pip_attr: {type}`struct` - the struct that comes from the tag class iteration. whl_overrides: {type}`dict[str, struct]` - per-wheel overrides. - evaluate_markers: the function to use to evaluate markers. get_index_urls: A function used to get the index URLs available_interpreters: {type}`dict[str, Label]` The dictionary of available interpreters that have been registered using the `python` bzlmod extension. @@ -162,14 +159,12 @@ def _create_whl_repos( requirements_osx = pip_attr.requirements_darwin, requirements_windows = pip_attr.requirements_windows, extra_pip_args = pip_attr.extra_pip_args, + # TODO @aignas 2025-04-15: pass the full version into here python_version = major_minor, logger = logger, ), extra_pip_args = pip_attr.extra_pip_args, get_index_urls = get_index_urls, - # NOTE @aignas 2025-02-24: we will use the "cp3xx_os_arch" platform labels - # for converting to the PEP508 environment and will evaluate them in starlark - # without involving the interpreter at all. evaluate_markers = evaluate_markers, logger = logger, ) @@ -191,7 +186,6 @@ def _create_whl_repos( enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs, environment = pip_attr.environment, envsubst = pip_attr.envsubst, - experimental_target_platforms = pip_attr.experimental_target_platforms, group_deps = group_deps, group_name = group_name, pip_data_exclude = pip_attr.pip_data_exclude, @@ -244,6 +238,12 @@ def _create_whl_repos( }, extra_aliases = extra_aliases, whl_libraries = whl_libraries, + target_platforms = { + plat: None + for reqs in requirements_by_platform.values() + for req in reqs + for plat in req.target_platforms + }, ) def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version): @@ -274,20 +274,11 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt args["urls"] = [distribution.url] args["sha256"] = distribution.sha256 args["filename"] = distribution.filename - args["experimental_target_platforms"] = requirement.target_platforms # Pure python wheels or sdists may need to have a platform here target_platforms = None if distribution.filename.endswith(".whl") and not distribution.filename.endswith("-any.whl"): - parsed_whl = parse_whl_name(distribution.filename) - whl_platforms = whl_target_platforms( - platform_tag = parsed_whl.platform_tag, - ) - args["experimental_target_platforms"] = [ - p - for p in requirement.target_platforms - if [None for wp in whl_platforms if p.endswith(wp.target_platform)] - ] + pass elif multiple_requirements_for_whl: target_platforms = requirement.target_platforms @@ -416,6 +407,7 @@ You cannot use both the additive_build_content and additive_build_content_file a hub_group_map = {} exposed_packages = {} extra_aliases = {} + target_platforms = {} whl_libraries = {} for mod in module_ctx.modules: @@ -498,6 +490,7 @@ You cannot use both the additive_build_content and additive_build_content_file a 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) + target_platforms.setdefault(hub_name, {}).update(out.target_platforms) whl_libraries.update(out.whl_libraries) # TODO @aignas 2024-04-05: how do we support different requirement @@ -535,6 +528,10 @@ You cannot use both the additive_build_content and additive_build_content_file a } for hub_name, extra_whl_aliases in extra_aliases.items() }, + target_platforms = { + hub_name: sorted(p) + for hub_name, p in target_platforms.items() + }, whl_libraries = { k: dict(sorted(args.items())) for k, args in sorted(whl_libraries.items()) @@ -626,15 +623,13 @@ def _pip_impl(module_ctx): }, packages = mods.exposed_packages.get(hub_name, []), groups = mods.hub_group_map.get(hub_name), + target_platforms = mods.target_platforms.get(hub_name, []), ) if bazel_features.external_deps.extension_metadata_has_reproducible: - # If we are not using the `experimental_index_url feature, the extension is fully - # deterministic and we don't need to create a lock entry for it. - # - # In order to be able to dogfood the `experimental_index_url` feature before it gets - # stabilized, we have created the `_pip_non_reproducible` function, that will result - # in extra entries in the lock file. + # NOTE @aignas 2025-04-15: this is set to be reproducible, because the + # results after calling the PyPI index should be reproducible on each + # machine. return module_ctx.extension_metadata(reproducible = True) else: return None diff --git a/python/private/pypi/generate_whl_library_build_bazel.bzl b/python/private/pypi/generate_whl_library_build_bazel.bzl index 8050cd22ad..801db56304 100644 --- a/python/private/pypi/generate_whl_library_build_bazel.bzl +++ b/python/private/pypi/generate_whl_library_build_bazel.bzl @@ -21,23 +21,23 @@ _RENDER = { "copy_files": render.dict, "data": render.list, "data_exclude": render.list, - "dependencies": render.list, - "dependencies_by_platform": lambda x: render.dict(x, value_repr = render.list), "entry_points": render.dict, + "extras": render.list, "group_deps": render.list, + "requires_dist": render.list, "srcs_exclude": render.list, - "tags": render.list, + "target_platforms": lambda x: render.list(x) if x else "target_platforms", } # NOTE @aignas 2024-10-25: We have to keep this so that files in # this repository can be publicly visible without the need for # export_files _TEMPLATE = """\ -load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets") +{loads} package(default_visibility = ["//visibility:public"]) -whl_library_targets( +whl_library_targets_from_requires( {kwargs} ) """ @@ -57,6 +57,18 @@ def generate_whl_library_build_bazel( A complete BUILD file as a string """ + loads = [ + """load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""", + ] + if not kwargs.setdefault("target_platforms", None): + dep_template = kwargs["dep_template"] + loads.append( + "load(\"{}\", \"{}\")".format( + dep_template.format(name = "", target = "config.bzl"), + "target_platforms", + ), + ) + additional_content = [] if annotation: kwargs["data"] = annotation.data @@ -70,6 +82,7 @@ def generate_whl_library_build_bazel( contents = "\n".join( [ _TEMPLATE.format( + loads = "\n".join(loads), kwargs = render.indent("\n".join([ "{} = {},".format(k, _RENDER.get(k, repr)(v)) for k, v in sorted(kwargs.items()) diff --git a/python/private/pypi/hub_repository.bzl b/python/private/pypi/hub_repository.bzl index 48245b4106..d2cbf88c24 100644 --- a/python/private/pypi/hub_repository.bzl +++ b/python/private/pypi/hub_repository.bzl @@ -45,7 +45,14 @@ def _impl(rctx): macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name) rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS) - rctx.template("requirements.bzl", rctx.attr._template, substitutions = { + rctx.template( + "config.bzl", + rctx.attr._config_template, + substitutions = { + "%%TARGET_PLATFORMS%%": render.list(rctx.attr.target_platforms), + }, + ) + rctx.template("requirements.bzl", rctx.attr._requirements_bzl_template, substitutions = { "%%ALL_DATA_REQUIREMENTS%%": render.list([ macro_tmpl.format(p, "data") for p in bzl_packages @@ -80,6 +87,10 @@ The list of packages that will be exposed via all_*requirements macros. Defaults mandatory = True, doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.", ), + "target_platforms": attr.string_list( + mandatory = True, + doc = "All of the target platforms for the hub repo", + ), "whl_map": attr.string_dict( mandatory = True, doc = """\ @@ -87,7 +98,10 @@ The wheel map where values are json.encoded strings of the whl_map constructed in the pip.parse tag class. """, ), - "_template": attr.label( + "_config_template": attr.label( + default = ":config.bzl.tmpl.bzlmod", + ), + "_requirements_bzl_template": attr.label( default = ":requirements.bzl.tmpl.bzlmod", ), }, diff --git a/python/private/pypi/pep508.bzl b/python/private/pypi/pep508.bzl deleted file mode 100644 index e74352def2..0000000000 --- a/python/private/pypi/pep508.bzl +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright 2025 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""This module is for implementing PEP508 in starlark as FeatureFlagInfo -""" - -load(":pep508_env.bzl", _env = "env") -load(":pep508_evaluate.bzl", _evaluate = "evaluate", _to_string = "to_string") - -to_string = _to_string -evaluate = _evaluate -env = _env diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index af0a75362b..fe8ca5ff4f 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -15,6 +15,7 @@ """This module is for implementing PEP508 compliant METADATA deps parsing. """ +load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION") load("//python/private:normalize_name.bzl", "normalize_name") load(":pep508_env.bzl", "env") load(":pep508_evaluate.bzl", "evaluate") @@ -35,7 +36,7 @@ _ALL_ARCH_VALUES = [ "x86_64", ] -def deps(name, *, requires_dist, platforms = [], extras = [], host_python_version = None): +def deps(name, *, requires_dist, platforms = [], extras = [], default_python_version = None): """Parse the RequiresDist from wheel METADATA Args: @@ -44,7 +45,7 @@ def deps(name, *, requires_dist, platforms = [], extras = [], host_python_versio METADATA file. extras: {type}`list[str]` the requested extras to generate targets for. platforms: {type}`list[str]` the list of target platform strings. - host_python_version: {type}`str` the host python version. + default_python_version: {type}`str` the host python version. Returns: A struct with attributes: @@ -52,38 +53,51 @@ def deps(name, *, requires_dist, platforms = [], extras = [], host_python_versio * deps_select: {type}`dict[str, list[str]]` dependencies to include on particular subset of target platforms. """ - reqs = sorted( - [requirement(r) for r in requires_dist], - key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker), - ) - deps = {} - deps_select = {} - name = normalize_name(name) - want_extras = _resolve_extras(name, reqs, extras) - - # drop self edges - reqs = [r for r in reqs if r.name != name] - + if not platforms: + fail("'platforms' arg is mandatory") + + # TODO @aignas 2025-04-16: this `default_abi` variable is only + # here so that we can have selects without "is_python_version" conditions. + # This would happen in the `multi_pip_parse` logic in WORKSPACE. + # + # In followup PRs I would like to remove this logic and mainly rely on the + # fact that there will be only a single ABI when internal rules pass the + # values in the WORKSPACE case and in that case we can get the default + # version from the target platform strings. What is more when we drop + # WORKSPACE, we can simplify the logic in `_add_req` substantially and get + # rid of `default_abi` all together. + default_python_version = default_python_version or DEFAULT_PYTHON_VERSION platforms = [ - platform_from_str(p, python_version = host_python_version) + platform_from_str(p, python_version = default_python_version) for p in platforms - ] or [ - platform_from_str("", python_version = host_python_version), ] abis = sorted({p.abi: True for p in platforms if p.abi}) - if host_python_version and len(abis) > 1: - _, _, minor_version = host_python_version.partition(".") + if default_python_version and len(abis) > 1: + _, _, minor_version = default_python_version.partition(".") minor_version, _, _ = minor_version.partition(".") default_abi = "cp3" + minor_version elif len(abis) > 1: fail( - "all python versions need to be specified explicitly, got: {}".format(platforms), + "all python versions need to be specified explicitly with the default_python_version, got: {}, {}".format(platforms, default_python_version), ) else: default_abi = None + reqs = sorted( + [requirement(r) for r in requires_dist], + key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker), + ) + deps = {} + deps_select = {} + name = normalize_name(name) + want_extras = _resolve_extras(name, reqs, extras) + for req in reqs: + if req.name == name: + # drop self edges + continue + _add_req( deps, deps_select, @@ -280,11 +294,6 @@ def _add_req(deps, deps_select, req, *, extras, platforms, default_abi = None): _add(deps, deps_select, req.name, None) return - # NOTE @aignas 2023-12-08: in order to have reasonable select statements - # we do have to have some parsing of the markers, so it begs the question - # if packaging should be reimplemented in Starlark to have the best solution - # for now we will implement it in Python and see what the best parsing result - # can be before making this decision. match_os = len([ tag for tag in [ diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 0a580011ab..ea22a25d2f 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -15,6 +15,7 @@ "" load("//python/private:auth.bzl", "AUTH_ATTRS", "get_auth") +load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") load("//python/private:envsubst.bzl", "envsubst") load("//python/private:is_standalone_interpreter.bzl", "is_standalone_interpreter") load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") @@ -22,13 +23,10 @@ load(":attrs.bzl", "ATTRS", "use_isolated") load(":deps.bzl", "all_repo_names", "record_files") load(":generate_whl_library_build_bazel.bzl", "generate_whl_library_build_bazel") load(":parse_requirements.bzl", "host_platform") -load(":parse_whl_name.bzl", "parse_whl_name") load(":patch_whl.bzl", "patch_whl") -load(":pep508_deps.bzl", "deps") load(":pep508_requirement.bzl", "requirement") load(":pypi_repo_utils.bzl", "pypi_repo_utils") load(":whl_metadata.bzl", "whl_metadata") -load(":whl_target_platforms.bzl", "whl_target_platforms") _CPPFLAGS = "CPPFLAGS" _COMMAND_LINE_TOOLS_PATH_SLUG = "commandlinetools" @@ -344,20 +342,6 @@ def _whl_library_impl(rctx): timeout = rctx.attr.timeout, ) - target_platforms = rctx.attr.experimental_target_platforms - if target_platforms: - parsed_whl = parse_whl_name(whl_path.basename) - if parsed_whl.platform_tag != "any": - # NOTE @aignas 2023-12-04: if the wheel is a platform specific - # wheel, we only include deps for that target platform - target_platforms = [ - p.target_platform - for p in whl_target_platforms( - platform_tag = parsed_whl.platform_tag, - abi_tag = parsed_whl.abi_tag.strip("tm"), - ) - ] - pypi_repo_utils.execute_checked( rctx, op = "whl_library.ExtractWheel({}, {})".format(rctx.attr.name, whl_path), @@ -400,63 +384,45 @@ def _whl_library_impl(rctx): ) entry_points[entry_point_without_py] = entry_point_script_name - # TODO @aignas 2025-04-04: move this to whl_library_targets.bzl to have - # this in the analysis phase. - # - # This means that whl_library_targets will have to accept the following args: - # * name - the name of the package in the METADATA. - # * requires_dist - the list of METADATA Requires-Dist. - # * platforms - the list of target platforms. The target_platforms - # should come from the hub repo via a 'load' statement so that they don't - # need to be passed as an argument to `whl_library`. - # * extras - the list of required extras. This comes from the - # `rctx.attr.requirement` for now. In the future the required extras could - # stay in the hub repo, where we calculate the extra aliases that we need - # to create automatically and this way expose the targets for the specific - # extras. The first step will be to generate a target per extra for the - # `py_library` and `filegroup`. Maybe we need to have a special provider - # or an output group so that we can return the `whl` file from the - # `py_library` target? filegroup can use output groups to expose files. - # * host_python_version/versons - the list of python versions to support - # should come from the hub, similar to how the target platforms are specified. - # - # Extra things that we should move at the same time: - # * group_name, group_deps - this info can stay in the hub repository so that - # it is piped at the analysis time and changing the requirement groups does - # cause to re-fetch the deps. - python_version = metadata["python_version"] + if BZLMOD_ENABLED: + # The following attributes are unset on bzlmod and we pass data through + # the hub via load statements. + python_version = None + target_platforms = [] + else: + # NOTE @aignas 2025-04-16: if BZLMOD_ENABLED, we should use + # DEFAULT_PYTHON_VERSION since platforms always come with the actual + # python version otherwise we should use the version of the interpreter + # here. In WORKSPACE `multi_pip_parse` is using an interpreter for each + # `pip_parse` invocation, so we will have the host target platform + # only. Even if somebody would change the code to support + # `experimental_target_platforms`, they would be for a single python + # version. Hence, using the `python_version` that we get from the + # interpreter is correct. Hence, we unset the argument if we are on bzlmod. + python_version = metadata["python_version"] + target_platforms = rctx.attr.experimental_target_platforms or [host_platform(rctx)] + metadata = whl_metadata( install_dir = rctx.path("site-packages"), read_fn = rctx.read, logger = logger, ) - # TODO @aignas 2025-04-09: this will later be removed when loaded through the hub - major_minor, _, _ = python_version.rpartition(".") - package_deps = deps( - name = metadata.name, - requires_dist = metadata.requires_dist, - platforms = target_platforms or [ - "cp{}_{}".format(major_minor.replace(".", ""), host_platform(rctx)), - ], - extras = requirement(rctx.attr.requirement).extras, - host_python_version = python_version, - ) - build_file_contents = generate_whl_library_build_bazel( name = whl_path.basename, + metadata_name = metadata.name, + metadata_version = metadata.version, + requires_dist = metadata.requires_dist, dep_template = rctx.attr.dep_template or "@{}{{name}}//:{{target}}".format(rctx.attr.repo_prefix), - dependencies = package_deps.deps, - dependencies_by_platform = package_deps.deps_select, - group_name = rctx.attr.group_name, - group_deps = rctx.attr.group_deps, - data_exclude = rctx.attr.pip_data_exclude, - tags = [ - "pypi_name=" + metadata.name, - "pypi_version=" + metadata.version, - ], entry_points = entry_points, + target_platforms = target_platforms, + python_version = python_version, + # TODO @aignas 2025-04-14: load through the hub: annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))), + data_exclude = rctx.attr.pip_data_exclude, + extras = requirement(rctx.attr.requirement).extras, + group_deps = rctx.attr.group_deps, + group_name = rctx.attr.group_name, ) rctx.file("BUILD.bazel", build_file_contents) @@ -517,10 +483,7 @@ and the target that we need respectively. doc = "Name of the group, if any.", ), "repo": attr.string( - doc = """\ -Pointer to parent repo name. Used to make these rules rerun if the parent repo changes. -Only used in WORKSPACE when the {attr}`dep_template` is not set. -""", + doc = "Pointer to parent repo name. Used to make these rules rerun if the parent repo changes.", ), "repo_prefix": attr.string( doc = """ diff --git a/python/private/pypi/whl_library_targets.bzl b/python/private/pypi/whl_library_targets.bzl index d32746b604..bfa38edf6d 100644 --- a/python/private/pypi/whl_library_targets.bzl +++ b/python/private/pypi/whl_library_targets.bzl @@ -29,6 +29,84 @@ load( "WHEEL_FILE_IMPL_LABEL", "WHEEL_FILE_PUBLIC_LABEL", ) +load(":parse_whl_name.bzl", "parse_whl_name") +load(":pep508_deps.bzl", "deps") +load(":whl_target_platforms.bzl", "whl_target_platforms") + +def whl_library_targets_from_requires( + *, + name, + metadata_name = "", + metadata_version = "", + requires_dist = [], + extras = [], + target_platforms = [], + python_version = None, + **kwargs): + """The macro to create whl targets from the METADATA. + + Args: + name: {type}`str` The wheel filename + metadata_name: {type}`str` The package name as written in wheel `METADATA`. + metadata_version: {type}`str` The package version as written in wheel `METADATA`. + requires_dist: {type}`list[str]` The list of `Requires-Dist` values from + the whl `METADATA`. + extras: {type}`list[str]` The list of requested extras. This essentially includes extra transitive dependencies in the final targets depending on the wheel `METADATA`. + target_platforms: {type}`list[str]` The list of target platforms to create + dependency closures for. + python_version: {type}`str` The python version to assume when parsing + the `METADATA`. This is only used when the `target_platforms` do not + include the version information. + **kwargs: Extra args passed to the {obj}`whl_library_targets` + """ + package_deps = _parse_requires_dist( + name = name, + python_version = python_version, + requires_dist = requires_dist, + extras = extras, + target_platforms = target_platforms, + ) + whl_library_targets( + name = name, + dependencies = package_deps.deps, + dependencies_by_platform = package_deps.deps_select, + tags = [ + "pypi_name={}".format(metadata_name), + "pypi_version={}".format(metadata_version), + ], + **kwargs + ) + +def _parse_requires_dist( + *, + name, + python_version, + requires_dist, + extras, + target_platforms): + # TODO @aignas 2025-04-15: split this function into 2 where we are passing `package_deps` from the first function to the second. + # + # this will make unit testing of the functions easier + parsed_whl = parse_whl_name(name) + + # NOTE @aignas 2023-12-04: if the wheel is a platform specific wheel, we + # only include deps for that target platform + if parsed_whl.platform_tag != "any": + target_platforms = [ + p.target_platform + for p in whl_target_platforms( + platform_tag = parsed_whl.platform_tag, + abi_tag = parsed_whl.abi_tag.strip("tm"), + ) + ] + + return deps( + name = normalize_name(parsed_whl.distribution), + requires_dist = requires_dist, + platforms = target_platforms, + extras = extras, + default_python_version = python_version, + ) def whl_library_targets( *, @@ -95,7 +173,6 @@ def whl_library_targets( platform: sorted([normalize_name(d) for d in deps]) for platform, deps in dependencies_by_platform.items() } - tags = sorted(tags) data = [] + data for filegroup_name, glob in filegroups.items(): diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index 4d86d6a6e0..ce5474e35b 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -436,7 +436,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \ pypi.whl_libraries().contains_exactly({ "pypi_312_torch_cp312_cp312_linux_x86_64_8800deef": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp312_linux_x86_64"], "filename": "torch-2.4.1+cpu-cp312-cp312-linux_x86_64.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "torch==2.4.1+cpu", @@ -445,7 +444,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \ }, "pypi_312_torch_cp312_cp312_manylinux_2_17_aarch64_36109432": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp312_linux_aarch64"], "filename": "torch-2.4.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "torch==2.4.1", @@ -454,7 +452,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \ }, "pypi_312_torch_cp312_cp312_win_amd64_3a570e5c": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp312_windows_x86_64"], "filename": "torch-2.4.1+cpu-cp312-cp312-win_amd64.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "torch==2.4.1+cpu", @@ -463,7 +460,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \ }, "pypi_312_torch_cp312_none_macosx_11_0_arm64_72b484d5": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp312_osx_aarch64"], "filename": "torch-2.4.1-cp312-none-macosx_11_0_arm64.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "torch==2.4.1", @@ -750,7 +746,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef pypi.whl_libraries().contains_exactly({ "pypi_315_any_name": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "extra_pip_args": ["--extra-args-for-sdist-building"], "filename": "any-name.tar.gz", "python_interpreter_target": "unit_test_interpreter_target", @@ -760,7 +755,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef }, "pypi_315_direct_without_sha_0_0_1_py3_none_any": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "filename": "direct_without_sha-0.0.1-py3-none-any.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "direct_without_sha==0.0.1 @ example-direct.org/direct_without_sha-0.0.1-py3-none-any.whl", @@ -781,7 +775,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef }, "pypi_315_simple_py3_none_any_deadb00f": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "filename": "simple-0.0.1-py3-none-any.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "simple==0.0.1", @@ -790,7 +783,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef }, "pypi_315_simple_sdist_deadbeef": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "extra_pip_args": ["--extra-args-for-sdist-building"], "filename": "simple-0.0.1.tar.gz", "python_interpreter_target": "unit_test_interpreter_target", @@ -800,7 +792,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef }, "pypi_315_some_pkg_py3_none_any_deadbaaf": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "filename": "some_pkg-0.0.1-py3-none-any.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl --hash=sha256:deadbaaf", @@ -809,7 +800,6 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef }, "pypi_315_some_py3_none_any_deadb33f": { "dep_template": "@pypi//{name}:{target}", - "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"], "filename": "some-other-pkg-0.0.1-py3-none-any.whl", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "some_other_pkg==0.0.1", diff --git a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl index b0d8f6d17e..7bd19b65c1 100644 --- a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl +++ b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl @@ -21,11 +21,11 @@ _tests = [] def _test_all(env): want = """\ -load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets") +load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires") package(default_visibility = ["//visibility:public"]) -whl_library_targets( +whl_library_targets_from_requires( copy_executables = { "exec_src": "exec_dest", }, @@ -38,19 +38,71 @@ whl_library_targets( "data_exclude_all", ], dep_template = "@pypi//{name}:{target}", - dependencies = [ + entry_points = { + "foo": "bar.py", + }, + group_deps = [ + "foo", + "fox", + "qux", + ], + group_name = "qux", + name = "foo.whl", + requires_dist = [ "foo", "bar-baz", "qux", ], - dependencies_by_platform = { - "linux_x86_64": [ - "box", - "box-amd64", - ], - "windows_x86_64": ["fox"], - "@platforms//os:linux": ["box"], + srcs_exclude = ["srcs_exclude_all"], + target_platforms = ["foo"], +) + +# SOMETHING SPECIAL AT THE END +""" + actual = generate_whl_library_build_bazel( + dep_template = "@pypi//{name}:{target}", + name = "foo.whl", + requires_dist = ["foo", "bar-baz", "qux"], + entry_points = { + "foo": "bar.py", + }, + data_exclude = ["exclude_via_attr"], + annotation = struct( + copy_files = {"file_src": "file_dest"}, + copy_executables = {"exec_src": "exec_dest"}, + data = ["extra_target"], + data_exclude_glob = ["data_exclude_all"], + srcs_exclude_glob = ["srcs_exclude_all"], + additive_build_content = """# SOMETHING SPECIAL AT THE END""", + ), + group_name = "qux", + target_platforms = ["foo"], + group_deps = ["foo", "fox", "qux"], + ) + env.expect.that_str(actual.replace("@@", "@")).equals(want) + +_tests.append(_test_all) + +def _test_all_with_loads(env): + want = """\ +load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires") +load("@pypi//:config.bzl", "target_platforms") + +package(default_visibility = ["//visibility:public"]) + +whl_library_targets_from_requires( + copy_executables = { + "exec_src": "exec_dest", }, + copy_files = { + "file_src": "file_dest", + }, + data = ["extra_target"], + data_exclude = [ + "exclude_via_attr", + "data_exclude_all", + ], + dep_template = "@pypi//{name}:{target}", entry_points = { "foo": "bar.py", }, @@ -61,11 +113,13 @@ whl_library_targets( ], group_name = "qux", name = "foo.whl", - srcs_exclude = ["srcs_exclude_all"], - tags = [ - "tag2", - "tag1", + requires_dist = [ + "foo", + "bar-baz", + "qux", ], + srcs_exclude = ["srcs_exclude_all"], + target_platforms = target_platforms, ) # SOMETHING SPECIAL AT THE END @@ -73,13 +127,7 @@ whl_library_targets( actual = generate_whl_library_build_bazel( dep_template = "@pypi//{name}:{target}", name = "foo.whl", - dependencies = ["foo", "bar-baz", "qux"], - dependencies_by_platform = { - "linux_x86_64": ["box", "box-amd64"], - "windows_x86_64": ["fox"], - "@platforms//os:linux": ["box"], # buildifier: disable=unsorted-dict-items to check that we sort inside the test - }, - tags = ["tag2", "tag1"], + requires_dist = ["foo", "bar-baz", "qux"], entry_points = { "foo": "bar.py", }, @@ -97,7 +145,7 @@ whl_library_targets( ) env.expect.that_str(actual.replace("@@", "@")).equals(want) -_tests.append(_test_all) +_tests.append(_test_all_with_loads) def generate_whl_library_build_bazel_test_suite(name): """Create the test suite. diff --git a/tests/pypi/pep508/deps_tests.bzl b/tests/pypi/pep508/deps_tests.bzl index 44031ab6a5..bb864d5d79 100644 --- a/tests/pypi/pep508/deps_tests.bzl +++ b/tests/pypi/pep508/deps_tests.bzl @@ -22,6 +22,7 @@ def test_simple_deps(env): got = deps( "foo", requires_dist = ["bar-Bar"], + platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar_bar"]) env.expect.that_dict(got.deps_select).contains_exactly({}) @@ -43,7 +44,7 @@ def test_can_add_os_specific_deps(env): "osx_aarch64", "windows_x86_64", ], - host_python_version = "3.3.1", + default_python_version = "3.3.1", ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -92,7 +93,7 @@ def test_deps_are_added_to_more_specialized_platforms(env): "osx_x86_64", "osx_aarch64", ], - host_python_version = "3.8.4", + default_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly([]) @@ -114,7 +115,7 @@ def test_deps_from_more_specialized_platforms_are_propagated(env): "osx_x86_64", "osx_aarch64", ], - host_python_version = "3.8.4", + default_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly([]) @@ -141,7 +142,7 @@ def test_non_platform_markers_are_added_to_common_deps(env): "osx_aarch64", "windows_x86_64", ], - host_python_version = "3.8.4", + default_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -161,6 +162,7 @@ def test_self_is_ignored(env): "ssl_lib; extra == 'ssl'", ], extras = ["ssl"], + platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar", "req_dep", "ssl_lib"]) @@ -179,6 +181,7 @@ def test_self_dependencies_can_come_in_any_order(env): "zdep; extra == 'all'", ], extras = ["all"], + platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz", "zdep"]) @@ -219,7 +222,7 @@ def _test_no_version_select_when_single_version(env): "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", "arch_dep; platform_machine=='x86_64' and python_version >= '3.8'", ] - host_python_version = "3.7.5" + default_python_version = "3.7.5" got = deps( "foo", @@ -228,7 +231,7 @@ def _test_no_version_select_when_single_version(env): "cp38_linux_x86_64", "cp38_windows_x86_64", ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -249,7 +252,7 @@ def _test_can_get_version_select(env): "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", "arch_dep; platform_machine=='x86_64' and python_version < '3.8'", ] - host_python_version = "3.7.4" + default_python_version = "3.7.4" got = deps( "foo", @@ -259,7 +262,7 @@ def _test_can_get_version_select(env): for minor in [7, 8, 9] for os in ["linux", "windows"] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -294,7 +297,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "baz (<2,>=1.11) ; python_version < '3.8'", "baz (<2,>=1.14) ; python_version >= '3.8'", ] - host_python_version = "3.8.4" + default_python_version = "3.8.4" got = deps( "foo", @@ -303,7 +306,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "cp3{}_linux_x86_64".format(minor) for minor in [7, 8, 9] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -312,7 +315,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): _tests.append(_test_deps_spanning_all_target_py_versions_are_added_to_common) def _test_deps_are_not_duplicated(env): - host_python_version = "3.7.4" + default_python_version = "3.7.4" # See an example in # https://files.pythonhosted.org/packages/76/9e/db1c2d56c04b97981c06663384f45f28950a73d9acf840c4006d60d0a1ff/opencv_python-4.9.0.80-cp37-abi3-win32.whl.metadata @@ -336,7 +339,7 @@ def _test_deps_are_not_duplicated(env): for os in ["linux", "osx", "windows"] for arch in ["x86_64", "aarch64"] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -345,7 +348,7 @@ def _test_deps_are_not_duplicated(env): _tests.append(_test_deps_are_not_duplicated) def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): - host_python_version = "3.7.1" + default_python_version = "3.7.1" # Note, that we are sorting the incoming `requires_dist` and we need to ensure that we are not getting any # issues even if the platform-specific line comes first. @@ -363,7 +366,7 @@ def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): "cp310_linux_aarch64", "cp310_linux_x86_64", ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) # TODO @aignas 2025-02-24: this test case in the python version is passing but diff --git a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl index f738e03b5d..f4b4490ec8 100644 --- a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl +++ b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl @@ -16,7 +16,7 @@ load("@rules_testing//lib:test_suite.bzl", "test_suite") load("//python/private:glob_excludes.bzl", "glob_excludes") # buildifier: disable=bzl-visibility -load("//python/private/pypi:whl_library_targets.bzl", "whl_library_targets") # buildifier: disable=bzl-visibility +load("//python/private/pypi:whl_library_targets.bzl", "whl_library_targets", "whl_library_targets_from_requires") # buildifier: disable=bzl-visibility _tests = [] @@ -183,6 +183,71 @@ def _test_entrypoints(env): _tests.append(_test_entrypoints) +def _test_whl_and_library_deps_from_requires(env): + filegroup_calls = [] + py_library_calls = [] + + whl_library_targets_from_requires( + name = "foo-0-py3-none-any.whl", + metadata_name = "Foo", + metadata_version = "0", + dep_template = "@pypi_{name}//:{target}", + requires_dist = [ + "foo", # this self-edge will be ignored + "bar-baz", + ], + target_platforms = ["cp38_linux_x86_64"], + python_version = "3.8.1", + data_exclude = [], + # Overrides for testing + filegroups = {}, + native = struct( + filegroup = lambda **kwargs: filegroup_calls.append(kwargs), + config_setting = lambda **_: None, + glob = _glob, + select = _select, + ), + rules = struct( + py_library = lambda **kwargs: py_library_calls.append(kwargs), + ), + ) + + env.expect.that_collection(filegroup_calls).contains_exactly([ + { + "name": "whl", + "srcs": ["foo-0-py3-none-any.whl"], + "data": ["@pypi_bar_baz//:whl"], + "visibility": ["//visibility:public"], + }, + ]) # buildifier: @unsorted-dict-items + env.expect.that_collection(py_library_calls).contains_exactly([ + { + "name": "pkg", + "srcs": _glob( + ["site-packages/**/*.py"], + exclude = [], + allow_empty = True, + ), + "pyi_srcs": _glob(["site-packages/**/*.pyi"], allow_empty = True), + "data": [] + _glob( + ["site-packages/**/*"], + exclude = [ + "**/*.py", + "**/*.pyc", + "**/*.pyc.*", + "**/*.dist-info/RECORD", + ] + glob_excludes.version_dependent_exclusions(), + ), + "imports": ["site-packages"], + "deps": ["@pypi_bar_baz//:pkg"], + "tags": ["pypi_name=Foo", "pypi_version=0"], + "visibility": ["//visibility:public"], + "experimental_venvs_site_packages": Label("//python/config_settings:venvs_site_packages"), + }, + ]) # buildifier: @unsorted-dict-items + +_tests.append(_test_whl_and_library_deps_from_requires) + def _test_whl_and_library_deps(env): filegroup_calls = [] py_library_calls = [] From 3757749300ecd9e033a1cc8933e5fdf5c1650502 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 16 Apr 2025 18:47:45 +0900 Subject: [PATCH 2/9] migrate mypy to ubuntu-latest --- .github/workflows/mypy.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mypy.yaml b/.github/workflows/mypy.yaml index 866c43abd1..477c4174a0 100644 --- a/.github/workflows/mypy.yaml +++ b/.github/workflows/mypy.yaml @@ -1,3 +1,4 @@ +--- name: mypy on: @@ -15,7 +16,7 @@ defaults: jobs: ci: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: # Checkout the code - uses: actions/checkout@v4 From a5777636443e21ce449e38d852385664ee184211 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 09:53:21 +0900 Subject: [PATCH 3/9] simplify the dependency algorithm --- python/private/pypi/pep508_deps.bzl | 245 +++++--------------- python/private/pypi/pep508_requirement.bzl | 4 +- python/private/pypi/whl_library_targets.bzl | 64 ++--- tests/pypi/pep508/deps_tests.bzl | 194 ++++++---------- 4 files changed, 152 insertions(+), 355 deletions(-) diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index fe8ca5ff4f..6b0595be84 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -15,37 +15,23 @@ """This module is for implementing PEP508 compliant METADATA deps parsing. """ -load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION") load("//python/private:normalize_name.bzl", "normalize_name") load(":pep508_env.bzl", "env") load(":pep508_evaluate.bzl", "evaluate") load(":pep508_platform.bzl", "platform", "platform_from_str") load(":pep508_requirement.bzl", "requirement") -_ALL_OS_VALUES = [ - "windows", - "osx", - "linux", -] -_ALL_ARCH_VALUES = [ - "aarch64", - "ppc64", - "ppc64le", - "s390x", - "x86_32", - "x86_64", -] - -def deps(name, *, requires_dist, platforms = [], extras = [], default_python_version = None): +def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], host_python_version = None): """Parse the RequiresDist from wheel METADATA Args: name: {type}`str` the name of the wheel. requires_dist: {type}`list[str]` the list of RequiresDist lines from the METADATA file. + excludes: {type}`list[str]` what packages should we exclude. extras: {type}`list[str]` the requested extras to generate targets for. platforms: {type}`list[str]` the list of target platform strings. - default_python_version: {type}`str` the host python version. + host_python_version: {type}`str` the host python version. Returns: A struct with attributes: @@ -53,55 +39,49 @@ def deps(name, *, requires_dist, platforms = [], extras = [], default_python_ver * deps_select: {type}`dict[str, list[str]]` dependencies to include on particular subset of target platforms. """ - if not platforms: - fail("'platforms' arg is mandatory") - - # TODO @aignas 2025-04-16: this `default_abi` variable is only - # here so that we can have selects without "is_python_version" conditions. - # This would happen in the `multi_pip_parse` logic in WORKSPACE. - # - # In followup PRs I would like to remove this logic and mainly rely on the - # fact that there will be only a single ABI when internal rules pass the - # values in the WORKSPACE case and in that case we can get the default - # version from the target platform strings. What is more when we drop - # WORKSPACE, we can simplify the logic in `_add_req` substantially and get - # rid of `default_abi` all together. - default_python_version = default_python_version or DEFAULT_PYTHON_VERSION + reqs = sorted( + [requirement(r) for r in requires_dist], + key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker), + ) + deps = {} + deps_select = {} + name = normalize_name(name) + want_extras = _resolve_extras(name, reqs, extras) + + # drop self edges + excludes = [name] + [normalize_name(x) for x in excludes] + platforms = [ - platform_from_str(p, python_version = default_python_version) + platform_from_str(p, python_version = host_python_version) for p in platforms ] abis = sorted({p.abi: True for p in platforms if p.abi}) - if default_python_version and len(abis) > 1: - _, _, minor_version = default_python_version.partition(".") + if host_python_version and len(abis) > 1: + _, _, minor_version = host_python_version.partition(".") minor_version, _, _ = minor_version.partition(".") default_abi = "cp3" + minor_version elif len(abis) > 1: fail( - "all python versions need to be specified explicitly with the default_python_version, got: {}, {}".format(platforms, default_python_version), + "all python versions need to be specified explicitly, got: {}".format(platforms), ) else: default_abi = None - reqs = sorted( - [requirement(r) for r in requires_dist], - key = lambda x: "{}:{}:".format(x.name, sorted(x.extras), x.marker), - ) - deps = {} - deps_select = {} - name = normalize_name(name) - want_extras = _resolve_extras(name, reqs, extras) + reqs_by_name = {} for req in reqs: - if req.name == name: - # drop self edges + if req.name_ in excludes: continue - _add_req( + reqs_by_name.setdefault(req.name, []).append(req) + + for name, reqs in reqs_by_name.items(): + _add_reqs( deps, deps_select, - req, + normalize_name(name), + reqs, extras = want_extras, platforms = platforms, default_abi = default_abi, @@ -120,13 +100,13 @@ def _platform_str(self): if not self.os and not self.arch: return "//conditions:default" elif not self.arch: - return "@platforms//os:{}".format(self.os) + fail("remove") else: return "{}_{}".format(self.os, self.arch) minor_version = self.abi[3:] if self.arch == None and self.os == None: - return str(Label("//python/config_settings:is_python_3.{}".format(minor_version))) + fail("remove") return "cp3{}_{}_{}".format( minor_version, @@ -134,32 +114,6 @@ def _platform_str(self): self.arch or "anyarch", ) -def _platform_specializations(self, cpu_values = _ALL_ARCH_VALUES, os_values = _ALL_OS_VALUES): - """Return the platform itself and all its unambiguous specializations. - - For more info about specializations see - https://bazel.build/docs/configurable-attributes - """ - specializations = [] - specializations.append(self) - if self.arch == None: - specializations.extend([ - platform(os = self.os, arch = arch, abi = self.abi) - for arch in cpu_values - ]) - if self.os == None: - specializations.extend([ - platform(os = os, arch = self.arch, abi = self.abi) - for os in os_values - ]) - if self.os == None and self.arch == None: - specializations.extend([ - platform(os = os, arch = arch, abi = self.abi) - for os in os_values - for arch in cpu_values - ]) - return specializations - def _add(deps, deps_select, dep, platform): dep = normalize_name(dep) @@ -186,53 +140,7 @@ def _add(deps, deps_select, dep, platform): return # Add the platform-specific branch - deps_select.setdefault(platform, {}) - - # Add the dep to specializations of the given platform if they - # exist in the select statement. - for p in _platform_specializations(platform): - if p not in deps_select: - continue - - deps_select[p][dep] = True - - if len(deps_select[platform]) == 1: - # We are adding a new item to the select and we need to ensure that - # existing dependencies from less specialized platforms are propagated - # to the newly added dependency set. - for p, _deps in deps_select.items(): - # Check if the existing platform overlaps with the given platform - if p == platform or platform not in _platform_specializations(p): - continue - - deps_select[platform].update(_deps) - -def _maybe_add_common_dep(deps, deps_select, platforms, dep): - abis = sorted({p.abi: True for p in platforms if p.abi}) - if len(abis) < 2: - return - - platforms = [platform()] + [ - platform(abi = abi) - for abi in abis - ] - - # If the dep is targeting all target python versions, lets add it to - # the common dependency list to simplify the select statements. - for p in platforms: - if p not in deps_select: - return - - if dep not in deps_select[p]: - return - - # All of the python version-specific branches have the dep, so lets add - # it to the common deps. - deps[dep] = True - for p in platforms: - deps_select[p].pop(dep) - if not deps_select[p]: - deps_select.pop(p) + deps_select.setdefault(platform, {})[dep] = True def _resolve_extras(self_name, reqs, extras): """Resolve extras which are due to depending on self[some_other_extra]. @@ -289,72 +197,37 @@ def _resolve_extras(self_name, reqs, extras): # Poor mans set return sorted({x: None for x in extras}) -def _add_req(deps, deps_select, req, *, extras, platforms, default_abi = None): - if not req.marker: - _add(deps, deps_select, req.name, None) - return - - match_os = len([ - tag - for tag in [ - "os_name", - "sys_platform", - "platform_system", - ] - if tag in req.marker - ]) > 0 - match_arch = "platform_machine" in req.marker - match_version = "version" in req.marker - - if not (match_os or match_arch or match_version): - if [ - True - for extra in extras - for p in platforms - if evaluate( - req.marker, - env = env( - target_platform = p, - extra = extra, - ), - ) - ]: - _add(deps, deps_select, req.name, None) - return +def _add_reqs(deps, deps_select, dep, reqs, *, extras, platforms, default_abi = None): + for req in reqs: + if not req.marker: + _add(deps, deps_select, dep, None) + return + platforms_to_add = {} for plat in platforms: - if not [ - True - for extra in extras - if evaluate( - req.marker, - env = env( - target_platform = plat, - extra = extra, - ), - ) - ]: + if plat in platforms_to_add: + # marker evaluation is more expensive than this check continue - if match_arch and default_abi: - _add(deps, deps_select, req.name, plat) - if plat.abi == default_abi: - _add(deps, deps_select, req.name, platform(os = plat.os, arch = plat.arch)) - elif match_arch: - _add(deps, deps_select, req.name, platform(os = plat.os, arch = plat.arch)) - elif match_os and default_abi: - _add(deps, deps_select, req.name, platform(os = plat.os, abi = plat.abi)) - if plat.abi == default_abi: - _add(deps, deps_select, req.name, platform(os = plat.os)) - elif match_os: - _add(deps, deps_select, req.name, platform(os = plat.os)) - elif match_version and default_abi: - _add(deps, deps_select, req.name, platform(abi = plat.abi)) - if plat.abi == default_abi: - _add(deps, deps_select, req.name, platform()) - elif match_version: - _add(deps, deps_select, req.name, None) - else: - fail("BUG: {} support is not implemented".format(req.marker)) + added = False + for extra in extras: + if added: + break + + for req in reqs: + if evaluate(req.marker, env = env(target_platform = plat, extra = extra)): + platforms_to_add[plat] = True + added = True + break + + if len(platforms_to_add) == len(platforms): + # the dep is in all target platforms, let's just add it to the regular + # list + _add(deps, deps_select, dep, None) + return - _maybe_add_common_dep(deps, deps_select, platforms, req.name) + for plat in platforms_to_add: + if default_abi: + _add(deps, deps_select, dep, plat) + if plat.abi == default_abi or not default_abi: + _add(deps, deps_select, dep, platform(os = plat.os, arch = plat.arch)) diff --git a/python/private/pypi/pep508_requirement.bzl b/python/private/pypi/pep508_requirement.bzl index ee7b5dfc35..b5be17f890 100644 --- a/python/private/pypi/pep508_requirement.bzl +++ b/python/private/pypi/pep508_requirement.bzl @@ -47,9 +47,11 @@ def requirement(spec): requires, _, _ = requires.partition(char) extras = extras_unparsed.replace(" ", "").split(",") name = requires.strip(" ") + name = normalize_name(name) return struct( - name = normalize_name(name).replace("_", "-"), + name = name.replace("_", "-"), + name_ = name, marker = marker.strip(" "), extras = extras, version = version, diff --git a/python/private/pypi/whl_library_targets.bzl b/python/private/pypi/whl_library_targets.bzl index bfa38edf6d..2b5b98f9c9 100644 --- a/python/private/pypi/whl_library_targets.bzl +++ b/python/private/pypi/whl_library_targets.bzl @@ -42,6 +42,7 @@ def whl_library_targets_from_requires( extras = [], target_platforms = [], python_version = None, + group_deps = [], **kwargs): """The macro to create whl targets from the METADATA. @@ -49,6 +50,10 @@ def whl_library_targets_from_requires( name: {type}`str` The wheel filename metadata_name: {type}`str` The package name as written in wheel `METADATA`. metadata_version: {type}`str` The package version as written in wheel `METADATA`. + group_deps: {type}`list[str]` names of fellow members of the group (if + any). These will be excluded from generated deps lists so as to avoid + direct cycles. These dependencies will be provided at runtime by the + group rules which wrap this library and its fellows together. requires_dist: {type}`list[str]` The list of `Requires-Dist` values from the whl `METADATA`. extras: {type}`list[str]` The list of requested extras. This essentially includes extra transitive dependencies in the final targets depending on the wheel `METADATA`. @@ -63,6 +68,7 @@ def whl_library_targets_from_requires( name = name, python_version = python_version, requires_dist = requires_dist, + exclude = group_deps, extras = extras, target_platforms = target_platforms, ) @@ -82,6 +88,7 @@ def _parse_requires_dist( name, python_version, requires_dist, + exclude, extras, target_platforms): # TODO @aignas 2025-04-15: split this function into 2 where we are passing `package_deps` from the first function to the second. @@ -104,6 +111,7 @@ def _parse_requires_dist( name = normalize_name(parsed_whl.distribution), requires_dist = requires_dist, platforms = target_platforms, + exclude = exclude, extras = extras, default_python_version = python_version, ) @@ -121,7 +129,6 @@ def whl_library_targets( }, dependencies = [], dependencies_by_platform = {}, - group_deps = [], group_name = "", data = [], copy_files = {}, @@ -150,10 +157,6 @@ def whl_library_targets( contains this library. If set, this library will behave as a shim to group implementation rules which will provide simultaneously installed dependencies which would otherwise form a cycle. - group_deps: {type}`list[str]` names of fellow members of the group (if - any). These will be excluded from generated deps lists so as to avoid - direct cycles. These dependencies will be provided at runtime by the - group rules which wrap this library and its fellows together. copy_executables: {type}`dict[str, str]` The mapping between src and dest locations for the targets. copy_files: {type}`dict[str, str]` The mapping between src and @@ -201,7 +204,11 @@ def whl_library_targets( data.append(dest) _config_settings( - dependencies_by_platform.keys(), + sorted({ + p: None + for platforms in dependencies_by_platform.values() + for p in platforms + }), native = native, visibility = ["//visibility:private"], ) @@ -220,25 +227,6 @@ def whl_library_targets( visibility = ["//visibility:public"], ) - # Ensure this list is normalized - # Note: mapping used as set - group_deps = { - normalize_name(d): True - for d in group_deps - } - - dependencies = [ - d - for d in dependencies - if d not in group_deps - ] - dependencies_by_platform = { - p: deps - for p, deps in dependencies_by_platform.items() - for deps in [[d for d in deps if d not in group_deps]] - if deps - } - # If this library is a member of a group, its public label aliases need to # point to the group implementation rule not the implementation rules. We # also need to mark the implementation rules as visible to the group @@ -405,22 +393,12 @@ def _plat_label(plat): def _deps(deps, deps_by_platform, tmpl, select = select): deps = [tmpl.format(d) for d in sorted(deps)] - if not deps_by_platform: - return deps + for dep, platforms in deps_by_platform.items(): + deps += select({ + "//conditions:default": [], + } | { + _plat_label(p): [tmpl.format(dep)] + for p in platforms + }) - deps_by_platform = { - _plat_label(p): [ - tmpl.format(d) - for d in sorted(deps) - ] - for p, deps in sorted(deps_by_platform.items()) - } - - # Add the default, which means that we will be just using the dependencies in - # `deps` for platforms that are not handled in a special way by the packages - deps_by_platform.setdefault("//conditions:default", []) - - if not deps: - return select(deps_by_platform) - else: - return deps + select(deps_by_platform) + return deps diff --git a/tests/pypi/pep508/deps_tests.bzl b/tests/pypi/pep508/deps_tests.bzl index bb864d5d79..efb28692e0 100644 --- a/tests/pypi/pep508/deps_tests.bzl +++ b/tests/pypi/pep508/deps_tests.bzl @@ -22,7 +22,6 @@ def test_simple_deps(env): got = deps( "foo", requires_dist = ["bar-Bar"], - platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar_bar"]) env.expect.that_dict(got.deps_select).contains_exactly({}) @@ -30,58 +29,48 @@ def test_simple_deps(env): _tests.append(test_simple_deps) def test_can_add_os_specific_deps(env): - got = deps( - "foo", - requires_dist = [ - "bar", - "an_osx_dep; sys_platform=='darwin'", - "posix_dep; os_name=='posix'", - "win_dep; os_name=='nt'", - ], - platforms = [ - "linux_x86_64", - "osx_x86_64", - "osx_aarch64", - "windows_x86_64", - ], - default_python_version = "3.3.1", - ) - - env.expect.that_collection(got.deps).contains_exactly(["bar"]) - env.expect.that_dict(got.deps_select).contains_exactly({ - "@platforms//os:linux": ["posix_dep"], - "@platforms//os:osx": ["an_osx_dep", "posix_dep"], - "@platforms//os:windows": ["win_dep"], - }) + for target in [ + struct( + platforms = [ + "linux_x86_64", + "osx_x86_64", + "osx_aarch64", + "windows_x86_64", + ], + python_version = "3.3.1", + ), + struct( + platforms = [ + "cp33_linux_x86_64", + "cp33_osx_x86_64", + "cp33_osx_aarch64", + "cp33_windows_x86_64", + ], + python_version = "", + ), + ]: + got = deps( + "foo", + requires_dist = [ + "bar", + "an_osx_dep; sys_platform=='darwin'", + "posix_dep; os_name=='posix'", + "win_dep; os_name=='nt'", + ], + platforms = target.platforms, + host_python_version = target.python_version, + ) + + env.expect.that_collection(got.deps).contains_exactly(["bar"]) + env.expect.that_dict(got.deps_select).contains_exactly({ + "linux_x86_64": ["posix_dep"], + "osx_aarch64": ["an_osx_dep", "posix_dep"], + "osx_x86_64": ["an_osx_dep", "posix_dep"], + "windows_x86_64": ["win_dep"], + }) _tests.append(test_can_add_os_specific_deps) -def test_can_add_os_specific_deps_with_python_version(env): - got = deps( - "foo", - requires_dist = [ - "bar", - "an_osx_dep; sys_platform=='darwin'", - "posix_dep; os_name=='posix'", - "win_dep; os_name=='nt'", - ], - platforms = [ - "cp33_linux_x86_64", - "cp33_osx_x86_64", - "cp33_osx_aarch64", - "cp33_windows_x86_64", - ], - ) - - env.expect.that_collection(got.deps).contains_exactly(["bar"]) - env.expect.that_dict(got.deps_select).contains_exactly({ - "@platforms//os:linux": ["posix_dep"], - "@platforms//os:osx": ["an_osx_dep", "posix_dep"], - "@platforms//os:windows": ["win_dep"], - }) - -_tests.append(test_can_add_os_specific_deps_with_python_version) - def test_deps_are_added_to_more_specialized_platforms(env): got = deps( "foo", @@ -93,41 +82,16 @@ def test_deps_are_added_to_more_specialized_platforms(env): "osx_x86_64", "osx_aarch64", ], - default_python_version = "3.8.4", + host_python_version = "3.8.4", ) - env.expect.that_collection(got.deps).contains_exactly([]) + env.expect.that_collection(got.deps).contains_exactly(["mac_dep"]) env.expect.that_dict(got.deps_select).contains_exactly({ - "@platforms//os:osx": ["mac_dep"], - "osx_aarch64": ["m1_dep", "mac_dep"], + "osx_aarch64": ["m1_dep"], }) _tests.append(test_deps_are_added_to_more_specialized_platforms) -def test_deps_from_more_specialized_platforms_are_propagated(env): - got = deps( - "foo", - requires_dist = [ - "a_mac_dep; sys_platform=='darwin'", - "m1_dep; sys_platform=='darwin' and platform_machine=='arm64'", - ], - platforms = [ - "osx_x86_64", - "osx_aarch64", - ], - default_python_version = "3.8.4", - ) - - env.expect.that_collection(got.deps).contains_exactly([]) - env.expect.that_dict(got.deps_select).contains_exactly( - { - "@platforms//os:osx": ["a_mac_dep"], - "osx_aarch64": ["a_mac_dep", "m1_dep"], - }, - ) - -_tests.append(test_deps_from_more_specialized_platforms_are_propagated) - def test_non_platform_markers_are_added_to_common_deps(env): got = deps( "foo", @@ -142,7 +106,7 @@ def test_non_platform_markers_are_added_to_common_deps(env): "osx_aarch64", "windows_x86_64", ], - default_python_version = "3.8.4", + host_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -162,7 +126,6 @@ def test_self_is_ignored(env): "ssl_lib; extra == 'ssl'", ], extras = ["ssl"], - platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar", "req_dep", "ssl_lib"]) @@ -181,7 +144,6 @@ def test_self_dependencies_can_come_in_any_order(env): "zdep; extra == 'all'", ], extras = ["all"], - platforms = ["cp38_linux_x86_64"], ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz", "zdep"]) @@ -207,38 +169,34 @@ def _test_can_get_deps_based_on_specific_python_version(env): platforms = ["cp37_linux_x86_64"], ) + # since there is a single target platform, the deps_select will be empty env.expect.that_collection(py37.deps).contains_exactly(["bar", "baz"]) env.expect.that_dict(py37.deps_select).contains_exactly({}) - env.expect.that_collection(py38.deps).contains_exactly(["bar"]) - env.expect.that_dict(py38.deps_select).contains_exactly({"@platforms//os:linux": ["posix_dep"]}) + env.expect.that_collection(py38.deps).contains_exactly(["bar", "posix_dep"]) + env.expect.that_dict(py38.deps_select).contains_exactly({}) _tests.append(_test_can_get_deps_based_on_specific_python_version) def _test_no_version_select_when_single_version(env): - requires_dist = [ - "bar", - "baz; python_version >= '3.8'", - "posix_dep; os_name=='posix'", - "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", - "arch_dep; platform_machine=='x86_64' and python_version >= '3.8'", - ] - default_python_version = "3.7.5" - got = deps( "foo", - requires_dist = requires_dist, + requires_dist = [ + "bar", + "baz; python_version >= '3.8'", + "posix_dep; os_name=='posix'", + "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", + "arch_dep; platform_machine=='x86_64' and python_version >= '3.8'", + ], platforms = [ "cp38_linux_x86_64", "cp38_windows_x86_64", ], - default_python_version = default_python_version, + host_python_version = "", ) - env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) + env.expect.that_collection(got.deps).contains_exactly(["bar", "baz", "arch_dep"]) env.expect.that_dict(got.deps_select).contains_exactly({ - "@platforms//os:linux": ["posix_dep", "posix_dep_with_version"], - "linux_x86_64": ["arch_dep", "posix_dep", "posix_dep_with_version"], - "windows_x86_64": ["arch_dep"], + "linux_x86_64": ["posix_dep", "posix_dep_with_version"], }) _tests.append(_test_no_version_select_when_single_version) @@ -252,7 +210,7 @@ def _test_can_get_version_select(env): "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", "arch_dep; platform_machine=='x86_64' and python_version < '3.8'", ] - default_python_version = "3.7.4" + host_python_version = "3.7.4" got = deps( "foo", @@ -262,31 +220,19 @@ def _test_can_get_version_select(env): for minor in [7, 8, 9] for os in ["linux", "windows"] ], - default_python_version = default_python_version, + host_python_version = host_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) env.expect.that_dict(got.deps_select).contains_exactly({ - str(Label("//python/config_settings:is_python_3.7")): ["baz"], - str(Label("//python/config_settings:is_python_3.8")): ["baz_new"], - str(Label("//python/config_settings:is_python_3.9")): ["baz_new"], - "@platforms//os:linux": ["baz", "posix_dep"], - "cp37_linux_anyarch": ["baz", "posix_dep"], "cp37_linux_x86_64": ["arch_dep", "baz", "posix_dep"], "cp37_windows_x86_64": ["arch_dep", "baz"], - "cp38_linux_anyarch": [ - "baz_new", - "posix_dep", - "posix_dep_with_version", - ], - "cp39_linux_anyarch": [ - "baz_new", - "posix_dep", - "posix_dep_with_version", - ], + "cp38_linux_x86_64": ["baz_new", "posix_dep", "posix_dep_with_version"], + "cp38_windows_x86_64": ["baz_new"], + "cp39_linux_x86_64": ["baz_new", "posix_dep", "posix_dep_with_version"], + "cp39_windows_x86_64": ["baz_new"], "linux_x86_64": ["arch_dep", "baz", "posix_dep"], "windows_x86_64": ["arch_dep", "baz"], - "//conditions:default": ["baz"], }) _tests.append(_test_can_get_version_select) @@ -297,7 +243,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "baz (<2,>=1.11) ; python_version < '3.8'", "baz (<2,>=1.14) ; python_version >= '3.8'", ] - default_python_version = "3.8.4" + host_python_version = "3.8.4" got = deps( "foo", @@ -306,7 +252,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "cp3{}_linux_x86_64".format(minor) for minor in [7, 8, 9] ], - default_python_version = default_python_version, + host_python_version = host_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -315,7 +261,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): _tests.append(_test_deps_spanning_all_target_py_versions_are_added_to_common) def _test_deps_are_not_duplicated(env): - default_python_version = "3.7.4" + host_python_version = "3.7.4" # See an example in # https://files.pythonhosted.org/packages/76/9e/db1c2d56c04b97981c06663384f45f28950a73d9acf840c4006d60d0a1ff/opencv_python-4.9.0.80-cp37-abi3-win32.whl.metadata @@ -339,7 +285,7 @@ def _test_deps_are_not_duplicated(env): for os in ["linux", "osx", "windows"] for arch in ["x86_64", "aarch64"] ], - default_python_version = default_python_version, + host_python_version = host_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -348,7 +294,7 @@ def _test_deps_are_not_duplicated(env): _tests.append(_test_deps_are_not_duplicated) def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): - default_python_version = "3.7.1" + host_python_version = "3.7.1" # Note, that we are sorting the incoming `requires_dist` and we need to ensure that we are not getting any # issues even if the platform-specific line comes first. @@ -366,15 +312,13 @@ def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): "cp310_linux_aarch64", "cp310_linux_x86_64", ], - default_python_version = default_python_version, + host_python_version = host_python_version, ) - # TODO @aignas 2025-02-24: this test case in the python version is passing but - # I am not sure why. The starlark version behaviour looks more correct. env.expect.that_collection(got.deps).contains_exactly([]) env.expect.that_dict(got.deps_select).contains_exactly({ - str(Label("//python/config_settings:is_python_3.10")): ["bar"], "cp310_linux_aarch64": ["bar"], + "cp310_linux_x86_64": ["bar"], "cp37_linux_aarch64": ["bar"], "linux_aarch64": ["bar"], }) From 6a611de79616d88756cdfb7f494b39ae5ec47396 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 09:59:28 +0900 Subject: [PATCH 4/9] fix it --- python/private/pypi/pep508_deps.bzl | 2 + python/private/pypi/whl_library_targets.bzl | 68 +++++++++++++++------ 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index 6b0595be84..246e8c4f97 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -15,6 +15,7 @@ """This module is for implementing PEP508 compliant METADATA deps parsing. """ +load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION") load("//python/private:normalize_name.bzl", "normalize_name") load(":pep508_env.bzl", "env") load(":pep508_evaluate.bzl", "evaluate") @@ -51,6 +52,7 @@ def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], hos # drop self edges excludes = [name] + [normalize_name(x) for x in excludes] + host_python_version = host_python_version or DEFAULT_PYTHON_VERSION platforms = [ platform_from_str(p, python_version = host_python_version) for p in platforms diff --git a/python/private/pypi/whl_library_targets.bzl b/python/private/pypi/whl_library_targets.bzl index 2b5b98f9c9..b2a53fee81 100644 --- a/python/private/pypi/whl_library_targets.bzl +++ b/python/private/pypi/whl_library_targets.bzl @@ -68,7 +68,7 @@ def whl_library_targets_from_requires( name = name, python_version = python_version, requires_dist = requires_dist, - exclude = group_deps, + excludes = group_deps, extras = extras, target_platforms = target_platforms, ) @@ -88,12 +88,9 @@ def _parse_requires_dist( name, python_version, requires_dist, - exclude, + excludes, extras, target_platforms): - # TODO @aignas 2025-04-15: split this function into 2 where we are passing `package_deps` from the first function to the second. - # - # this will make unit testing of the functions easier parsed_whl = parse_whl_name(name) # NOTE @aignas 2023-12-04: if the wheel is a platform specific wheel, we @@ -111,9 +108,9 @@ def _parse_requires_dist( name = normalize_name(parsed_whl.distribution), requires_dist = requires_dist, platforms = target_platforms, - exclude = exclude, + excludes = excludes, extras = extras, - default_python_version = python_version, + host_python_version = python_version, ) def whl_library_targets( @@ -129,6 +126,7 @@ def whl_library_targets( }, dependencies = [], dependencies_by_platform = {}, + group_deps = [], group_name = "", data = [], copy_files = {}, @@ -157,6 +155,10 @@ def whl_library_targets( contains this library. If set, this library will behave as a shim to group implementation rules which will provide simultaneously installed dependencies which would otherwise form a cycle. + group_deps: {type}`list[str]` names of fellow members of the group (if + any). These will be excluded from generated deps lists so as to avoid + direct cycles. These dependencies will be provided at runtime by the + group rules which wrap this library and its fellows together. copy_executables: {type}`dict[str, str]` The mapping between src and dest locations for the targets. copy_files: {type}`dict[str, str]` The mapping between src and @@ -176,6 +178,7 @@ def whl_library_targets( platform: sorted([normalize_name(d) for d in deps]) for platform, deps in dependencies_by_platform.items() } + tags = sorted(tags) data = [] + data for filegroup_name, glob in filegroups.items(): @@ -204,11 +207,7 @@ def whl_library_targets( data.append(dest) _config_settings( - sorted({ - p: None - for platforms in dependencies_by_platform.values() - for p in platforms - }), + dependencies_by_platform.keys(), native = native, visibility = ["//visibility:private"], ) @@ -227,6 +226,25 @@ def whl_library_targets( visibility = ["//visibility:public"], ) + # Ensure this list is normalized + # Note: mapping used as set + group_deps = { + normalize_name(d): True + for d in group_deps + } + + dependencies = [ + d + for d in dependencies + if d not in group_deps + ] + dependencies_by_platform = { + p: deps + for p, deps in dependencies_by_platform.items() + for deps in [[d for d in deps if d not in group_deps]] + if deps + } + # If this library is a member of a group, its public label aliases need to # point to the group implementation rule not the implementation rules. We # also need to mark the implementation rules as visible to the group @@ -393,12 +411,22 @@ def _plat_label(plat): def _deps(deps, deps_by_platform, tmpl, select = select): deps = [tmpl.format(d) for d in sorted(deps)] - for dep, platforms in deps_by_platform.items(): - deps += select({ - "//conditions:default": [], - } | { - _plat_label(p): [tmpl.format(dep)] - for p in platforms - }) + if not deps_by_platform: + return deps - return deps + deps_by_platform = { + _plat_label(p): [ + tmpl.format(d) + for d in sorted(deps) + ] + for p, deps in sorted(deps_by_platform.items()) + } + + # Add the default, which means that we will be just using the dependencies in + # `deps` for platforms that are not handled in a special way by the packages + deps_by_platform.setdefault("//conditions:default", []) + + if not deps: + return select(deps_by_platform) + else: + return deps + select(deps_by_platform) From f768fe68a770bdec6fc0430b22f57cdae29643f0 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 10:07:12 +0900 Subject: [PATCH 5/9] cleanup dead code --- .../pypi/generate_whl_library_build_bazel.bzl | 4 ++++ python/private/pypi/pep508_deps.bzl | 15 +++------------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/python/private/pypi/generate_whl_library_build_bazel.bzl b/python/private/pypi/generate_whl_library_build_bazel.bzl index 801db56304..2a70859273 100644 --- a/python/private/pypi/generate_whl_library_build_bazel.bzl +++ b/python/private/pypi/generate_whl_library_build_bazel.bzl @@ -45,11 +45,13 @@ whl_library_targets_from_requires( def generate_whl_library_build_bazel( *, annotation = None, + python_version = None, **kwargs): """Generate a BUILD file for an unzipped Wheel Args: annotation: The annotation for the build file. + python_version: The python version to use to parse the METADATA. **kwargs: Extra args serialized to be passed to the {obj}`whl_library_targets`. @@ -78,6 +80,8 @@ def generate_whl_library_build_bazel( kwargs["srcs_exclude"] = annotation.srcs_exclude_glob if annotation.additive_build_content: additional_content.append(annotation.additive_build_content) + if python_version: + kwargs["python_version"] = repr(python_version) contents = "\n".join( [ diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index 246e8c4f97..da7bb2c127 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -99,19 +99,10 @@ def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], hos def _platform_str(self): if self.abi == None: - if not self.os and not self.arch: - return "//conditions:default" - elif not self.arch: - fail("remove") - else: - return "{}_{}".format(self.os, self.arch) - - minor_version = self.abi[3:] - if self.arch == None and self.os == None: - fail("remove") + return "{}_{}".format(self.os, self.arch) - return "cp3{}_{}_{}".format( - minor_version, + return "{}_{}_{}".format( + self.abi, self.os or "anyos", self.arch or "anyarch", ) From 3b9cdd834ee9b73bee10097fadc879b83eabc76e Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 10:44:11 +0900 Subject: [PATCH 6/9] fixup the default_python_version setting --- .../pypi/generate_whl_library_build_bazel.bzl | 8 ++++---- python/private/pypi/pep508_deps.bzl | 12 ++++++------ python/private/pypi/whl_library.bzl | 8 ++++---- python/private/pypi/whl_library_targets.bzl | 10 +++++----- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/python/private/pypi/generate_whl_library_build_bazel.bzl b/python/private/pypi/generate_whl_library_build_bazel.bzl index 2a70859273..7988aca1c4 100644 --- a/python/private/pypi/generate_whl_library_build_bazel.bzl +++ b/python/private/pypi/generate_whl_library_build_bazel.bzl @@ -45,13 +45,13 @@ whl_library_targets_from_requires( def generate_whl_library_build_bazel( *, annotation = None, - python_version = None, + default_python_version = None, **kwargs): """Generate a BUILD file for an unzipped Wheel Args: annotation: The annotation for the build file. - python_version: The python version to use to parse the METADATA. + default_python_version: The python version to use to parse the METADATA. **kwargs: Extra args serialized to be passed to the {obj}`whl_library_targets`. @@ -80,8 +80,8 @@ def generate_whl_library_build_bazel( kwargs["srcs_exclude"] = annotation.srcs_exclude_glob if annotation.additive_build_content: additional_content.append(annotation.additive_build_content) - if python_version: - kwargs["python_version"] = repr(python_version) + if default_python_version: + kwargs["default_python_version"] = default_python_version contents = "\n".join( [ diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index da7bb2c127..115bbd78d8 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -22,7 +22,7 @@ load(":pep508_evaluate.bzl", "evaluate") load(":pep508_platform.bzl", "platform", "platform_from_str") load(":pep508_requirement.bzl", "requirement") -def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], host_python_version = None): +def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], default_python_version = None): """Parse the RequiresDist from wheel METADATA Args: @@ -32,7 +32,7 @@ def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], hos excludes: {type}`list[str]` what packages should we exclude. extras: {type}`list[str]` the requested extras to generate targets for. platforms: {type}`list[str]` the list of target platform strings. - host_python_version: {type}`str` the host python version. + default_python_version: {type}`str` the host python version. Returns: A struct with attributes: @@ -52,15 +52,15 @@ def deps(name, *, requires_dist, platforms = [], extras = [], excludes = [], hos # drop self edges excludes = [name] + [normalize_name(x) for x in excludes] - host_python_version = host_python_version or DEFAULT_PYTHON_VERSION + default_python_version = default_python_version or DEFAULT_PYTHON_VERSION platforms = [ - platform_from_str(p, python_version = host_python_version) + platform_from_str(p, python_version = default_python_version) for p in platforms ] abis = sorted({p.abi: True for p in platforms if p.abi}) - if host_python_version and len(abis) > 1: - _, _, minor_version = host_python_version.partition(".") + if default_python_version and len(abis) > 1: + _, _, minor_version = default_python_version.partition(".") minor_version, _, _ = minor_version.partition(".") default_abi = "cp3" + minor_version elif len(abis) > 1: diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index ea22a25d2f..630dc8519f 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -387,7 +387,7 @@ def _whl_library_impl(rctx): if BZLMOD_ENABLED: # The following attributes are unset on bzlmod and we pass data through # the hub via load statements. - python_version = None + default_python_version = None target_platforms = [] else: # NOTE @aignas 2025-04-16: if BZLMOD_ENABLED, we should use @@ -397,9 +397,9 @@ def _whl_library_impl(rctx): # `pip_parse` invocation, so we will have the host target platform # only. Even if somebody would change the code to support # `experimental_target_platforms`, they would be for a single python - # version. Hence, using the `python_version` that we get from the + # version. Hence, using the `default_python_version` that we get from the # interpreter is correct. Hence, we unset the argument if we are on bzlmod. - python_version = metadata["python_version"] + default_python_version = metadata["python_version"] target_platforms = rctx.attr.experimental_target_platforms or [host_platform(rctx)] metadata = whl_metadata( @@ -416,7 +416,7 @@ def _whl_library_impl(rctx): dep_template = rctx.attr.dep_template or "@{}{{name}}//:{{target}}".format(rctx.attr.repo_prefix), entry_points = entry_points, target_platforms = target_platforms, - python_version = python_version, + default_python_version = default_python_version, # TODO @aignas 2025-04-14: load through the hub: annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))), data_exclude = rctx.attr.pip_data_exclude, diff --git a/python/private/pypi/whl_library_targets.bzl b/python/private/pypi/whl_library_targets.bzl index b2a53fee81..cf3df133c4 100644 --- a/python/private/pypi/whl_library_targets.bzl +++ b/python/private/pypi/whl_library_targets.bzl @@ -41,7 +41,7 @@ def whl_library_targets_from_requires( requires_dist = [], extras = [], target_platforms = [], - python_version = None, + default_python_version = None, group_deps = [], **kwargs): """The macro to create whl targets from the METADATA. @@ -59,14 +59,14 @@ def whl_library_targets_from_requires( extras: {type}`list[str]` The list of requested extras. This essentially includes extra transitive dependencies in the final targets depending on the wheel `METADATA`. target_platforms: {type}`list[str]` The list of target platforms to create dependency closures for. - python_version: {type}`str` The python version to assume when parsing + default_python_version: {type}`str` The python version to assume when parsing the `METADATA`. This is only used when the `target_platforms` do not include the version information. **kwargs: Extra args passed to the {obj}`whl_library_targets` """ package_deps = _parse_requires_dist( name = name, - python_version = python_version, + default_python_version = default_python_version, requires_dist = requires_dist, excludes = group_deps, extras = extras, @@ -86,7 +86,7 @@ def whl_library_targets_from_requires( def _parse_requires_dist( *, name, - python_version, + default_python_version, requires_dist, excludes, extras, @@ -110,7 +110,7 @@ def _parse_requires_dist( platforms = target_platforms, excludes = excludes, extras = extras, - host_python_version = python_version, + default_python_version = default_python_version, ) def whl_library_targets( From 3ce70d41a87ecb9ff99a00e600306c4691b3b225 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 10:46:50 +0900 Subject: [PATCH 7/9] fixup unit tests --- tests/pypi/pep508/deps_tests.bzl | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/pypi/pep508/deps_tests.bzl b/tests/pypi/pep508/deps_tests.bzl index efb28692e0..d362925080 100644 --- a/tests/pypi/pep508/deps_tests.bzl +++ b/tests/pypi/pep508/deps_tests.bzl @@ -58,7 +58,7 @@ def test_can_add_os_specific_deps(env): "win_dep; os_name=='nt'", ], platforms = target.platforms, - host_python_version = target.python_version, + default_python_version = target.python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -82,7 +82,7 @@ def test_deps_are_added_to_more_specialized_platforms(env): "osx_x86_64", "osx_aarch64", ], - host_python_version = "3.8.4", + default_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly(["mac_dep"]) @@ -106,7 +106,7 @@ def test_non_platform_markers_are_added_to_common_deps(env): "osx_aarch64", "windows_x86_64", ], - host_python_version = "3.8.4", + default_python_version = "3.8.4", ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -191,7 +191,7 @@ def _test_no_version_select_when_single_version(env): "cp38_linux_x86_64", "cp38_windows_x86_64", ], - host_python_version = "", + default_python_version = "", ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz", "arch_dep"]) @@ -210,7 +210,7 @@ def _test_can_get_version_select(env): "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", "arch_dep; platform_machine=='x86_64' and python_version < '3.8'", ] - host_python_version = "3.7.4" + default_python_version = "3.7.4" got = deps( "foo", @@ -220,7 +220,7 @@ def _test_can_get_version_select(env): for minor in [7, 8, 9] for os in ["linux", "windows"] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -243,7 +243,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "baz (<2,>=1.11) ; python_version < '3.8'", "baz (<2,>=1.14) ; python_version >= '3.8'", ] - host_python_version = "3.8.4" + default_python_version = "3.8.4" got = deps( "foo", @@ -252,7 +252,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): "cp3{}_linux_x86_64".format(minor) for minor in [7, 8, 9] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) @@ -261,7 +261,7 @@ def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): _tests.append(_test_deps_spanning_all_target_py_versions_are_added_to_common) def _test_deps_are_not_duplicated(env): - host_python_version = "3.7.4" + default_python_version = "3.7.4" # See an example in # https://files.pythonhosted.org/packages/76/9e/db1c2d56c04b97981c06663384f45f28950a73d9acf840c4006d60d0a1ff/opencv_python-4.9.0.80-cp37-abi3-win32.whl.metadata @@ -285,7 +285,7 @@ def _test_deps_are_not_duplicated(env): for os in ["linux", "osx", "windows"] for arch in ["x86_64", "aarch64"] ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) @@ -294,7 +294,7 @@ def _test_deps_are_not_duplicated(env): _tests.append(_test_deps_are_not_duplicated) def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): - host_python_version = "3.7.1" + default_python_version = "3.7.1" # Note, that we are sorting the incoming `requires_dist` and we need to ensure that we are not getting any # issues even if the platform-specific line comes first. @@ -312,7 +312,7 @@ def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): "cp310_linux_aarch64", "cp310_linux_x86_64", ], - host_python_version = host_python_version, + default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly([]) From 85acec5bf8e120ad1f7e847e5b9e439f6d823916 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 10:47:29 +0900 Subject: [PATCH 8/9] fixup the rename of the var --- tests/pypi/whl_library_targets/whl_library_targets_tests.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl index f4b4490ec8..61e5441050 100644 --- a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl +++ b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl @@ -197,7 +197,7 @@ def _test_whl_and_library_deps_from_requires(env): "bar-baz", ], target_platforms = ["cp38_linux_x86_64"], - python_version = "3.8.1", + default_python_version = "3.8.1", data_exclude = [], # Overrides for testing filegroups = {}, From 3ea01bb2b48ec89a131b55644c641a3f6c63cfc6 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 20 Apr 2025 19:01:17 +0900 Subject: [PATCH 9/9] address comments and add docs --- .github/workflows/mypy.yaml | 1 - CHANGELOG.md | 7 +++++++ python/private/pypi/attrs.bzl | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mypy.yaml b/.github/workflows/mypy.yaml index 477c4174a0..e774b9b03b 100644 --- a/.github/workflows/mypy.yaml +++ b/.github/workflows/mypy.yaml @@ -1,4 +1,3 @@ ---- name: mypy on: diff --git a/CHANGELOG.md b/CHANGELOG.md index 47ccd2459a..dfbdce8049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,13 @@ Unreleased changes template. [PR #2746](https://github.com/bazel-contrib/rules_python/pull/2746). * (rules) {attr}`py_binary.srcs` and {attr}`py_test.srcs` is no longer mandatory when `main_module` is specified (for `--bootstrap_impl=script`) +* (pypi) From now on the `Requires-Dist` from the wheel metadata is analysed in + the loading phase instead of repository rule phase giving better caching + performance when the target platforms are changed (e.g. target python + versions). This is preparatory work for stabilizing the cross-platform wheel + support. From now on the usage of `experimental_target_platforms` should be + avoided and the `requirements_by_platform` values should be instead used to + specify the target platforms for the given dependencies. [20250317]: https://github.com/astral-sh/python-build-standalone/releases/tag/20250317 diff --git a/python/private/pypi/attrs.bzl b/python/private/pypi/attrs.bzl index 9d88c1e32c..fe35d8bf7d 100644 --- a/python/private/pypi/attrs.bzl +++ b/python/private/pypi/attrs.bzl @@ -123,6 +123,9 @@ Warning: "experimental_target_platforms": attr.string_list( default = [], doc = """\ +*NOTE*: This will be removed in the next major version, so please consider migrating +to `bzlmod` and rely on {attr}`pip.parse.requirements_by_platform` for this feature. + A list of platforms that we will generate the conditional dependency graph for cross platform wheels by parsing the wheel metadata. This will generate the correct dependencies for packages like `sphinx` or `pylint`, which include