Skip to content

Commit

Permalink
Remove lossy resolution-to-requirements conversion in install plan
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Sep 30, 2024
1 parent 66d7ec5 commit f67c469
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 341 deletions.
8 changes: 1 addition & 7 deletions crates/distribution-types/src/resolved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,8 @@ impl From<Dist> for ResolvedDist {
}
}

impl From<InstalledDist> for ResolvedDist {
fn from(value: InstalledDist) -> Self {
ResolvedDist::Installed(value)
}
}

impl Display for ResolvedDist {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::Installed(dist) => dist.fmt(f),
Self::Installable(dist) => dist.fmt(f),
Expand Down
17 changes: 1 addition & 16 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,16 @@ impl<'a> BuildContext for BuildDispatch<'a> {

// Determine the current environment markers.
let tags = self.interpreter.tags()?;
let markers = self.interpreter.resolver_markers();

// Determine the set of installed packages.
let site_packages = SitePackages::from_environment(venv)?;

let requirements = resolution.requirements().collect::<Vec<_>>();

let Plan {
cached,
remote,
reinstalls,
extraneous: _,
} = Planner::new(&requirements).build(
} = Planner::new(resolution).build(
site_packages,
&Reinstall::default(),
&BuildOptions::default(),
Expand All @@ -234,7 +231,6 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.config_settings,
self.cache(),
venv,
&markers,
tags,
)?;

Expand All @@ -244,17 +240,6 @@ impl<'a> BuildContext for BuildDispatch<'a> {
return Ok(vec![]);
}

// Resolve any registry-based requirements.
let remote = remote
.iter()
.map(|dist| {
resolution
.get_remote(&dist.name)
.cloned()
.expect("Resolution should contain all packages")
})
.collect::<Vec<_>>();

// Download any missing distributions.
let wheels = if remote.is_empty() {
vec![]
Expand Down
455 changes: 171 additions & 284 deletions crates/uv-installer/src/plan.rs

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions crates/uv-installer/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ impl SitePackages {
.collect()
}

pub fn remove_distribution(&mut self, dist: &InstalledDist) -> Option<InstalledDist> {
let index = self
.distributions
.iter()
.position(|distribution| distribution.as_ref() == Some(dist));
index.and_then(|index| self.distributions[index].take())
}

/// Remove the given packages from the index, returning all installed versions, if any.
pub fn remove_packages(&mut self, name: &PackageName) -> Vec<InstalledDist> {
let Some(indexes) = self.by_name.get(name) else {
Expand Down
1 change: 1 addition & 0 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
url.verbatim
);

// STOPSHIP(charlie): We should actually grab the installed distribution here, probably?
let dist = PubGrubDistribution::from_url(name, url);
let response = self
.index
Expand Down
1 change: 0 additions & 1 deletion crates/uv/src/commands/pip/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ pub(crate) async fn pip_install(
&index_locations,
config_settings,
&hasher,
&markers,
&tags,
&client,
&state.in_flight,
Expand Down
29 changes: 1 addition & 28 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ pub(crate) async fn install(
index_urls: &IndexLocations,
config_settings: &ConfigSettings,
hasher: &HashStrategy,
markers: &ResolverMarkerEnvironment,
tags: &Tags,
client: &RegistryClient,
in_flight: &InFlight,
Expand All @@ -405,12 +404,9 @@ pub(crate) async fn install(
) -> Result<Changelog, Error> {
let start = std::time::Instant::now();

// Extract the requirements from the resolution.
let requirements = resolution.requirements().collect::<Vec<_>>();

// Partition into those that should be linked from the cache (`local`), those that need to be
// downloaded (`remote`), and those that should be removed (`extraneous`).
let plan = Planner::new(&requirements)
let plan = Planner::new(resolution)
.build(
site_packages,
reinstall,
Expand All @@ -420,7 +416,6 @@ pub(crate) async fn install(
config_settings,
cache,
venv,
markers,
tags,
)
.context("Failed to determine installation plan")?;
Expand Down Expand Up @@ -449,17 +444,6 @@ pub(crate) async fn install(
return Ok(Changelog::default());
}

// Map any registry-based requirements back to those returned by the resolver.
let remote = remote
.iter()
.map(|dist| {
resolution
.get_remote(&dist.name)
.cloned()
.expect("Resolution should contain all packages")
})
.collect::<Vec<_>>();

// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
vec![]
Expand Down Expand Up @@ -582,17 +566,6 @@ fn report_dry_run(
return Ok(());
}

// Map any registry-based requirements back to those returned by the resolver.
let remote = remote
.iter()
.map(|dist| {
resolution
.get_remote(&dist.name)
.cloned()
.expect("Resolution should contain all packages")
})
.collect::<Vec<_>>();

// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
vec![]
Expand Down
1 change: 0 additions & 1 deletion crates/uv/src/commands/pip/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ pub(crate) async fn pip_sync(
&index_locations,
config_settings,
&hasher,
&markers,
&tags,
&client,
&state.in_flight,
Expand Down
3 changes: 0 additions & 3 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,6 @@ pub(crate) async fn sync_environment(
// Determine the markers tags to use for resolution.
let interpreter = venv.interpreter();
let tags = venv.interpreter().tags()?;
let markers = interpreter.resolver_markers();

// Add all authenticated sources to the cache.
for url in index_locations.urls() {
Expand Down Expand Up @@ -1006,7 +1005,6 @@ pub(crate) async fn sync_environment(
index_locations,
config_setting,
&hasher,
&markers,
tags,
&client,
&state.in_flight,
Expand Down Expand Up @@ -1242,7 +1240,6 @@ pub(crate) async fn update_environment(
index_locations,
config_setting,
&hasher,
&markers,
tags,
&client,
&state.in_flight,
Expand Down
1 change: 0 additions & 1 deletion crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ pub(super) async fn do_sync(
index_locations,
config_setting,
&hasher,
&markers,
tags,
&client,
&state.in_flight,
Expand Down

0 comments on commit f67c469

Please sign in to comment.