Skip to content

Commit

Permalink
fix(render_pkg_aliases): correctly render when we have target_platfor…
Browse files Browse the repository at this point in the history
…ms set

It seems that during #2424 I broke the rendering of aliases for the
cases when the target platform is set. This means that the feature for
multiplatform whls when `experimental_index_url` has never worked even
though it was advertised. This ensures that the rendering is happening
correctly and adds extra missing tests.

Whilst at it:
- add an extra test for `pip.parse` handling of env markers that I added
  to ensure that the error is not in the module extension.
- Cleanup unused code - error message constant and the repo arg in
  `whl_config_setting`.

Fixes #2446
  • Loading branch information
aignas committed Nov 27, 2024
1 parent 077eec6 commit 9f75621
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 21 deletions.
2 changes: 2 additions & 0 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def _create_whl_repos(
pip_attr,
whl_overrides,
simpleapi_cache,
evaluate_markers = evaluate_markers,
available_interpreters = INTERPRETER_LABELS,
simpleapi_download = simpleapi_download):
"""create all of the whl repositories
Expand All @@ -78,6 +79,7 @@ def _create_whl_repos(
whl_overrides: {type}`dict[str, struct]` - per-wheel overrides.
simpleapi_cache: {type}`dict` - an opaque dictionary used for caching the results from calling
SimpleAPI evaluating all of the tag class invocations {bzl:obj}`pip.parse`.
evaluate_markers: the function to use to evaluate markers.
simpleapi_download: Used for testing overrides
available_interpreters: {type}`dict[str, Label]` The dictionary of available
interpreters that have been registered using the `python` bzlmod extension.
Expand Down
20 changes: 2 additions & 18 deletions python/private/pypi/render_pkg_aliases.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -44,33 +44,17 @@ If the value is missing, then the "default" Python version is being used,
which has a "null" version value and will not match version constraints.
"""

NO_MATCH_ERROR_MESSAGE_TEMPLATE_V2 = """\
No matching wheel for current configuration's Python version.
The current build configuration's Python version doesn't match any of the Python
wheels available for this wheel. This wheel supports the following Python
configuration settings:
{config_settings}
To determine the current configuration's Python version, run:
`bazel config <config id>` (shown further below)
and look for
{rules_python}//python/config_settings:python_version
If the value is missing, then the "default" Python version is being used,
which has a "null" version value and will not match version constraints.
"""

def _repr_dict(*, value_repr = repr, **kwargs):
return {k: value_repr(v) for k, v in kwargs.items() if v}

def _repr_config_setting(alias):
if alias.filename:
if alias.filename or alias.target_platforms:
return render.call(
"whl_config_setting",
**_repr_dict(
filename = alias.filename,
target_platforms = alias.target_platforms,
config_setting = alias.config_setting,
version = alias.version,
)
)
Expand Down
4 changes: 1 addition & 3 deletions python/private/pypi/whl_config_setting.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@

"A small function to create an alias for a whl distribution"

def whl_config_setting(*, repo = None, version = None, config_setting = None, filename = None, target_platforms = None):
def whl_config_setting(*, version = None, config_setting = None, filename = None, target_platforms = None):
"""The bzl_packages value used by by the render_pkg_aliases function.
This contains the minimum amount of information required to generate correct
aliases in a hub repository.
Args:
repo: str, the repo of where to find the things to be aliased.
version: optional(str), the version of the python toolchain that this
whl alias is for. If not set, then non-version aware aliases will be
constructed. This is mainly used for better error messages when there
Expand All @@ -43,7 +42,6 @@ def whl_config_setting(*, repo = None, version = None, config_setting = None, fi
return struct(
config_setting = config_setting,
filename = filename,
repo = repo,
# Make the struct hashable
target_platforms = tuple(target_platforms) if target_platforms else None,
version = version,
Expand Down
81 changes: 81 additions & 0 deletions tests/pypi/extension/extension_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,87 @@ def _test_simple_multiple_requirements(env):

_tests.append(_test_simple_multiple_requirements)

def _test_simple_with_markers(env):
pypi = _parse_modules(
env,
module_ctx = _mock_mctx(
_mod(
name = "rules_python",
parse = [
_parse(
hub_name = "pypi",
python_version = "3.15",
requirements_lock = "universal.txt",
),
],
),
read = lambda x: {
"universal.txt": """\
torch==2.4.1+cpu ; platform_machine == 'x86_64'
torch==2.4.1 ; platform_machine != 'x86_64'
""",
}[x],
),
available_interpreters = {
"python_3_15_host": "unit_test_interpreter_target",
},
evaluate_markers = lambda _, requirements, **__: {
key: [
platform
for platform in platforms
if ("x86_64" in platform and "platform_machine ==" in key) or ("x86_64" not in platform and "platform_machine !=" in key)
]
for key, platforms in requirements.items()
},
)

pypi.is_reproducible().equals(True)
pypi.exposed_packages().contains_exactly({"pypi": ["torch"]})
pypi.hub_group_map().contains_exactly({"pypi": {}})
pypi.hub_whl_map().contains_exactly({"pypi": {
"torch": {
"pypi_315_torch_linux_aarch64_linux_arm_linux_ppc_linux_s390x_osx_aarch64": [
whl_config_setting(
target_platforms = [
"cp315_linux_aarch64",
"cp315_linux_arm",
"cp315_linux_ppc",
"cp315_linux_s390x",
"cp315_osx_aarch64",
],
version = "3.15",
),
],
"pypi_315_torch_linux_x86_64_osx_x86_64_windows_x86_64": [
whl_config_setting(
target_platforms = [
"cp315_linux_x86_64",
"cp315_osx_x86_64",
"cp315_windows_x86_64",
],
version = "3.15",
),
]
},
}})
pypi.whl_libraries().contains_exactly({
"pypi_315_torch_linux_aarch64_linux_arm_linux_ppc_linux_s390x_osx_aarch64": {
"dep_template": "@pypi//{name}:{target}",
"python_interpreter_target": "unit_test_interpreter_target",
"repo": "pypi_315",
"requirement": "torch==2.4.1 ; platform_machine != 'x86_64'",
},
"pypi_315_torch_linux_x86_64_osx_x86_64_windows_x86_64": {
"dep_template": "@pypi//{name}:{target}",
"python_interpreter_target": "unit_test_interpreter_target",
"repo": "pypi_315",
"requirement": "torch==2.4.1+cpu ; platform_machine == 'x86_64'",
},
})
pypi.whl_mods().contains_exactly({})

_tests.append(_test_simple_with_markers)

def _test_download_only_multiple(env):
pypi = _parse_modules(
env,
Expand Down
24 changes: 24 additions & 0 deletions tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,24 @@ def _test_bzlmod_aliases(env):
version = "3.2",
config_setting = "//:my_config_setting",
): "pypi_32_bar_baz",
whl_config_setting(
version = "3.2",
config_setting = "//:my_config_setting",
target_platforms = [
"cp32_linux_x86_64",
],
): "pypi_32_bar_baz_linux_x86_64",
whl_config_setting(
version = "3.2",
filename = "foo-0.0.0-py3-none-any.whl",
): "filename_repo",
whl_config_setting(
version = "3.2",
filename = "foo-0.0.0-py3-none-any.whl",
target_platforms = [
"cp32_linux_x86_64",
],
): "filename_repo_linux_x86_64",
},
},
extra_hub_aliases = {"bar_baz": ["foo"]},
Expand All @@ -91,10 +105,20 @@ pkg_aliases(
name = "bar_baz",
actual = {
"//:my_config_setting": "pypi_32_bar_baz",
whl_config_setting(
target_platforms = ("cp32_linux_x86_64",),
config_setting = "//:my_config_setting",
version = "3.2",
): "pypi_32_bar_baz_linux_x86_64",
whl_config_setting(
filename = "foo-0.0.0-py3-none-any.whl",
version = "3.2",
): "filename_repo",
whl_config_setting(
filename = "foo-0.0.0-py3-none-any.whl",
target_platforms = ("cp32_linux_x86_64",),
version = "3.2",
): "filename_repo_linux_x86_64",
},
extra_aliases = ["foo"],
)"""
Expand Down

0 comments on commit 9f75621

Please sign in to comment.