Skip to content

Commit e7d5362

Browse files
aignasrickeylev
andcommitted
fix(pipstar): correctly handle complex self deps (#3527)
It seems that with the `pipstar` port there was a typo and the initial tests that we had for Python were insufficient to catch such a regression. The second if statement where we loop through packages again had a `req` instead of `req_` in the `if` statement and the test coverage was not sufficient. I have abstracted the if statement into a function to easier spot such issues and added an extra test to ensure that a regression would be actually caught. With this the Starlark test suite is now officially more robust than the Python version. Fixes #3524 --------- Co-authored-by: Richard Levasseur <[email protected]> (cherry picked from commit c52aeaa)
1 parent 060bfeb commit e7d5362

File tree

3 files changed

+53
-14
lines changed

3 files changed

+53
-14
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ BEGIN_UNRELEASED_TEMPLATE
4747
END_UNRELEASED_TEMPLATE
4848
-->
4949

50+
{#v1-8-1}
51+
## [1.8.1] - 2026-01-20
52+
53+
{#v1-8-1-fixed}
54+
### Fixed
55+
* (pipstar) Extra resolution that refers back to the package being resolved works again.
56+
Fixes [#3524](https://github.com/bazel-contrib/rules_python/issues/3524).
57+
5058
{#v1-8-0}
5159
## [1.8.0] - 2025-12-19
5260

python/private/pypi/pep508_deps.bzl

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ def _resolve_extras(self_name, reqs, extras):
115115
# extras The empty string in the set is just a way to make the handling
116116
# of no extras and a single extra easier and having a set of {"", "foo"}
117117
# is equivalent to having {"foo"}.
118-
extras = extras or [""]
118+
#
119+
# Use a dict as a set here to simplify operations.
120+
extras = {x: None for x in (extras or [""])}
119121

120122
self_reqs = []
121123
for req in reqs:
@@ -128,26 +130,36 @@ def _resolve_extras(self_name, reqs, extras):
128130
# easy to handle, lets do it.
129131
#
130132
# TODO @aignas 2023-12-08: add a test
131-
extras = extras + req.extras
133+
extras = extras | {x: None for x in req.extras}
132134
else:
133135
# process these in a separate loop
134136
self_reqs.append(req)
135137

136-
# A double loop is not strictly optimal, but always correct without recursion
137-
for req in self_reqs:
138-
if [True for extra in extras if evaluate(req.marker, env = {"extra": extra})]:
139-
extras = extras + req.extras
140-
else:
141-
continue
138+
for _ in range(10000):
139+
# handles packages with up to 10000 recursive extras
140+
new_extras = {}
141+
for req in self_reqs:
142+
if _evaluate_any(req, extras):
143+
new_extras.update({x: None for x in req.extras})
144+
else:
145+
continue
142146

143-
# Iterate through all packages to ensure that we include all of the extras from previously
144-
# visited packages.
145-
for req_ in self_reqs:
146-
if [True for extra in extras if evaluate(req.marker, env = {"extra": extra})]:
147-
extras = extras + req_.extras
147+
num_extras_before = len(extras)
148+
extras = extras | new_extras
149+
num_extras_after = len(new_extras)
150+
151+
if num_extras_before == num_extras_after:
152+
break
148153

149154
# Poor mans set
150-
return sorted({x: None for x in extras})
155+
return sorted(extras)
156+
157+
def _evaluate_any(req, extras):
158+
for extra in extras:
159+
if evaluate(req.marker, env = {"extra": extra}):
160+
return True
161+
162+
return False
151163

152164
def _add_reqs(deps, deps_select, dep, reqs, *, extras):
153165
for req in reqs:

tests/pypi/pep508/deps_tests.bzl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ def test_self_dependencies_can_come_in_any_order(env):
9090
"baz; extra == 'feat'",
9191
"foo[feat2]; extra == 'all'",
9292
"foo[feat]; extra == 'feat2'",
93+
"foo[feat3]; extra == 'all'",
9394
"zdep; extra == 'all'",
9495
],
9596
extras = ["all"],
@@ -100,6 +101,24 @@ def test_self_dependencies_can_come_in_any_order(env):
100101

101102
_tests.append(test_self_dependencies_can_come_in_any_order)
102103

104+
def test_self_include_deps_from_previously_visited(env):
105+
got = deps(
106+
"foo",
107+
requires_dist = [
108+
"bar",
109+
"baz; extra == 'feat'",
110+
"foo[dev]; extra == 'all'",
111+
"foo[feat]; extra == 'feat2'",
112+
"dev_dep; extra == 'dev'",
113+
],
114+
extras = ["feat2"],
115+
)
116+
117+
env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"])
118+
env.expect.that_dict(got.deps_select).contains_exactly({})
119+
120+
_tests.append(test_self_include_deps_from_previously_visited)
121+
103122
def _test_can_get_deps_based_on_specific_python_version(env):
104123
requires_dist = [
105124
"bar",

0 commit comments

Comments
 (0)