fix(pypi): resolve self-referencing extras to a real fixed point - #4039

Summary

_resolve_extras decides that its fixed-point loop has converged by comparing num_extras_before — the size of the extras set at the start of the round — against len(new_extras), the number of extras discovered during that round:

num_extras_before = len(extras)
extras = extras | new_extras
num_extras_after = len(new_extras)   # the delta, not the merged size

if num_extras_before == num_extras_after:
    break

Those are unrelated quantities, and the mismatch breaks in two separate ways.

1. Extras are silently dropped

The loop exits early whenever the two counts happen to coincide while the set is still growing, so extras reachable only through a further round are never resolved —- and every dependency gated on them silently disappears from the generated target.

The smallest reproducer is a two-hop self-extras chain:

Anything behind extra == 'c' is lost, with no error. This is not limited to the first round: a chain that branches before it deepens (all → {p, q}, p → r, r → t) hits the same equality on round two and drops t.

2. The loop never terminates early for ordinary packages

For a package with no self-referencing extras — the overwhelmingly common case — self_reqs is empty, so new_extras is always {} while extras holds at least one entry. The condition can never hold, and the loop runs all 10000 rounds, allocating a dict each time, while evaluating the generated BUILD file of every wheel in the build.

On a ~52k-package repository this dominated loading-phase Starlark CPU:

The fix

Compare the size of the merged set, which is what the before/after naming already implied. The loop is monotonic, so the converged set is unchanged wherever it previously terminated correctly — this only stops it terminating too early, or not at all.

Tests

Three regression tests are added to tests/pypi/pep508/deps_tests.bzl, covering the three shapes that trigger the early exit: a two-hop chain, multiple requested extras, and a chain where the counts only coincide after the first round.

All three fail on main with exactly the dropped dependency, and pass with the fix:

//tests/pypi/pep508:test_self_extras_chain_is_fully_resolved              FAILED   1 missing: c_dep
//tests/pypi/pep508:test_self_extras_chain_with_multiple_requested_extras FAILED   1 missing: c_dep
//tests/pypi/pep508:test_self_extras_chain_resolved_beyond_the_first_round FAILED  1 missing: t_dep

The three existing tests that exercise self-extras chains (test_self_is_ignored, test_self_dependencies_can_come_in_any_order, test_self_include_deps_from_previously_visited) pass either way, which is why this went unnoticed.

bazel test //tests/pypi/... is green (249/249) with the fix applied.

Notes

The condition was introduced in #3527, which replaced the previous double loop with this fixed-point loop.