From ead607103cf40fbe6bd1d26fa89c976fd0f78c99 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 4 Jul 2025 09:59:48 +0900 Subject: [PATCH 1/2] fix(pypi): correctly handle custom names in pipstar platforms Before it seems that we were relying on particular names in the pipstar platforms. This ensures that we rely on this less. Whilst at it fix a few typos and improve the formatting of the code. Work towards #2949 Work towards #2747 --- CHANGELOG.md | 2 ++ python/private/pypi/BUILD.bazel | 4 ---- python/private/pypi/evaluate_markers.bzl | 2 +- python/private/pypi/extension.bzl | 14 ++++++++++++-- python/private/pypi/parse_requirements.bzl | 6 +++++- python/private/pypi/pep508_env.bzl | 5 ----- tests/pypi/extension/extension_tests.bzl | 20 +++++++++----------- tests/pypi/pep508/evaluate_tests.bzl | 3 ++- 8 files changed, 31 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81768af36a..2b57af606e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,8 @@ END_UNRELEASED_TEMPLATE * (toolchains) `local_runtime_repo` now checks if the include directory exists before attempting to watch it, fixing issues on macOS with system Python ({gh-issue}`3043`). +* (pypi) The pipstar `defaults` configuration now supports any custom platform + name. {#v0-0-0-added} ### Added diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index 2666197786..b098f29e94 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -252,10 +252,6 @@ bzl_library( bzl_library( name = "pep508_env_bzl", srcs = ["pep508_env.bzl"], - deps = [ - ":pep508_platform_bzl", - "//python/private:version_bzl", - ], ) bzl_library( diff --git a/python/private/pypi/evaluate_markers.bzl b/python/private/pypi/evaluate_markers.bzl index 2b805c33e6..6167cdbc96 100644 --- a/python/private/pypi/evaluate_markers.bzl +++ b/python/private/pypi/evaluate_markers.bzl @@ -57,7 +57,7 @@ def evaluate_markers_py(mrctx, *, requirements, python_interpreter, python_inter Args: mrctx: repository_ctx or module_ctx. - requirements: list[str] of the requirement file lines to evaluate. + requirements: {type}`dict[str, list[str]]` of the requirement file lines to evaluate. python_interpreter: str, path to the python_interpreter to use to evaluate the env markers in the given requirements files. It will be only called if the requirements files have env markers. This diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index a0095f8f15..64006bd9db 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -76,7 +76,11 @@ def _platforms(*, python_version, minor_mapping, config): for platform, values in config.platforms.items(): key = "{}_{}".format(abi, platform) - platforms[key] = env(key) | values.env + platforms[key] = env(struct( + abi = abi, + os = values.os_name, + arch = values.arch_name, + )) | values.env return platforms def _create_whl_repos( @@ -348,7 +352,7 @@ def _whl_repo(*, src, whl_library_args, is_multiple_versions, download_only, net args["filename"] = src.filename if not enable_pipstar: args["experimental_target_platforms"] = [ - # Get rid of the version fot the target platforms because we are + # Get rid of the version for the target platforms because we are # passing the interpreter any way. Ideally we should search of ways # how to pass the target platforms through the hub repo. p.partition("_")[2] @@ -383,6 +387,12 @@ def _configure(config, *, platform, os_name, arch_name, config_settings, env = { if key not in _SUPPORTED_PEP508_KEYS: fail("Unsupported key in the PEP508 environment: {}".format(key)) + if not os_name: + fail("'os_name' is required") + + if not arch_name: + fail("'arch_name' is required") + config["platforms"][platform] = struct( name = platform.replace("-", "_").lower(), os_name = os_name, diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index e4a8b90acb..9c610f11d3 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -402,6 +402,10 @@ def _add_dists(*, requirement, index_urls, logger = None): ])) # Filter out the wheels that are incompatible with the target_platforms. - whls = select_whls(whls = whls, want_platforms = requirement.target_platforms, logger = logger) + whls = select_whls( + whls = whls, + want_platforms = requirement.target_platforms, + logger = logger, + ) return whls, sdist diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl index a6efb3c50c..c2d404bc3e 100644 --- a/python/private/pypi/pep508_env.bzl +++ b/python/private/pypi/pep508_env.bzl @@ -15,8 +15,6 @@ """This module is for implementing PEP508 environment definition. """ -load(":pep508_platform.bzl", "platform_from_str") - # See https://stackoverflow.com/a/45125525 platform_machine_aliases = { # These pairs mean the same hardware, but different values may be used @@ -175,9 +173,6 @@ def env(target_platform, *, extra = None): if extra != None: env["extra"] = extra - if type(target_platform) == type(""): - target_platform = platform_from_str(target_platform, python_version = "") - if target_platform.abi: minor_version, _, micro_version = target_platform.abi[3:].partition(".") micro_version = micro_version or "0" diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index 146293ee8d..51e11342fc 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -1048,7 +1048,9 @@ def _test_pipstar_platforms(env): name = "rules_python", default = [ _default( - platform = "{}_{}".format(os, cpu), + platform = "my{}_{}".format(os, cpu), + os_name = os, + arch_name = cpu, config_settings = [ "@platforms//os:{}".format(os), "@platforms//cpu:{}".format(cpu), @@ -1086,24 +1088,20 @@ optimum[onnxruntime-gpu]==1.17.1 ; sys_platform == 'linux' pypi.hub_whl_map().contains_exactly({ "pypi": { "optimum": { - "pypi_315_optimum_linux_x86_64": [ + "pypi_315_optimum_mylinux_x86_64": [ whl_config_setting( version = "3.15", target_platforms = [ - "cp315_linux_x86_64", + "cp315_mylinux_x86_64", ], - config_setting = None, - filename = None, ), ], - "pypi_315_optimum_osx_aarch64": [ + "pypi_315_optimum_myosx_aarch64": [ whl_config_setting( version = "3.15", target_platforms = [ - "cp315_osx_aarch64", + "cp315_myosx_aarch64", ], - config_setting = None, - filename = None, ), ], }, @@ -1111,12 +1109,12 @@ optimum[onnxruntime-gpu]==1.17.1 ; sys_platform == 'linux' }) pypi.whl_libraries().contains_exactly({ - "pypi_315_optimum_linux_x86_64": { + "pypi_315_optimum_mylinux_x86_64": { "dep_template": "@pypi//{name}:{target}", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "optimum[onnxruntime-gpu]==1.17.1", }, - "pypi_315_optimum_osx_aarch64": { + "pypi_315_optimum_myosx_aarch64": { "dep_template": "@pypi//{name}:{target}", "python_interpreter_target": "unit_test_interpreter_target", "requirement": "optimum[onnxruntime]==1.17.1", diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl index 7b6c064b94..cc867f346c 100644 --- a/tests/pypi/pep508/evaluate_tests.bzl +++ b/tests/pypi/pep508/evaluate_tests.bzl @@ -16,6 +16,7 @@ load("@rules_testing//lib:test_suite.bzl", "test_suite") load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # buildifier: disable=bzl-visibility +load("//python/private/pypi:pep508_platform.bzl", "platform_from_str") # buildifier: disable=bzl-visibility _tests = [] @@ -262,7 +263,7 @@ def _evaluate_with_aliases(env): }, }.items(): # buildifier: @unsorted-dict-items for input, want in tests.items(): - _check_evaluate(env, input, want, pep508_env(target_platform)) + _check_evaluate(env, input, want, pep508_env(platform_from_str(target_platform, ""))) _tests.append(_evaluate_with_aliases) From eda85c5a87713b8a9fd42a79b0ccebcc622ac9ac Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 7 Jul 2025 11:07:48 +0900 Subject: [PATCH 2/2] Apply suggestion from @aignas --- python/private/pypi/extension.bzl | 6 ------ 1 file changed, 6 deletions(-) diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 64006bd9db..1495ebfef7 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -387,12 +387,6 @@ def _configure(config, *, platform, os_name, arch_name, config_settings, env = { if key not in _SUPPORTED_PEP508_KEYS: fail("Unsupported key in the PEP508 environment: {}".format(key)) - if not os_name: - fail("'os_name' is required") - - if not arch_name: - fail("'arch_name' is required") - config["platforms"][platform] = struct( name = platform.replace("-", "_").lower(), os_name = os_name,