Skip to content

Commit

Permalink
Remove same-graph merging in resolver
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 14, 2024
1 parent fbefe60 commit 591f126
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 49 deletions.
13 changes: 6 additions & 7 deletions crates/uv-resolver/src/resolution/graph.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
use std::collections::BTreeSet;

use indexmap::IndexSet;
use petgraph::{
graph::{Graph, NodeIndex},
Directed, Direction,
};
use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};

use distribution_types::{
Dist, DistributionMetadata, Name, ResolutionDiagnostic, ResolvedDist, VersionId,
VersionOrUrlRef,
};
use indexmap::IndexSet;
use pep440_rs::{Version, VersionSpecifier};
use pep508_rs::{MarkerEnvironment, MarkerTree, MarkerTreeKind, VerbatimUrl};
use petgraph::{
graph::{Graph, NodeIndex},
Directed, Direction,
};
use pypi_types::{HashDigest, ParsedUrlError, Requirement, VerbatimParsedUrl, Yanked};
use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
use uv_configuration::{Constraints, Overrides};
use uv_distribution::Metadata;
use uv_git::GitResolver;
Expand Down
38 changes: 0 additions & 38 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,29 +387,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
);
}

// If another fork had the same resolution, merge into that fork instead.
if let Some(existing_resolution) = resolutions
.iter_mut()
.find(|existing_resolution| resolution.same_graph(existing_resolution))
{
let ResolverMarkers::Fork(existing_markers) = &existing_resolution.markers
else {
panic!("A non-forking resolution exists in forking mode")
};
let mut new_markers = existing_markers.clone();
new_markers.or(resolution
.markers
.fork_markers()
.expect("A non-forking resolution exists in forking mode")
.clone());
existing_resolution.markers = if new_markers.is_true() {
ResolverMarkers::universal(None)
} else {
ResolverMarkers::Fork(new_markers)
};
continue 'FORK;
}

Self::trace_resolution(&resolution);
resolutions.push(resolution);
continue 'FORK;
Expand Down Expand Up @@ -2529,21 +2506,6 @@ pub(crate) struct ResolutionDependencyEdge {
pub(crate) marker: MarkerTree,
}

impl Resolution {
/// Whether we got two identical resolutions in two separate forks.
///
/// Ignores pins since the which distribution we prioritized for each version doesn't matter.
fn same_graph(&self, other: &Self) -> bool {
// TODO(konsti): The edges being equal is not a requirement for the graph being equal. While
// an exact solution is too much here, we should ignore different in edges that point to
// nodes that are always installed. Example: root requires foo, root requires bar. bar
// forks, and one for the branches has bar -> foo while the other doesn't. The resolution
// is still the same graph since the presence or absence of the bar -> foo edge cannot
// change which packages and versions are installed.
self.nodes == other.nodes && self.edges == other.edges
}
}

impl ResolutionPackage {
pub(crate) fn is_base(&self) -> bool {
self.extra.is_none() && self.dev.is_none()
Expand Down
8 changes: 4 additions & 4 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7521,11 +7521,11 @@ fn universal_nested_disjoint_local_requirement() -> Result<()> {
# via torch
tbb==2021.11.0 ; os_name != 'Linux' and platform_system == 'Windows'
# via mkl
torch==2.0.0+cpu ; os_name == 'Linux'
torch==2.0.0+cpu ; os_name == 'Linux' and platform_machine != 'x86_64'
# via
# -r requirements.in
# example
torch==2.0.0+cu118 ; os_name == 'Linux'
torch==2.0.0+cu118 ; os_name == 'Linux' and platform_machine == 'x86_64'
# via
# -r requirements.in
# example
Expand Down Expand Up @@ -7768,11 +7768,11 @@ fn universal_transitive_disjoint_prerelease_requirement() -> Result<()> {
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] requirements.in --universal
cffi==1.16.0 ; platform_python_implementation != 'PyPy' or os_name == 'linux'
cffi==1.16.0 ; os_name == 'linux'
# via
# -r requirements.in
# cryptography
cffi==1.17.0rc1 ; os_name != 'linux' or platform_python_implementation != 'PyPy'
cffi==1.17.0rc1 ; os_name != 'linux'
# via
# -r requirements.in
# cryptography
Expand Down

0 comments on commit 591f126

Please sign in to comment.