From c500b789369c3801b32a65847e109a50c43c2001 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 2 Jun 2024 17:58:25 -0400 Subject: [PATCH] Avoid re-adding solutions to forked state (#3967) ## 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. --- crates/uv-resolver/src/resolver/mod.rs | 49 +++++++++++++------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index d2b0e31cb..873238871 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -485,21 +485,21 @@ impl ResolverState { - 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; } @@ -520,23 +520,24 @@ impl ResolverState 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, - ); - - state.pubgrub.partial_solution.add_version( - package.clone(), - version.clone(), - dep_incompats, - &state.pubgrub.incompatibility_store, - ); - let forked_state = cur_state.take().unwrap(); + let mut forked_state = cur_state.take().unwrap(); if !is_last { cur_state = Some(forked_state.clone()); } + + // 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, + &forked_state.pubgrub.incompatibility_store, + ); forked_states.push(forked_state); } continue 'FORK;