Skip to content

Commit

Permalink
Unify dependency iteration in ResolverState::get_dependencies
Browse files Browse the repository at this point in the history 
Upstack PR: #4430

Split out from #4430 according to #4430 (comment).
  • Loading branch information
@konstin
konstin committed Jun 25, 2024
1 parent 7fe7d8b commit 384a290
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 99 deletions.
3 changes: 0 additions & 3 deletions crates/uv-resolver/src/resolver/availability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ pub(crate) enum UnavailableVersion {
InvalidStructure,
/// The wheel metadata was not found in the cache and the network is not available.
Offline,
/// Forward any kind of resolver error.
ResolverError(String),
}

impl Display for UnavailableVersion {
Expand All @@ -57,7 +55,6 @@ impl Display for UnavailableVersion {
UnavailableVersion::Offline => f.write_str(
"network connectivity is disabled, but the metadata wasn't found in the cache",
),
UnavailableVersion::ResolverError(err) => f.write_str(err),
}
}
}
Expand Down
176 changes: 80 additions & 96 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
priorities: &mut PubGrubPriorities,
request_sink: &Sender<Request>,
) -> Result<Dependencies, ResolveError> {
match &**package {
let (dependencies, name) = match &**package {
PubGrubPackageInner::Root(_) => {
// Add the root requirements.
let dependencies = PubGrubDependencies::from_requirements(
Expand All @@ -953,31 +953,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
self.markers.as_ref(),
self.requires_python.as_ref(),
markers,
);

let dependencies = match dependencies {
Ok(dependencies) => dependencies,
Err(err) => {
return Ok(Dependencies::Unavailable(
UnavailableVersion::ResolverError(uncapitalize(err.to_string())),
));
}
};

for (package, version) in dependencies.iter() {
debug!("Adding direct dependency: {package}{version}");

// Update the package priorities.
priorities.insert(package, version);

// Emit a request to fetch the metadata for this package.
self.visit_package(package, request_sink)?;
}

Ok(Dependencies::Available(dependencies.into()))
)?;
(dependencies, None)
}

PubGrubPackageInner::Python(_) => Ok(Dependencies::Available(Vec::default())),
PubGrubPackageInner::Python(_) => return Ok(Dependencies::Available(Vec::default())),

PubGrubPackageInner::Package {
name,
Expand Down Expand Up @@ -1108,16 +1088,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
markers,
)?;

for (dep_package, dep_version) in dependencies.iter() {
debug!("Adding transitive dependency for {package}=={version}: {dep_package}{dep_version}");

// Update the package priorities.
priorities.insert(dep_package, dep_version);

// Emit a request to fetch the metadata for this package.
self.visit_package(dep_package, request_sink)?;
}

// If a package has metadata for an enabled dependency group,
// add a dependency from it to the same package with the group
// enabled.
Expand All @@ -1137,54 +1107,58 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
}

Ok(Dependencies::Available(dependencies.into()))
(dependencies, Some(name))
}

// Add a dependency on both the marker and base package.
PubGrubPackageInner::Marker { name, marker, url } => Ok(Dependencies::Available(
[None, Some(marker)]
.into_iter()
.map(move |marker| {
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: None,
marker: marker.cloned(),
url: url.clone(),
}),
Range::singleton(version.clone()),
)
})
.collect(),
)),

// Add a dependency on both the extra and base package, with and without the marker.
PubGrubPackageInner::Extra {
name,
extra,
marker,
url,
} => Ok(Dependencies::Available(
[None, marker.as_ref()]
.into_iter()
.dedup()
.flat_map(move |marker| {
[None, Some(extra)].into_iter().map(move |extra| {
PubGrubPackageInner::Marker { name, marker, url } => {
return Ok(Dependencies::Available(
[None, Some(marker)]
.into_iter()
.map(move |marker| {
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: extra.cloned(),
extra: None,
dev: None,
marker: marker.cloned(),
url: url.clone(),
}),
Range::singleton(version.clone()),
)
})
})
.collect(),
)),
.collect(),
))
}

// Add a dependency on both the extra and base package, with and without the marker.
PubGrubPackageInner::Extra {
name,
extra,
marker,
url,
} => {
return Ok(Dependencies::Available(
[None, marker.as_ref()]
.into_iter()
.dedup()
.flat_map(move |marker| {
[None, Some(extra)].into_iter().map(move |extra| {
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: extra.cloned(),
dev: None,
marker: marker.cloned(),
url: url.clone(),
}),
Range::singleton(version.clone()),
)
})
})
.collect(),
))
}

// Add a dependency on both the development dependency group and base package, with and
// without the marker.
Expand All @@ -1193,27 +1167,45 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
dev,
marker,
url,
} => Ok(Dependencies::Available(
[None, marker.as_ref()]
.into_iter()
.dedup()
.flat_map(move |marker| {
[None, Some(dev)].into_iter().map(move |dev| {
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: dev.cloned(),
marker: marker.cloned(),
url: url.clone(),
}),
Range::singleton(version.clone()),
)
} => {
return Ok(Dependencies::Available(
[None, marker.as_ref()]
.into_iter()
.dedup()
.flat_map(move |marker| {
[None, Some(dev)].into_iter().map(move |dev| {
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: dev.cloned(),
marker: marker.cloned(),
url: url.clone(),
}),
Range::singleton(version.clone()),
)
})
})
})
.collect(),
)),
.collect(),
))
}
};

for (dep_package, dep_version) in dependencies.iter() {
if let Some(name) = name {
debug!("Adding transitive dependency for {name}=={version}: {dep_package}{dep_version}");
} else {
debug!("Adding direct dependency: {dep_package}{dep_version}");
}

// Update the package priorities.
priorities.insert(dep_package, dep_version);

// Emit a request to fetch the metadata for this package.
self.visit_package(dep_package, request_sink)?;
}

Ok(Dependencies::Available(dependencies.into()))
}

/// Fetch the metadata for a stream of packages and versions.
Expand Down Expand Up @@ -2295,11 +2287,3 @@ impl<'a> PossibleFork<'a> {
false
}
}

fn uncapitalize<T: AsRef<str>>(string: T) -> String {
let mut chars = string.as_ref().chars();
match chars.next() {
None => String::new(),
Some(first) => first.to_lowercase().chain(chars).collect(),
}
}

0 comments on commit 384a290

Please sign in to comment.