Skip to content

Commit 5e75007

Browse files
authored
fix(pypi): correctly handle different package versions (bazel-contrib#3186)
After merging bazel-contrib#3058, I wanted to try creating a `universal` requirements file for our `sphinx` setup and have an integration test in that way. It seems that `alabaster` has different versions for different python versions and we were not handling this correctly. In bazel-contrib#3058 I have attempted adding a unit test for this but it escaped me that the only time where this bug manifests itself is when the `parse_requirements` is used multiple times, once per each `python_version`. With this I think it is safe to say that the bazel-contrib#2797 is fixed, because we found a bug, where we were not skipping requirements. As an added counter measure, I have added an extra check elsewhere to catch a regression. Fixes bazel-contrib#2797
1 parent e09a6f4 commit 5e75007

File tree

8 files changed

+294
-140
lines changed

8 files changed

+294
-140
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ END_UNRELEASED_TEMPLATE
123123
installations (Mac frameworks, missing dynamic libraries, and other
124124
esoteric cases, see
125125
[#3148](https://github.com/bazel-contrib/rules_python/pull/3148) for details).
126+
* (pypi) Support `requirements.txt` files that use different versions of the same
127+
package targeting different target platforms.
128+
([#2797](https://github.com/bazel-contrib/rules_python/issues/2797)).
126129

127130
{#v0-0-0-added}
128131
### Added

MODULE.bazel

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -241,21 +241,25 @@ dev_pip = use_extension(
241241
"pip",
242242
dev_dependency = True,
243243
)
244-
dev_pip.parse(
245-
download_only = True,
246-
experimental_index_url = "https://pypi.org/simple",
247-
hub_name = "dev_pip",
248-
parallel_download = False,
249-
python_version = "3.11",
250-
requirements_lock = "//docs:requirements.txt",
251-
)
252-
dev_pip.parse(
253-
download_only = True,
254-
experimental_index_url = "https://pypi.org/simple",
255-
hub_name = "dev_pip",
256-
python_version = "3.13",
257-
requirements_lock = "//docs:requirements.txt",
258-
)
244+
245+
[
246+
dev_pip.parse(
247+
download_only = True,
248+
experimental_index_url = "https://pypi.org/simple",
249+
hub_name = "dev_pip",
250+
parallel_download = False,
251+
python_version = python_version,
252+
requirements_lock = "//docs:requirements.txt",
253+
)
254+
for python_version in [
255+
"3.9",
256+
"3.10",
257+
"3.11",
258+
"3.12",
259+
"3.13",
260+
]
261+
]
262+
259263
dev_pip.parse(
260264
download_only = True,
261265
experimental_index_url = "https://pypi.org/simple",

docs/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,5 +186,8 @@ lock(
186186
"--universal",
187187
"--upgrade",
188188
],
189+
# NOTE @aignas 2025-08-17: here we select the lowest actively supported version so that the
190+
# requirements file is generated to be compatible with Python version 3.9 or greater.
191+
python_version = "3.9",
189192
visibility = ["//:__subpackages__"],
190193
)

docs/requirements.txt

Lines changed: 171 additions & 100 deletions
Large diffs are not rendered by default.

python/private/pypi/extension.bzl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,17 @@ def _create_whl_repos(
346346
))
347347

348348
whl_libraries[repo_name] = repo.args
349-
whl_map.setdefault(whl.name, {})[repo.config_setting] = repo_name
349+
mapping = whl_map.setdefault(whl.name, {})
350+
if repo.config_setting in mapping and mapping[repo.config_setting] != repo_name:
351+
fail(
352+
"attempting to override an existing repo '{}' for config setting '{}' with a new repo '{}'".format(
353+
mapping[repo.config_setting],
354+
repo.config_setting,
355+
repo_name,
356+
),
357+
)
358+
else:
359+
mapping[repo.config_setting] = repo_name
350360

351361
return struct(
352362
whl_map = whl_map,

python/private/pypi/parse_requirements.bzl

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def parse_requirements(
4242
get_index_urls = None,
4343
evaluate_markers = None,
4444
extract_url_srcs = True,
45-
logger = None):
45+
logger):
4646
"""Get the requirements with platforms that the requirements apply to.
4747
4848
Args:
@@ -63,7 +63,7 @@ def parse_requirements(
6363
requirements line.
6464
extract_url_srcs: A boolean to enable extracting URLs from requirement
6565
lines to enable using bazel downloader.
66-
logger: repo_utils.logger or None, a simple struct to log diagnostic messages.
66+
logger: repo_utils.logger, a simple struct to log diagnostic messages.
6767
6868
Returns:
6969
{type}`dict[str, list[struct]]` where the key is the distribution name and the struct
@@ -89,8 +89,7 @@ def parse_requirements(
8989
options = {}
9090
requirements = {}
9191
for file, plats in requirements_by_platform.items():
92-
if logger:
93-
logger.trace(lambda: "Using {} for {}".format(file, plats))
92+
logger.trace(lambda: "Using {} for {}".format(file, plats))
9493
contents = ctx.read(file)
9594

9695
# Parse the requirements file directly in starlark to get the information
@@ -162,11 +161,10 @@ def parse_requirements(
162161
# URL of the files to download things from. This should be important for
163162
# VCS package references.
164163
env_marker_target_platforms = evaluate_markers(ctx, reqs_with_env_markers)
165-
if logger:
166-
logger.trace(lambda: "Evaluated env markers from:\n{}\n\nTo:\n{}".format(
167-
reqs_with_env_markers,
168-
env_marker_target_platforms,
169-
))
164+
logger.trace(lambda: "Evaluated env markers from:\n{}\n\nTo:\n{}".format(
165+
reqs_with_env_markers,
166+
env_marker_target_platforms,
167+
))
170168

171169
index_urls = {}
172170
if get_index_urls:
@@ -212,8 +210,7 @@ def parse_requirements(
212210
sorted(requirements),
213211
))
214212

215-
if logger:
216-
logger.debug(lambda: "Will configure whl repos: {}".format([w.name for w in ret]))
213+
logger.debug(lambda: "Will configure whl repos: {}".format([w.name for w in ret]))
217214

218215
return ret
219216

@@ -229,7 +226,10 @@ def _package_srcs(
229226
"""A function to return sources for a particular package."""
230227
srcs = {}
231228
for r in sorted(reqs.values(), key = lambda r: r.requirement_line):
232-
target_platforms = env_marker_target_platforms.get(r.requirement_line, r.target_platforms)
229+
if ";" in r.requirement_line:
230+
target_platforms = env_marker_target_platforms.get(r.requirement_line, [])
231+
else:
232+
target_platforms = r.target_platforms
233233
extra_pip_args = tuple(r.extra_pip_args)
234234

235235
for target_platform in target_platforms:
@@ -245,8 +245,7 @@ def _package_srcs(
245245
index_urls = index_urls.get(name),
246246
logger = logger,
247247
)
248-
if logger:
249-
logger.debug(lambda: "The whl dist is: {}".format(dist.filename if dist else dist))
248+
logger.debug(lambda: "The whl dist is: {}".format(dist.filename if dist else dist))
250249

251250
if extract_url_srcs and dist:
252251
req_line = r.srcs.requirement
@@ -347,10 +346,9 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
347346

348347
if requirement.srcs.url:
349348
if not requirement.srcs.filename:
350-
if logger:
351-
logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format(
352-
requirement.srcs.url,
353-
))
349+
logger.debug(lambda: "Could not detect the filename from the URL, falling back to pip: {}".format(
350+
requirement.srcs.url,
351+
))
354352
return None
355353

356354
# Handle direct URLs in requirements
@@ -377,8 +375,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
377375
if not shas_to_use:
378376
version = requirement.srcs.version
379377
shas_to_use = index_urls.sha256s_by_version.get(version, [])
380-
if logger:
381-
logger.warn(lambda: "requirement file has been generated without hashes, will use all hashes for the given version {} that could find on the index:\n {}".format(version, shas_to_use))
378+
logger.warn(lambda: "requirement file has been generated without hashes, will use all hashes for the given version {} that could find on the index:\n {}".format(version, shas_to_use))
382379

383380
for sha256 in shas_to_use:
384381
# For now if the artifact is marked as yanked we just ignore it.
@@ -395,8 +392,7 @@ def _add_dists(*, requirement, index_urls, target_platform, logger = None):
395392
sdist = maybe_sdist
396393
continue
397394

398-
if logger:
399-
logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
395+
logger.warn(lambda: "Could not find a whl or an sdist with sha256={}".format(sha256))
400396

401397
yanked = {}
402398
for dist in whls + [sdist]:

python/private/pypi/pip_repository.bzl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
load("@bazel_skylib//lib:sets.bzl", "sets")
1818
load("//python/private:normalize_name.bzl", "normalize_name")
19-
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR")
19+
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
2020
load("//python/private:text_util.bzl", "render")
2121
load(":evaluate_markers.bzl", "evaluate_markers_py", EVALUATE_MARKERS_SRCS = "SRCS")
2222
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
@@ -71,6 +71,7 @@ exports_files(["requirements.bzl"])
7171
"""
7272

7373
def _pip_repository_impl(rctx):
74+
logger = repo_utils.logger(rctx)
7475
requirements_by_platform = parse_requirements(
7576
rctx,
7677
requirements_by_platform = requirements_files_by_platform(
@@ -100,6 +101,7 @@ def _pip_repository_impl(rctx):
100101
srcs = rctx.attr._evaluate_markers_srcs,
101102
),
102103
extract_url_srcs = False,
104+
logger = logger,
103105
)
104106
selected_requirements = {}
105107
options = None

tests/pypi/parse_requirements/parse_requirements_tests.bzl

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ def _test_get_index_urls_different_versions(env):
639639
platforms = {
640640
"cp310_linux_x86_64": struct(
641641
env = pep508_env(
642-
python_version = "3.9.0",
642+
python_version = "3.10.0",
643643
os = "linux",
644644
arch = "x86_64",
645645
),
@@ -686,6 +686,7 @@ def _test_get_index_urls_different_versions(env):
686686
),
687687
},
688688
),
689+
debug = True,
689690
)
690691

691692
env.expect.that_collection(got).contains_exactly([
@@ -720,6 +721,70 @@ def _test_get_index_urls_different_versions(env):
720721

721722
_tests.append(_test_get_index_urls_different_versions)
722723

724+
def _test_get_index_urls_single_py_version(env):
725+
got = parse_requirements(
726+
requirements_by_platform = {
727+
"requirements_multi_version": [
728+
"cp310_linux_x86_64",
729+
],
730+
},
731+
platforms = {
732+
"cp310_linux_x86_64": struct(
733+
env = pep508_env(
734+
python_version = "3.10.0",
735+
os = "linux",
736+
arch = "x86_64",
737+
),
738+
whl_abi_tags = ["none"],
739+
whl_platform_tags = ["any"],
740+
),
741+
},
742+
get_index_urls = lambda _, __: {
743+
"foo": struct(
744+
sdists = {},
745+
whls = {
746+
"deadb11f": struct(
747+
url = "super2",
748+
sha256 = "deadb11f",
749+
filename = "foo-0.0.2-py3-none-any.whl",
750+
yanked = False,
751+
),
752+
},
753+
),
754+
},
755+
evaluate_markers = lambda _, requirements: evaluate_markers(
756+
requirements = requirements,
757+
platforms = {
758+
"cp310_linux_x86_64": struct(
759+
env = {"python_full_version": "3.10.0"},
760+
),
761+
},
762+
),
763+
debug = True,
764+
)
765+
766+
env.expect.that_collection(got).contains_exactly([
767+
struct(
768+
is_exposed = True,
769+
is_multiple_versions = True,
770+
name = "foo",
771+
srcs = [
772+
struct(
773+
distribution = "foo",
774+
extra_pip_args = [],
775+
filename = "foo-0.0.2-py3-none-any.whl",
776+
requirement_line = "foo==0.0.2",
777+
sha256 = "deadb11f",
778+
target_platforms = ["cp310_linux_x86_64"],
779+
url = "super2",
780+
yanked = False,
781+
),
782+
],
783+
),
784+
])
785+
786+
_tests.append(_test_get_index_urls_single_py_version)
787+
723788
def parse_requirements_test_suite(name):
724789
"""Create the test suite.
725790

0 commit comments

Comments
 (0)