Skip to content

Hold a transitive update that would re-fork a deduped package - #38919

Open
robobun wants to merge 31 commits into
mainfrom
farm/38bd817a/update-dedupe-fixed-point
Open

Hold a transitive update that would re-fork a deduped package#38919
robobun wants to merge 31 commits into
mainfrom
farm/38bd817a/update-dedupe-fixed-point

Shorten the forks_surviving_instance doc comment

c0d7fd9
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 15, 2026 in 34m 28s

Code review found 2 important issues

Found 1 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/install/update_transitive.rs:1310-1318 Some(w) arm treats a planned-but-held sibling as vacating, re-introducing the fork

Annotations

Check failure on line 1318 in src/install/update_transitive.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Some(w) arm treats a planned-but-held sibling as vacating, re-introducing the fork

🔴 The `Some(w)` arm treats any sibling want with `planned[w].is_some()` as vacating `current`, but that sibling may itself be held by `forks_surviving_instance` a few iterations later in the same loop — so it never gets a pin, stays at `current`, and the current want re-forks it anyway. With instance `pkg@1.0.0`, in-scope wants A=`*`→3.0.0 and B=`^1||^2`→2.0.0, plus an out-of-scope transitive edge D=`^1||^3` (None branch under `--filter`, or a provided peer): A's check sees `planned[B].is_some()