From df6eb5d76ddf6fe2073f3088956765c105fd4951 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:21:40 +0900 Subject: [PATCH 1/9] add a test --- tests/pypi/hub_builder/hub_builder_tests.bzl | 39 +++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index 6d061f4d56..7b95e0905f 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -566,6 +566,7 @@ def _test_torch_experimental_index_url(env): for (os, cpu), whl_platform_tags in { ("linux", "x86_64"): ["linux_x86_64", "manylinux_*_x86_64"], ("linux", "aarch64"): ["linux_aarch64", "manylinux_*_aarch64"], + ("osx", "x86_64"): ["macosx_*_x86_64"], ("osx", "aarch64"): ["macosx_*_arm64"], ("windows", "x86_64"): ["win_amd64"], ("windows", "aarch64"): ["win_arm64"], # this should be ignored @@ -800,6 +801,20 @@ def _test_simple_get_index(env): got_simpleapi_download_args.extend(args) got_simpleapi_download_kwargs.update(kwargs) return { + "plat_pkg": struct( + whls = { + "deadb44f": struct( + yanked = False, + filename = "plat-pkg-0.0.4-py3-none-linux_x86_64.whl", + sha256 = "deadb44f", + url = "example2.org/index/plat_pkg/", + ), + }, + sdists = {}, + sha256s_by_version = { + "0.0.4": ["deadb44f"], + }, + ), "simple": struct( whls = { "deadb00f": struct( @@ -850,6 +865,7 @@ some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl \ --hash=sha256:deadbaaf direct_without_sha==0.0.1 @ example-direct.org/direct_without_sha-0.0.1-py3-none-any.whl some_other_pkg==0.0.1 +plat_pkg==0.0.4 pip_fallback==0.0.1 direct_sdist_without_sha @ some-archive/any-name.tar.gz git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef @@ -873,6 +889,7 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef "direct_without_sha", "git_dep", "pip_fallback", + "plat_pkg", "simple", "some_other_pkg", "some_pkg", @@ -921,6 +938,17 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef ), ], }, + "plat_pkg": { + "pypi_315_plat_py3_none_linux_x86_64_deadb44f": [ + whl_config_setting( + target_platforms = [ + "cp315_linux_x86_64", + "cp315_linux_x86_64_freethreaded", + ], + version = "3.15", + ), + ], + }, "simple": { "pypi_315_simple_py3_none_any_deadb00f": [ whl_config_setting( @@ -998,6 +1026,15 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef "python_interpreter_target": "unit_test_interpreter_target", "requirement": "pip_fallback==0.0.1", }, + "pypi_315_plat_py3_none_linux_x86_64_deadb44f": { + "config_load": "@pypi//:config.bzl", + "dep_template": "@pypi//{name}:{target}", + "filename": "plat-pkg-0.0.4-py3-none-linux_x86_64.whl", + "python_interpreter_target": "unit_test_interpreter_target", + "requirement": "plat_pkg==0.0.4", + "sha256": "deadb44f", + "urls": ["example2.org/index/plat_pkg/"], + }, "pypi_315_simple_py3_none_any_deadb00f": { "config_load": "@pypi//:config.bzl", "dep_template": "@pypi//{name}:{target}", @@ -1036,7 +1073,7 @@ git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef index_url = "pypi.org", index_url_overrides = {}, netrc = None, - sources = ["simple", "pip_fallback", "some_other_pkg"], + sources = ["simple", "plat_pkg", "pip_fallback", "some_other_pkg"], ), "cache": {}, "parallel_download": False, From 13247661281d303e9090c834bc72fc4b35ee1c76 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:25:31 +0900 Subject: [PATCH 2/9] remove unused marker functions --- tests/pypi/hub_builder/hub_builder_tests.bzl | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index 7b95e0905f..1aa4607c45 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -566,7 +566,6 @@ def _test_torch_experimental_index_url(env): for (os, cpu), whl_platform_tags in { ("linux", "x86_64"): ["linux_x86_64", "manylinux_*_x86_64"], ("linux", "aarch64"): ["linux_aarch64", "manylinux_*_aarch64"], - ("osx", "x86_64"): ["macosx_*_x86_64"], ("osx", "aarch64"): ["macosx_*_arm64"], ("windows", "x86_64"): ["win_amd64"], ("windows", "aarch64"): ["win_arm64"], # this should be ignored @@ -577,15 +576,6 @@ def _test_torch_experimental_index_url(env): "python_3_12_host": "unit_test_interpreter_target", }, minor_mapping = {"3.12": "3.12.19"}, - evaluate_markers_fn = lambda _, requirements, **__: { - # todo once 2692 is merged, this is going to be easier to test. - 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() - }, simpleapi_download_fn = mocksimpleapi_download, ) builder.pip_parse( @@ -1085,14 +1075,6 @@ _tests.append(_test_simple_get_index) def _test_optimum_sys_platform_extra(env): builder = hub_builder( env, - evaluate_markers_fn = lambda _, requirements, **__: { - key: [ - platform - for platform in platforms - if ("darwin" in key and "osx" in platform) or ("linux" in key and "linux" in platform) - ] - for key, platforms in requirements.items() - }, ) builder.pip_parse( _mock_mctx( From a82f384ac370757c4aa191fd1b91c7a9e7ec2255 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:26:37 +0900 Subject: [PATCH 3/9] fix the code --- python/private/pypi/parse_requirements.bzl | 24 +++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index 7d210abbaa..b0a67107b5 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -234,7 +234,7 @@ def _package_srcs( platforms.keys(), )) - dist = _add_dists( + dist, can_fallback = _add_dists( requirement = r, target_platform = platforms.get(target_platform), index_urls = index_urls.get(name), @@ -244,7 +244,7 @@ def _package_srcs( if extract_url_srcs and dist: req_line = r.srcs.requirement - else: + elif can_fallback: dist = struct( url = "", filename = "", @@ -252,6 +252,8 @@ def _package_srcs( yanked = False, ) req_line = r.srcs.requirement_line + else: + continue key = ( dist.filename, @@ -337,6 +339,14 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): index_urls: The result of simpleapi_download. target_platform: The target_platform information. logger: A logger for printing diagnostic info. + + Returns: + (dist, can_fallback_to_pip): a struct with distribution details and how to fetch + it and a boolean flag to tell the other layers if we should add an entry to + fallback for pip if there are no supported whls found - if there is an sdist, we + can attempt the fallback, otherwise better to not, because the pip command will + fail and the error message will be confusing. What is more that would lead to + breakage of the bazel query. """ if requirement.srcs.url: @@ -344,7 +354,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format( requirement.srcs.url, )) - return None + return None, True # Handle direct URLs in requirements dist = struct( @@ -355,12 +365,12 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): ) if dist.filename.endswith(".whl"): - return dist + return dist, False else: - return dist + return dist, False if not index_urls: - return None + return None, True whls = [] sdist = None @@ -413,4 +423,4 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): whl_abi_tags = target_platform.whl_abi_tags, whl_platform_tags = target_platform.whl_platform_tags, logger = logger, - ) or sdist + ) or sdist, sdist != None From 1a0950d5df866582b2d3d47a6c97df457351e2d1 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:28:50 +0900 Subject: [PATCH 4/9] Revert "fix the code" This reverts commit a82f384ac370757c4aa191fd1b91c7a9e7ec2255. --- python/private/pypi/parse_requirements.bzl | 24 +++++++--------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index b0a67107b5..7d210abbaa 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -234,7 +234,7 @@ def _package_srcs( platforms.keys(), )) - dist, can_fallback = _add_dists( + dist = _add_dists( requirement = r, target_platform = platforms.get(target_platform), index_urls = index_urls.get(name), @@ -244,7 +244,7 @@ def _package_srcs( if extract_url_srcs and dist: req_line = r.srcs.requirement - elif can_fallback: + else: dist = struct( url = "", filename = "", @@ -252,8 +252,6 @@ def _package_srcs( yanked = False, ) req_line = r.srcs.requirement_line - else: - continue key = ( dist.filename, @@ -339,14 +337,6 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): index_urls: The result of simpleapi_download. target_platform: The target_platform information. logger: A logger for printing diagnostic info. - - Returns: - (dist, can_fallback_to_pip): a struct with distribution details and how to fetch - it and a boolean flag to tell the other layers if we should add an entry to - fallback for pip if there are no supported whls found - if there is an sdist, we - can attempt the fallback, otherwise better to not, because the pip command will - fail and the error message will be confusing. What is more that would lead to - breakage of the bazel query. """ if requirement.srcs.url: @@ -354,7 +344,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format( requirement.srcs.url, )) - return None, True + return None # Handle direct URLs in requirements dist = struct( @@ -365,12 +355,12 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): ) if dist.filename.endswith(".whl"): - return dist, False + return dist else: - return dist, False + return dist if not index_urls: - return None, True + return None whls = [] sdist = None @@ -423,4 +413,4 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): whl_abi_tags = target_platform.whl_abi_tags, whl_platform_tags = target_platform.whl_platform_tags, logger = logger, - ) or sdist, sdist != None + ) or sdist From 250ac177d923f22e956c5265caec635f5abe0859 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:31:01 +0900 Subject: [PATCH 5/9] add a better test --- tests/pypi/hub_builder/hub_builder_tests.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index 1aa4607c45..83594037d1 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -566,6 +566,8 @@ def _test_torch_experimental_index_url(env): for (os, cpu), whl_platform_tags in { ("linux", "x86_64"): ["linux_x86_64", "manylinux_*_x86_64"], ("linux", "aarch64"): ["linux_aarch64", "manylinux_*_aarch64"], + # this should be ignored as well because there is no sdist + ("osx", "x86_64"): ["macosx_*_x86_64"], ("osx", "aarch64"): ["macosx_*_arm64"], ("windows", "x86_64"): ["win_amd64"], ("windows", "aarch64"): ["win_arm64"], # this should be ignored @@ -612,7 +614,6 @@ torch==2.4.1+cpu ; platform_machine == 'x86_64' \ _parse( hub_name = "pypi", python_version = "3.12", - download_only = True, experimental_index_url = "https://torch.index", requirements_lock = "universal.txt", ), From 799ae4644cbf044a5e69d302fc2687acc1ded118 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:31:04 +0900 Subject: [PATCH 6/9] Reapply "fix the code" This reverts commit 1a0950d5df866582b2d3d47a6c97df457351e2d1. --- python/private/pypi/parse_requirements.bzl | 24 +++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index 7d210abbaa..b0a67107b5 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -234,7 +234,7 @@ def _package_srcs( platforms.keys(), )) - dist = _add_dists( + dist, can_fallback = _add_dists( requirement = r, target_platform = platforms.get(target_platform), index_urls = index_urls.get(name), @@ -244,7 +244,7 @@ def _package_srcs( if extract_url_srcs and dist: req_line = r.srcs.requirement - else: + elif can_fallback: dist = struct( url = "", filename = "", @@ -252,6 +252,8 @@ def _package_srcs( yanked = False, ) req_line = r.srcs.requirement_line + else: + continue key = ( dist.filename, @@ -337,6 +339,14 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): index_urls: The result of simpleapi_download. target_platform: The target_platform information. logger: A logger for printing diagnostic info. + + Returns: + (dist, can_fallback_to_pip): a struct with distribution details and how to fetch + it and a boolean flag to tell the other layers if we should add an entry to + fallback for pip if there are no supported whls found - if there is an sdist, we + can attempt the fallback, otherwise better to not, because the pip command will + fail and the error message will be confusing. What is more that would lead to + breakage of the bazel query. """ if requirement.srcs.url: @@ -344,7 +354,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format( requirement.srcs.url, )) - return None + return None, True # Handle direct URLs in requirements dist = struct( @@ -355,12 +365,12 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): ) if dist.filename.endswith(".whl"): - return dist + return dist, False else: - return dist + return dist, False if not index_urls: - return None + return None, True whls = [] sdist = None @@ -413,4 +423,4 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): whl_abi_tags = target_platform.whl_abi_tags, whl_platform_tags = target_platform.whl_platform_tags, logger = logger, - ) or sdist + ) or sdist, sdist != None From 88edf3ddd58a746a39748b952191c712cbc8f565 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 19:36:40 +0900 Subject: [PATCH 7/9] clarify the comment --- tests/pypi/hub_builder/hub_builder_tests.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index 83594037d1..a0ab919d68 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -566,7 +566,8 @@ def _test_torch_experimental_index_url(env): for (os, cpu), whl_platform_tags in { ("linux", "x86_64"): ["linux_x86_64", "manylinux_*_x86_64"], ("linux", "aarch64"): ["linux_aarch64", "manylinux_*_aarch64"], - # this should be ignored as well because there is no sdist + # this should be ignored as well because there is no sdist and no whls + # for intel Macs ("osx", "x86_64"): ["macosx_*_x86_64"], ("osx", "aarch64"): ["macosx_*_arm64"], ("windows", "x86_64"): ["win_amd64"], From 13ae030fd03a6fbea8b92341b511569568c41fd9 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 20:17:00 +0900 Subject: [PATCH 8/9] add a comment --- python/private/pypi/parse_requirements.bzl | 37 +++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index b0a67107b5..59683597cd 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -188,19 +188,35 @@ def parse_requirements( for p in r.target_platforms: requirement_target_platforms[p] = None + package_srcs = _package_srcs( + name = name, + reqs = reqs, + index_urls = index_urls, + platforms = platforms, + extract_url_srcs = extract_url_srcs, + logger = logger, + ) + + # FIXME @aignas 2025-11-24: we can get the list of target platforms here + # + # However it is likely that we may stop exposing packages like torch in here + # which do not have wheels for all osx platforms. + # + # If users specify the target platforms accurately, then it is a different + # (better) story, but we may not be able to guarantee this + # + # target_platforms = [ + # p + # for dist in package_srcs + # for p in dist.target_platforms + # ] + item = struct( # Return normalized names name = normalize_name(name), is_exposed = len(requirement_target_platforms) == len(requirements), is_multiple_versions = len(reqs.values()) > 1, - srcs = _package_srcs( - name = name, - reqs = reqs, - index_urls = index_urls, - platforms = platforms, - extract_url_srcs = extract_url_srcs, - logger = logger, - ), + srcs = package_srcs, ) ret.append(item) if not item.is_exposed and logger: @@ -364,10 +380,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): yanked = False, ) - if dist.filename.endswith(".whl"): - return dist, False - else: - return dist, False + return dist, False if not index_urls: return None, True From 31ec6bb815cff1e7d6bb66dd049f34451243ddcf Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 24 Nov 2025 20:39:45 +0900 Subject: [PATCH 9/9] add a better explanation --- python/private/pypi/parse_requirements.bzl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index 59683597cd..5c05c753fd 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -426,7 +426,14 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None): if not target_platform: # The pipstar platforms are undefined here, so we cannot do any matching - return sdist + return sdist, True + + if not whls and not sdist: + # If there are no suitable wheels to handle for now allow fallback to pip, it + # may be a little bit more helpful when debugging? Most likely something is + # going a bit wrong here, should we raise an error because the sha256 have most + # likely mismatched? We are already printing a warning above. + return None, True # Select a single wheel that can work on the target_platform return select_whl(