From 3b8974466212b300ef68a06407746ee9c9d93a51 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 20 Jan 2026 18:26:19 +0900 Subject: [PATCH 1/4] fix(pipstar): correctly handle complex self deps 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 --- CHANGELOG.md | 8 ++++++++ python/private/pypi/pep508_deps.bzl | 11 +++++++++-- tests/pypi/pep508/deps_tests.bzl | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00cac20717..4a4c6a16b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,14 @@ END_UNRELEASED_TEMPLATE Use the `bazel_binary_info` module to access it. The {flag}`--stamp` flag will add {flag}`--workspace_status` information. +{#v1-8-1} +## [1.8.1] - 2026-01-20 + +{#v1-8-1-fixed} +### Fixed +* (pipstar) The extra resolution for complex packages is now handled again correctly. + Fixes [#3524](https://github.com/bazel-contrib/rules_python/issues/3524). + {#v1-8-0} ## [1.8.0] - 2025-12-19 diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index cdb449bdc2..c6d60595d3 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -135,7 +135,7 @@ def _resolve_extras(self_name, reqs, extras): # A double loop is not strictly optimal, but always correct without recursion for req in self_reqs: - if [True for extra in extras if evaluate(req.marker, env = {"extra": extra})]: + if _evaluate_any(req, extras): extras = extras + req.extras else: continue @@ -143,12 +143,19 @@ def _resolve_extras(self_name, reqs, extras): # Iterate through all packages to ensure that we include all of the extras from previously # visited packages. for req_ in self_reqs: - if [True for extra in extras if evaluate(req.marker, env = {"extra": extra})]: + if _evaluate_any(req_, extras): extras = extras + req_.extras # Poor mans set return sorted({x: None for x in extras}) +def _evaluate_any(req, extras): + for extra in extras: + if evaluate(req.marker, env = {"extra": extra}): + return True + + return False + def _add_reqs(deps, deps_select, dep, reqs, *, extras): for req in reqs: if not req.marker: diff --git a/tests/pypi/pep508/deps_tests.bzl b/tests/pypi/pep508/deps_tests.bzl index 679ba58396..f566845b70 100644 --- a/tests/pypi/pep508/deps_tests.bzl +++ b/tests/pypi/pep508/deps_tests.bzl @@ -90,6 +90,7 @@ def test_self_dependencies_can_come_in_any_order(env): "baz; extra == 'feat'", "foo[feat2]; extra == 'all'", "foo[feat]; extra == 'feat2'", + "foo[feat3]; extra == 'all'", "zdep; extra == 'all'", ], extras = ["all"], @@ -100,6 +101,24 @@ def test_self_dependencies_can_come_in_any_order(env): _tests.append(test_self_dependencies_can_come_in_any_order) +def test_self_include_deps_from_previously_visited(env): + got = deps( + "foo", + requires_dist = [ + "bar", + "baz; extra == 'feat'", + "foo[dev]; extra == 'all'", + "foo[feat]; extra == 'feat2'", + "dev_dep; extra == 'dev'", + ], + extras = ["feat2"], + ) + + env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) + env.expect.that_dict(got.deps_select).contains_exactly({}) + +_tests.append(test_self_include_deps_from_previously_visited) + def _test_can_get_deps_based_on_specific_python_version(env): requires_dist = [ "bar", From 6b13388501a79c5a4b92dc13db17bd1c553b9938 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 20 Jan 2026 18:43:22 +0900 Subject: [PATCH 2/4] use a more robust algo than a double loop --- python/private/pypi/pep508_deps.bzl | 33 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index c6d60595d3..ef1a178951 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -115,7 +115,9 @@ def _resolve_extras(self_name, reqs, extras): # extras The empty string in the set is just a way to make the handling # of no extras and a single extra easier and having a set of {"", "foo"} # is equivalent to having {"foo"}. - extras = extras or [""] + # + # Use a dict as a set here to simplify operations. + extras = {x: None for x in extras or [""]} self_reqs = [] for req in reqs: @@ -128,26 +130,29 @@ def _resolve_extras(self_name, reqs, extras): # easy to handle, lets do it. # # TODO @aignas 2023-12-08: add a test - extras = extras + req.extras + extras = extras | {x: None for x in req.extras} else: # process these in a separate loop self_reqs.append(req) - # A double loop is not strictly optimal, but always correct without recursion - for req in self_reqs: - if _evaluate_any(req, extras): - extras = extras + req.extras - else: - continue + for _ in range(10000): + # handles packages with up to 10000 recursive extras + new_extras = {} + for req in self_reqs: + if _evaluate_any(req, extras): + new_extras.update({x: None for x in req.extras}) + else: + continue + + num_extras_before = len(extras) + extras = extras | new_extras + num_extras_after = len(new_extras) - # Iterate through all packages to ensure that we include all of the extras from previously - # visited packages. - for req_ in self_reqs: - if _evaluate_any(req_, extras): - extras = extras + req_.extras + if num_extras_before == num_extras_after: + break # Poor mans set - return sorted({x: None for x in extras}) + return sorted(extras) def _evaluate_any(req, extras): for extra in extras: From 7d347d80890384455e4a29e966b1189497253c20 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 20 Jan 2026 08:20:33 -0800 Subject: [PATCH 3/4] Add parens to clarify operator order --- python/private/pypi/pep508_deps.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index ef1a178951..ad6589cfac 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -117,7 +117,7 @@ def _resolve_extras(self_name, reqs, extras): # is equivalent to having {"foo"}. # # Use a dict as a set here to simplify operations. - extras = {x: None for x in extras or [""]} + extras = {x: None for x in (extras or [""])} self_reqs = [] for req in reqs: From a2d57c831dc6f076e44ac19d153184f0e900847a Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 20 Jan 2026 08:21:49 -0800 Subject: [PATCH 4/4] Clarify changelog message --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69e4f11d0d..40ae51bfc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,7 +84,7 @@ END_UNRELEASED_TEMPLATE {#v1-8-1-fixed} ### Fixed -* (pipstar) The extra resolution for complex packages is now handled again correctly. +* (pipstar) Extra resolution that refers back to the package being resolved works again. Fixes [#3524](https://github.com/bazel-contrib/rules_python/issues/3524). {#v1-8-0}