From 1e4e8bf2bce61082c2ac51204f2b0be5e4e96689 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sun, 25 May 2025 00:05:09 +0900 Subject: [PATCH 1/3] fix: correctly aggregate the whl results from multiple req files --- python/private/pypi/parse_requirements.bzl | 18 ++++-- .../parse_requirements_tests.bzl | 64 ++++++++++++++++++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index bd2981efc0..e4a8b90acb 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -223,7 +223,7 @@ def _package_srcs( env_marker_target_platforms, extract_url_srcs): """A function to return sources for a particular package.""" - srcs = [] + srcs = {} for r in sorted(reqs.values(), key = lambda r: r.requirement_line): whls, sdist = _add_dists( requirement = r, @@ -249,21 +249,31 @@ def _package_srcs( )] req_line = r.srcs.requirement_line + extra_pip_args = tuple(r.extra_pip_args) for dist in all_dists: - srcs.append( + key = ( + dist.filename, + req_line, + extra_pip_args, + ) + entry = srcs.setdefault( + key, struct( distribution = name, extra_pip_args = r.extra_pip_args, requirement_line = req_line, - target_platforms = target_platforms, + target_platforms = [], filename = dist.filename, sha256 = dist.sha256, url = dist.url, yanked = dist.yanked, ), ) + for p in target_platforms: + if p not in entry.target_platforms: + entry.target_platforms.append(p) - return srcs + return srcs.values() def select_requirement(requirements, *, platform): """A simple function to get a requirement for a particular platform. diff --git a/tests/pypi/parse_requirements/parse_requirements_tests.bzl b/tests/pypi/parse_requirements/parse_requirements_tests.bzl index 926a7e0c50..f11dd6daf1 100644 --- a/tests/pypi/parse_requirements/parse_requirements_tests.bzl +++ b/tests/pypi/parse_requirements/parse_requirements_tests.bzl @@ -67,7 +67,7 @@ foo==0.0.4 @ https://example.org/foo-0.0.4.whl foo==0.0.5 @ https://example.org/foo-0.0.5.whl --hash=sha256:deadbeef """, "requirements_osx": """\ -foo==0.0.3 --hash=sha256:deadbaaf +foo==0.0.3 --hash=sha256:deadbaaf --hash=sha256:deadb11f """, "requirements_osx_download_only": """\ --platform=macosx_10_9_arm64 @@ -515,6 +515,68 @@ def _test_git_sources(env): _tests.append(_test_git_sources) +def _test_overlapping_shas_with_index_results(env): + got = parse_requirements( + ctx = _mock_ctx(), + requirements_by_platform = { + "requirements_linux": ["cp39_linux_x86_64"], + "requirements_osx": ["cp39_osx_x86_64"], + }, + get_index_urls = lambda _, __: { + "foo": struct( + sdists = { + }, + whls = { + "deadb11f": struct( + url = "super2", + sha256 = "deadb11f", + filename = "foo-0.0.1-py3-none-macosx_14_0_x86_64.whl", + yanked = False, + ), + "deadbaaf": struct( + url = "super2", + sha256 = "deadbaaf", + filename = "foo-0.0.1-py3-none-any.whl", + yanked = False, + ), + }, + ), + }, + ) + + env.expect.that_collection(got).contains_exactly([ + struct( + name = "foo", + is_exposed = True, + # TODO @aignas 2025-05-25: how do we rename this? + is_multiple_versions = True, + srcs = [ + struct( + distribution = "foo", + extra_pip_args = [], + filename = "foo-0.0.1-py3-none-any.whl", + requirement_line = "foo==0.0.3", + sha256 = "deadbaaf", + target_platforms = ["cp39_linux_x86_64", "cp39_osx_x86_64"], + url = "super2", + yanked = False, + ), + struct( + distribution = "foo", + extra_pip_args = [], + filename = "foo-0.0.1-py3-none-macosx_14_0_x86_64.whl", + requirement_line = "foo==0.0.3", + sha256 = "deadb11f", + target_platforms = ["cp39_osx_x86_64"], + url = "super2", + yanked = False, + ), + ], + ), + ]) + +_tests.append(_test_overlapping_shas_with_index_results) + def parse_requirements_test_suite(name): """Create the test suite. From 026d17f1dee47826e99e06a60cd0feb4c65378b3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 30 May 2025 18:08:54 +0900 Subject: [PATCH 2/3] doc: changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a6bdf0a96..57eb154f55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,8 @@ END_UNRELEASED_TEMPLATE [#2363](https://github.com/bazel-contrib/rules_python/issues/2363). * (pypi) `whl_library` now infers file names from its `urls` attribute correctly. * (py_test, py_binary) Allow external files to be used for main +* (pypi) Correctly aggregate the sources when the hashes specified in the lockfile differ + by platform even though the same version is used. Fixes [#2648](https://github.com/bazel-contrib/rules_python/issues/2648). {#v0-0-0-added} ### Added From 5678418a387b320272c97ca0821920bcd98dce68 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 30 May 2025 18:13:06 +0900 Subject: [PATCH 3/3] test: add sdist test --- .../parse_requirements_tests.bzl | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/pypi/parse_requirements/parse_requirements_tests.bzl b/tests/pypi/parse_requirements/parse_requirements_tests.bzl index f11dd6daf1..82fdd0a051 100644 --- a/tests/pypi/parse_requirements/parse_requirements_tests.bzl +++ b/tests/pypi/parse_requirements/parse_requirements_tests.bzl @@ -38,7 +38,7 @@ foo[extra]==0.0.1 \ foo @ git+https://github.com/org/foo.git@deadbeef """, "requirements_linux": """\ -foo==0.0.3 --hash=sha256:deadbaaf +foo==0.0.3 --hash=sha256:deadbaaf --hash=sha256:5d15t """, # download_only = True "requirements_linux_download_only": """\ @@ -67,7 +67,7 @@ foo==0.0.4 @ https://example.org/foo-0.0.4.whl foo==0.0.5 @ https://example.org/foo-0.0.5.whl --hash=sha256:deadbeef """, "requirements_osx": """\ -foo==0.0.3 --hash=sha256:deadbaaf --hash=sha256:deadb11f +foo==0.0.3 --hash=sha256:deadbaaf --hash=sha256:deadb11f --hash=sha256:5d15t """, "requirements_osx_download_only": """\ --platform=macosx_10_9_arm64 @@ -251,7 +251,7 @@ def _test_multi_os(env): struct( distribution = "foo", extra_pip_args = [], - requirement_line = "foo==0.0.3 --hash=sha256:deadbaaf", + requirement_line = "foo==0.0.3 --hash=sha256:deadbaaf --hash=sha256:5d15t", target_platforms = ["linux_x86_64"], url = "", filename = "", @@ -525,6 +525,12 @@ def _test_overlapping_shas_with_index_results(env): get_index_urls = lambda _, __: { "foo": struct( sdists = { + "5d15t": struct( + url = "sdist", + sha256 = "5d15t", + filename = "foo-0.0.1.tar.gz", + yanked = False, + ), }, whls = { "deadb11f": struct( @@ -561,6 +567,16 @@ def _test_overlapping_shas_with_index_results(env): url = "super2", yanked = False, ), + struct( + distribution = "foo", + extra_pip_args = [], + filename = "foo-0.0.1.tar.gz", + requirement_line = "foo==0.0.3", + sha256 = "5d15t", + target_platforms = ["cp39_linux_x86_64", "cp39_osx_x86_64"], + url = "sdist", + yanked = False, + ), struct( distribution = "foo", extra_pip_args = [],