Skip to content

Commit

Permalink
Avoid re-adding solutions to forked state (#3967)
Browse files Browse the repository at this point in the history
## Summary

Running a resolution that required forking was failing due to breaking
an invariant in PubGrub. It looks like we were adding the same
incompatibility multiple times, or something like that. The issue
appears to be that when forking, we modify the current state, then clone
it as the "next state", then push to the "forked states" -- but that
means we're cloning the _modified_ state.

This PR changes the order of operations such that we clone, then modify.
It shouldn't introduce any additional clones though.
  • Loading branch information
charliermarsh authored Jun 2, 2024
1 parent 01d1a39 commit c500b78
Showing 1 changed file with 23 additions and 22 deletions.
45 changes: 23 additions & 22 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,21 +485,21 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let mut cur_state = Some(state);
for (i, fork) in forks.into_iter().enumerate() {
let is_last = i == forks_len - 1;
let state = cur_state.as_mut().unwrap();
// let mut state = state.clone();

let dependencies = match fork {
Dependencies::Unavailable(reason) => {
state
.pubgrub
.add_incompatibility(Incompatibility::custom_version(
package.clone(),
version.clone(),
UnavailableReason::Version(reason),
));
let forked_state = cur_state.take().unwrap();
let mut forked_state = cur_state.take().unwrap();
if !is_last {
cur_state = Some(forked_state.clone());
}

forked_state.pubgrub.add_incompatibility(
Incompatibility::custom_version(
package.clone(),
version.clone(),
UnavailableReason::Version(reason),
),
);
forked_states.push(forked_state);
continue;
}
Expand All @@ -520,23 +520,24 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
Dependencies::Available(constraints) => constraints,
};

// Add that package and version if the dependencies are not problematic.
let dep_incompats = state.pubgrub.add_incompatibility_from_dependencies(
package.clone(),
version.clone(),
dependencies,
);
let mut forked_state = cur_state.take().unwrap();
if !is_last {
cur_state = Some(forked_state.clone());
}

state.pubgrub.partial_solution.add_version(
// Add that package and version if the dependencies are not problematic.
let dep_incompats =
forked_state.pubgrub.add_incompatibility_from_dependencies(
package.clone(),
version.clone(),
dependencies,
);
forked_state.pubgrub.partial_solution.add_version(
package.clone(),
version.clone(),
dep_incompats,
&state.pubgrub.incompatibility_store,
&forked_state.pubgrub.incompatibility_store,
);
let forked_state = cur_state.take().unwrap();
if !is_last {
cur_state = Some(forked_state.clone());
}
forked_states.push(forked_state);
}
continue 'FORK;
Expand Down

0 comments on commit c500b78

Please sign in to comment.