fix(render_pkg_aliases): correctly render when we have target_platforms set (bazel-contrib#2447)

aignas · web-flow · commit 4e60d07ff6ee · 2024-11-27T14:37:22.000-08:00
It seems that during bazel-contrib#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 bazel-contrib#2446
diff --git 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
@@ -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,
             )
         )
diff --git 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
@@ -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
@@ -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"],
 )"""
@@ -106,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__"],
 )""",
     )
@@ -204,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 = {
@@ -223,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",
             )
@@ -261,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",
             )
@@ -270,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 = [
