Skip to content

fix(pypi): correctly handle custom names in pipstar platforms #3054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2025
Merged

fix(pypi): correctly handle custom names in pipstar platforms #3054

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ead6071
fix(pypi): correctly handle custom names in pipstar platforms
aignas Jul 4, 2025
eda85c5
Apply suggestion from @aignas
aignas Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions python/private/pypi/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion python/private/pypi/evaluate_markers.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions python/private/pypi/extension.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were os_name and arch_name effectively required before, or is this new? Though, I think this pip.defaults API is still experimental, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is new. It is experimental, but I've been dragging some of the hidden constants from various parts of the PyPI code into the internally defined pip.default instances. This has been really useful in that sense. I think maybe I want to revert this requirement. It is not going to be compatible with the builder pattern.


if not arch_name:
fail("'arch_name' is required")

config["platforms"][platform] = struct(
name = platform.replace("-", "_").lower(),
os_name = os_name,
Expand Down
6 changes: 5 additions & 1 deletion python/private/pypi/parse_requirements.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 0 additions & 5 deletions python/private/pypi/pep508_env.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
20 changes: 9 additions & 11 deletions tests/pypi/extension/extension_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -1086,37 +1088,33 @@ 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,
),
],
},
},
})

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",
Expand Down
3 changes: 2 additions & 1 deletion tests/pypi/pep508/evaluate_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []

Expand Down Expand Up @@ -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)

Expand Down