From 9f756218116e33e219b80a8d110e3e5088aab3a9 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 27 Nov 2024 09:41:47 +0900 Subject: [PATCH 1/3] fix(render_pkg_aliases): correctly render when we have target_platforms 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 --- python/private/pypi/extension.bzl | 2 + python/private/pypi/render_pkg_aliases.bzl | 20 +---- python/private/pypi/whl_config_setting.bzl | 4 +- tests/pypi/extension/extension_tests.bzl | 81 +++++++++++++++++++ .../render_pkg_aliases_test.bzl | 24 ++++++ 5 files changed, 110 insertions(+), 21 deletions(-) diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index fd224d1592..edfd5809f4 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -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 @@ -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. diff --git a/python/private/pypi/render_pkg_aliases.bzl b/python/private/pypi/render_pkg_aliases.bzl index 66968c11e2..62c893d3e7 100644 --- a/python/private/pypi/render_pkg_aliases.bzl +++ b/python/private/pypi/render_pkg_aliases.bzl @@ -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 ` (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, ) ) diff --git a/python/private/pypi/whl_config_setting.bzl b/python/private/pypi/whl_config_setting.bzl index e46c7d37d7..d966206372 100644 --- a/python/private/pypi/whl_config_setting.bzl +++ b/python/private/pypi/whl_config_setting.bzl @@ -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 @@ -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, diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index 7dfd8762a7..bc5792b47d 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -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, diff --git a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl index 0ba642eca9..eaa7e28286 100644 --- a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl +++ b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl @@ -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"]}, @@ -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"], )""" From 0d41f2e3c627a9497761749dc73fe8ebab4d9363 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:18:07 +0900 Subject: [PATCH 2/3] fixup! fix(render_pkg_aliases): correctly render when we have target_platforms set --- tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl index eaa7e28286..ca1651aa1d 100644 --- a/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl +++ b/tests/pypi/render_pkg_aliases/render_pkg_aliases_test.bzl @@ -130,6 +130,7 @@ load("@rules_python//python/private/pypi:config_settings.bzl", "config_settings" config_settings( name = "config_settings", python_versions = ["3.2"], + target_platforms = ["linux_x86_64"], visibility = ["//:__subpackages__"], )""", ) @@ -228,8 +229,8 @@ _tests.append(_test_get_python_versions) def _test_get_python_versions_with_target_platforms(env): got = get_whl_flag_versions( settings = [ - whl_config_setting(repo = "foo", version = "3.3", target_platforms = ["cp33_linux_x86_64"]), - whl_config_setting(repo = "foo", version = "3.2", target_platforms = ["cp32_linux_x86_64", "cp32_osx_aarch64"]), + whl_config_setting(version = "3.3", target_platforms = ["cp33_linux_x86_64"]), + whl_config_setting(version = "3.2", target_platforms = ["cp32_linux_x86_64", "cp32_osx_aarch64"]), ], ) want = { @@ -247,7 +248,6 @@ def _test_get_python_versions_from_filenames(env): got = get_whl_flag_versions( settings = [ whl_config_setting( - repo = "foo", version = "3.3", filename = "foo-0.0.0-py3-none-" + plat + ".whl", ) @@ -285,7 +285,6 @@ def _test_get_flag_versions_from_alias_target_platforms(env): got = get_whl_flag_versions( settings = [ whl_config_setting( - repo = "foo", version = "3.3", filename = "foo-0.0.0-py3-none-" + plat + ".whl", ) @@ -294,7 +293,6 @@ def _test_get_flag_versions_from_alias_target_platforms(env): ] ] + [ whl_config_setting( - repo = "foo", version = "3.3", filename = "foo-0.0.0-py3-none-any.whl", target_platforms = [ From 1f8c96a117934fc164c370c2f9ddc6fcb5a5def6 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:27:00 +0900 Subject: [PATCH 3/3] buildifier --- tests/pypi/extension/extension_tests.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index bc5792b47d..b9427795ec 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -305,7 +305,7 @@ torch==2.4.1 ; platform_machine != 'x86_64' ], version = "3.15", ), - ] + ], }, }}) pypi.whl_libraries().contains_exactly({