mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-07-07 21:35:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Rebase: Uninstall existing non-editable versions when installing editable requirements bug (#682)

Browse source 
Separate branch for rebasing #677 onto main because i don't trust the
rebase enough to force push.

Closes #677.

---

If you install `black` from PyPI, then `-e ../black`, we need to
uninstall the existing `black`. This sounds simple, but that in turn
requires that we _know_ `-e ../black` maps to the package `black`, so
that we can mark it for uninstallation in the install plan. This, in
turn, means that we need to build editable dependencies prior to the
install plan.

This is just a bunch of reorganization to fix that specific bug
(installing multiple versions of `black` if you run through the above
workflow): we now run through the list of editables upfront, mark those
that are already installed, build those that aren't, and then ensure
that `InstallPlan` correctly removes those that need to be removed, etc.

Closes #676.

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
konsti 2023-12-18 10:28:14 +01:00 • committed by GitHub
parent 0bb2c92246
commit f4f67ebde0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 626 additions and 279 deletions
Download patch file Download diff file Expand all files Collapse all files

26
crates/distribution-types/src/resolution.rs
View file

@ -2,7 +2,6 @@ use rustc_hash::FxHashMap;
use pep508_rs::Requirement;
use puffin_normalize::PackageName;
use requirements_txt::EditableRequirement;
use crate::{BuiltDist, Dist, PathSourceDist, SourceDist};
@ -60,31 +59,6 @@ impl Resolution {
requirements.sort_unstable_by(|a, b| a.name.cmp(&b.name));
requirements
}
/// Return the set of [`EditableRequirement`]s that this resolution represents.
pub fn editable_requirements(&self) -> Vec<EditableRequirement> {
let mut requirements = self
.0
.values()
.filter_map(|dist| {
let Dist::Source(SourceDist::Path(PathSourceDist {
url,
path,
editable: true,
..
})) = dist
else {
return None;
};
Some(EditableRequirement::Path {
path: path.clone(),
url: url.clone(),
})
})
.collect::<Vec<_>>();
requirements.sort_unstable_by(|a, b| a.url().cmp(b.url()));
requirements
}
}
impl From<Dist> for Requirement {

13
crates/puffin-cli/src/commands/pip_compile.rs
View file

@ -114,9 +114,6 @@ pub(crate) async fn pip_compile(
.map(|spec| spec.requirements)
.unwrap_or_default();
// Create a manifest of the requirements.
let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer);
// Detect the current Python interpreter.
let platform = Platform::current()?;
let venv = Virtualenv::from_env(platform, &cache)?;
@ -127,13 +124,9 @@ pub(crate) async fn pip_compile(
venv.python_executable().display()
);
// Determine the compatible platform tags.
let tags = Tags::from_interpreter(venv.interpreter())?;
// Determine the interpreter to use for resolution.
// Determine the tags, markers, and interpreter to use for resolution.
let interpreter = venv.interpreter().clone();
// Determine the markers to use for resolution.
let tags = Tags::from_interpreter(venv.interpreter())?;
let markers = python_version.map_or_else(
|| Cow::Borrowed(venv.interpreter().markers()),
|python_version| Cow::Owned(python_version.markers(venv.interpreter().markers())),
@ -144,6 +137,7 @@ pub(crate) async fn pip_compile(
.index_urls(index_urls.clone())
.build();
let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer);
let build_dispatch = BuildDispatch::new(
client.clone(),
cache.clone(),
@ -203,6 +197,7 @@ pub(crate) async fn pip_compile(
editable_metadata
};
// Create a manifest of the requirements.
let manifest = Manifest::new(
requirements,
constraints,

130
crates/puffin-cli/src/commands/pip_install.rs
View file

@ -18,9 +18,10 @@ use puffin_cache::Cache;
use puffin_client::{RegistryClient, RegistryClientBuilder};
use puffin_dispatch::BuildDispatch;
use puffin_installer::{
BuiltEditable, Downloader, EditableMode, InstallPlan, Reinstall, SitePackages,
BuiltEditable, Downloader, InstallPlan, Reinstall, ResolvedEditable, SitePackages,
};
use puffin_interpreter::Virtualenv;
use puffin_normalize::PackageName;
use puffin_resolver::{
Manifest, PreReleaseMode, ResolutionGraph, ResolutionMode, ResolutionOptions, Resolver,
};
@ -62,8 +63,32 @@ pub(crate) async fn pip_install(
let start = std::time::Instant::now();
// Determine the requirements.
let spec = specification(requirements, constraints, overrides, extras)?;
// Read all requirements from the provided sources.
let RequirementsSpecification {
project,
requirements,
constraints,
overrides,
editables,
extras: used_extras,
} = specification(requirements, constraints, overrides, extras)?;
// Check that all provided extras are used
if let ExtrasSpecification::Some(extras) = extras {
let mut unused_extras = extras
.iter()
.filter(|extra| !used_extras.contains(extra))
.collect::<Vec<_>>();
if !unused_extras.is_empty() {
unused_extras.sort_unstable();
unused_extras.dedup();
let s = if unused_extras.len() == 1 { "" } else { "s" };
return Err(anyhow!(
"Requested extra{s} not found: {}",
unused_extras.iter().join(", ")
));
}
}
// Detect the current Python interpreter.
let platform = Platform::current()?;
@ -73,11 +98,15 @@ pub(crate) async fn pip_install(
venv.python_executable().display()
);
// Determine the set of installed packages.
let site_packages =
SitePackages::from_executable(&venv).context("Failed to list installed packages")?;
// If the requirements are already satisfied, we're done. Ideally, the resolver would be fast
// enough to let us remove this check. But right now, for large environments, it's an order of
// magnitude faster to validate the environment than to resolve the requirements.
if reinstall.is_none() && satisfied(&spec, &venv)? {
let num_requirements = spec.requirements.len() + spec.editables.len();
if reinstall.is_none() && site_packages.satisfies(&requirements, &editables, &constraints)? {
let num_requirements = requirements.len() + editables.len();
let s = if num_requirements == 1 { "" } else { "s" };
writeln!(
printer,
@ -92,13 +121,9 @@ pub(crate) async fn pip_install(
return Ok(ExitStatus::Success);
}
// Determine the compatible platform tags.
let tags = Tags::from_interpreter(venv.interpreter())?;
// Determine the interpreter to use for resolution.
// Determine the tags, markers, and interpreter to use for resolution.
let interpreter = venv.interpreter().clone();
// Determine the markers to use for resolution.
let tags = Tags::from_interpreter(venv.interpreter())?;
let markers = venv.interpreter().markers();
// Instantiate a client.
@ -107,6 +132,7 @@ pub(crate) async fn pip_install(
.build();
let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer);
let build_dispatch = BuildDispatch::new(
client.clone(),
cache.clone(),
@ -121,12 +147,12 @@ pub(crate) async fn pip_install(
// installation, and should live for the duration of the command. If an editable is already
// installed in the environment, we'll still re-build it here.
let editable_wheel_dir;
let editables = if spec.editables.is_empty() {
let editables = if editables.is_empty() {
vec![]
} else {
editable_wheel_dir = tempdir_in(venv.root())?;
build_editables(
&spec.editables,
&editables,
editable_wheel_dir.path(),
&cache,
&tags,
@ -139,15 +165,18 @@ pub(crate) async fn pip_install(
// Resolve the requirements.
let resolution = match resolve(
spec,
requirements,
constraints,
overrides,
project,
&editables,
&site_packages,
reinstall,
&tags,
markers,
&client,
&build_dispatch,
options,
&venv,
printer,
)
.await
@ -168,7 +197,8 @@ pub(crate) async fn pip_install(
// Sync the environment.
install(
&resolution,
&editables,
editables,
site_packages,
reinstall,
link_mode,
index_urls,
@ -228,15 +258,6 @@ fn specification(
Ok(spec)
}
/// Returns `true` if the requirements are already satisfied.
fn satisfied(spec: &RequirementsSpecification, venv: &Virtualenv) -> Result<bool, Error> {
Ok(SitePackages::from_executable(venv)?.satisfies(
&spec.requirements,
&spec.editables,
&spec.constraints,
)?)
}
/// Build a set of editable distributions.
async fn build_editables(
editables: &[EditableRequirement],
@ -290,36 +311,27 @@ async fn build_editables(
/// Resolve a set of requirements, similar to running `pip-compile`.
#[allow(clippy::too_many_arguments)]
async fn resolve(
spec: RequirementsSpecification,
requirements: Vec<Requirement>,
constraints: Vec<Requirement>,
overrides: Vec<Requirement>,
project: Option<PackageName>,
editables: &[BuiltEditable],
site_packages: &SitePackages<'_>,
reinstall: &Reinstall,
tags: &Tags,
markers: &MarkerEnvironment,
client: &RegistryClient,
build_dispatch: &BuildDispatch,
options: ResolutionOptions,
venv: &Virtualenv,
mut printer: Printer,
) -> Result<ResolutionGraph, Error> {
let start = std::time::Instant::now();
// Create a manifest of the requirements.
let RequirementsSpecification {
project,
requirements,
constraints,
overrides,
editables: _,
extras: _,
} = spec;
// Respect preferences from the existing environments.
let preferences: Vec<Requirement> = match reinstall {
Reinstall::All => vec![],
Reinstall::None => SitePackages::from_executable(venv)?
.requirements()
.collect(),
Reinstall::Packages(packages) => SitePackages::from_executable(venv)?
Reinstall::None => site_packages.requirements().collect(),
Reinstall::Packages(packages) => site_packages
.requirements()
.filter(|requirement| !packages.contains(&requirement.name))
.collect(),
@ -336,6 +348,7 @@ async fn resolve(
})
.collect();
// Create a manifest of the requirements.
let manifest = Manifest::new(
requirements,
constraints,
@ -369,7 +382,8 @@ async fn resolve(
#[allow(clippy::too_many_arguments)]
async fn install(
resolution: &Resolution,
built_editables: &[BuiltEditable],
built_editables: Vec<BuiltEditable>,
site_packages: SitePackages<'_>,
reinstall: &Reinstall,
link_mode: LinkMode,
index_urls: IndexUrls,
@ -384,26 +398,30 @@ async fn install(
// 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 requirements = resolution.requirements();
let editables = built_editables
.into_iter()
.map(ResolvedEditable::Built)
.collect::<Vec<_>>();
let InstallPlan {
local,
remote,
reinstalls,
editables,
extraneous: _,
} = InstallPlan::from_requirements(
&resolution.requirements(),
&resolution.editable_requirements(),
&requirements,
editables,
site_packages,
reinstall,
&index_urls,
cache,
venv,
tags,
EditableMode::Mutable,
)
.context("Failed to determine installation plan")?;
// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() && editables.is_empty() {
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() {
let s = if resolution.len() == 1 { "" } else { "s" };
writeln!(
printer,
@ -430,18 +448,6 @@ async fn install(
})
.collect::<Vec<_>>();
// Map any local editable requirements back to those that were built ahead of time.
let built_editables = editables
.iter()
.map(|editable| {
let built_editable = built_editables
.iter()
.find(|built_editable| built_editable.editable.requirement == *editable)
.expect("Editable should be built");
built_editable.wheel.clone()
})
.collect::<Vec<_>>();
// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
vec![]
@ -487,11 +493,7 @@ async fn install(
}
// Install the resolved distributions.
let wheels = wheels
.into_iter()
.chain(local)
.chain(built_editables)
.collect::<Vec<_>>();
let wheels = wheels.into_iter().chain(local).collect::<Vec<_>>();
if !wheels.is_empty() {
let start = std::time::Instant::now();
puffin_installer::Installer::new(venv)

265
crates/puffin-cli/src/commands/pip_sync.rs
View file

@ -4,18 +4,16 @@ use anyhow::{bail, Context, Result};
use colored::Colorize;
use fs_err as fs;
use itertools::Itertools;
use tempfile::tempdir_in;
use tracing::debug;
use distribution_types::{AnyDist, CachedDist, LocalEditable, Metadata};
use distribution_types::{AnyDist, LocalEditable, Metadata};
use install_wheel_rs::linker::LinkMode;
use pep508_rs::Requirement;
use platform_host::Platform;
use platform_tags::Tags;
use puffin_cache::Cache;
use puffin_client::RegistryClientBuilder;
use puffin_client::{RegistryClient, RegistryClientBuilder};
use puffin_dispatch::BuildDispatch;
use puffin_installer::{Downloader, EditableMode, InstallPlan, Reinstall, SitePackages};
use puffin_installer::{Downloader, InstallPlan, Reinstall, ResolvedEditable, SitePackages};
use puffin_interpreter::Virtualenv;
use puffin_traits::OnceMap;
use pypi_types::{IndexUrls, Yanked};
@ -27,6 +25,7 @@ use crate::printer::Printer;
use crate::requirements::{RequirementsSource, RequirementsSpecification};
/// Install a set of locked requirements into the current Python environment.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn pip_sync(
sources: &[RequirementsSource],
reinstall: &Reinstall,
@ -36,44 +35,19 @@ pub(crate) async fn pip_sync(
cache: Cache,
mut printer: Printer,
) -> Result<ExitStatus> {
let start = std::time::Instant::now();
// Read all requirements from the provided sources.
let (requirements, editables) = RequirementsSpecification::requirements_and_editables(sources)?;
if requirements.is_empty() && editables.is_empty() {
let num_requirements = requirements.len() + editables.len();
if num_requirements == 0 {
writeln!(printer, "No requirements found")?;
return Ok(ExitStatus::Success);
}
sync_requirements(
&requirements,
reinstall,
&editables,
link_mode,
index_urls,
no_build,
&cache,
printer,
)
.await
}
/// Install a set of locked requirements into the current Python environment.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn sync_requirements(
requirements: &[Requirement],
reinstall: &Reinstall,
editable_requirements: &[EditableRequirement],
link_mode: LinkMode,
index_urls: IndexUrls,
no_build: bool,
cache: &Cache,
mut printer: Printer,
) -> Result<ExitStatus> {
let start = std::time::Instant::now();
// Detect the current Python interpreter.
let platform = Platform::current()?;
let venv = Virtualenv::from_env(platform, cache)?;
let venv = Virtualenv::from_env(platform, &cache)?;
debug!(
"Using Python interpreter: {}",
venv.python_executable().display()
@ -82,34 +56,60 @@ pub(crate) async fn sync_requirements(
// Determine the current environment markers.
let tags = Tags::from_interpreter(venv.interpreter())?;
// Prep the registry client.
let client = RegistryClientBuilder::new(cache.clone())
.index_urls(index_urls.clone())
.build();
// Prep the build context.
let build_dispatch = BuildDispatch::new(
client.clone(),
cache.clone(),
venv.interpreter().clone(),
fs::canonicalize(venv.python_executable())?,
no_build,
index_urls.clone(),
);
// Determine the set of installed packages.
let site_packages =
SitePackages::from_executable(&venv).context("Failed to list installed packages")?;
// Resolve any editables.
let resolved_editables = resolve_editables(
editables,
&site_packages,
reinstall,
&venv,
&tags,
&cache,
&client,
&build_dispatch,
printer,
)
.await?;
// 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 InstallPlan {
local,
editables,
remote,
reinstalls,
extraneous,
} = InstallPlan::from_requirements(
requirements,
editable_requirements,
&requirements,
resolved_editables.editables,
site_packages,
reinstall,
&index_urls,
cache,
&cache,
&venv,
&tags,
EditableMode::Immutable,
)
.context("Failed to determine installation plan")?;
// Nothing to do.
if remote.is_empty()
&& local.is_empty()
&& reinstalls.is_empty()
&& extraneous.is_empty()
&& editables.is_empty()
{
let num_requirements = requirements.len() + editable_requirements.len();
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() && extraneous.is_empty() {
let s = if num_requirements == 1 { "" } else { "s" };
writeln!(
printer,
@ -182,66 +182,15 @@ pub(crate) async fn sync_requirements(
}
}
let build_dispatch = BuildDispatch::new(
client.clone(),
cache.clone(),
venv.interpreter().clone(),
fs::canonicalize(venv.python_executable())?,
no_build,
index_urls.clone(),
);
let downloader = Downloader::new(cache, &tags, &client, &build_dispatch).with_reporter(
DownloadReporter::from(printer).with_length((editables.len() + remote.len()) as u64),
);
// Build any editable requirements.
let editable_wheel_dir = tempdir_in(venv.root())?;
let built_editables = if editables.is_empty() {
Vec::new()
} else {
let start = std::time::Instant::now();
let editables: Vec<LocalEditable> = editables
.into_iter()
.map(|editable| match editable.clone() {
EditableRequirement::Path { path, .. } => Ok(LocalEditable {
requirement: editable,
path,
}),
EditableRequirement::Url(_) => {
bail!("url editables are not supported yet");
}
})
.collect::<Result<_>>()?;
let built_editables: Vec<CachedDist> = downloader
.build_editables(editables, editable_wheel_dir.path())
.await
.context("Failed to build editables")?
.into_iter()
.map(|built_editable| built_editable.wheel)
.collect();
let s = if built_editables.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Built {} in {}",
format!("{} editable{}", built_editables.len(), s).bold(),
elapsed(start.elapsed())
)
.dimmed()
)?;
built_editables
};
// Download, build, and unzip any missing distributions.
let wheels = if remote.is_empty() {
Vec::new()
} else {
let start = std::time::Instant::now();
let downloader = Downloader::new(&cache, &tags, &client, &build_dispatch)
.with_reporter(DownloadReporter::from(printer).with_length(remote.len() as u64));
let wheels = downloader
.download(remote, &OnceMap::default())
.await
@ -296,11 +245,7 @@ pub(crate) async fn sync_requirements(
}
// Install the resolved distributions.
let wheels = wheels
.into_iter()
.chain(local)
.chain(built_editables)
.collect::<Vec<_>>();
let wheels = wheels.into_iter().chain(local).collect::<Vec<_>>();
if !wheels.is_empty() {
let start = std::time::Instant::now();
puffin_installer::Installer::new(&venv)
@ -376,3 +321,111 @@ pub(crate) async fn sync_requirements(
Ok(ExitStatus::Success)
}
#[derive(Debug)]
struct ResolvedEditables {
/// The set of resolved editables, including both those that were already installed and those
/// that were built.
editables: Vec<ResolvedEditable>,
/// The temporary directory in which the built editables were stored.
#[allow(dead_code)]
temp_dir: Option<tempfile::TempDir>,
}
/// Resolve the set of editables that need to be installed.
#[allow(clippy::too_many_arguments)]
async fn resolve_editables(
editables: Vec<EditableRequirement>,
site_packages: &SitePackages<'_>,
reinstall: &Reinstall,
venv: &Virtualenv,
tags: &Tags,
cache: &Cache,
client: &RegistryClient,
build_dispatch: &BuildDispatch,
mut printer: Printer,
) -> Result<ResolvedEditables> {
// Partition the editables into those that are already installed, and those that must be built.
let mut installed = Vec::with_capacity(editables.len());
let mut uninstalled = Vec::with_capacity(editables.len());
for editable in editables {
match reinstall {
Reinstall::None => {
if let Some(dist) = site_packages.get_editable(editable.raw()) {
installed.push(dist.clone());
} else {
uninstalled.push(editable);
}
}
Reinstall::All => {
uninstalled.push(editable);
}
Reinstall::Packages(packages) => {
if let Some(dist) = site_packages.get_editable(editable.raw()) {
if packages.contains(dist.name()) {
uninstalled.push(editable);
} else {
installed.push(dist.clone());
}
} else {
uninstalled.push(editable);
}
}
}
}
// Build any editable installs.
let (built_editables, temp_dir) = if uninstalled.is_empty() {
(Vec::new(), None)
} else {
let start = std::time::Instant::now();
let temp_dir = tempfile::tempdir_in(venv.root())?;
let downloader = Downloader::new(cache, tags, client, build_dispatch)
.with_reporter(DownloadReporter::from(printer).with_length(uninstalled.len() as u64));
let local_editables: Vec<LocalEditable> = uninstalled
.iter()
.map(|editable| match editable {
EditableRequirement::Path { path, .. } => Ok(LocalEditable {
requirement: editable.clone(),
path: path.clone(),
}),
EditableRequirement::Url(_) => {
bail!("Editable installs for URLs are not yet supported");
}
})
.collect::<Result<_>>()?;
let built_editables: Vec<_> = downloader
.build_editables(local_editables, temp_dir.path())
.await
.context("Failed to build editables")?
.into_iter()
.collect();
let s = if built_editables.len() == 1 { "" } else { "s" };
writeln!(
printer,
"{}",
format!(
"Built {} in {}",
format!("{} editable{}", built_editables.len(), s).bold(),
elapsed(start.elapsed())
)
.dimmed()
)?;
(built_editables, Some(temp_dir))
};
Ok(ResolvedEditables {
editables: installed
.into_iter()
.map(ResolvedEditable::Installed)
.chain(built_editables.into_iter().map(ResolvedEditable::Built))
.collect::<Vec<_>>(),
temp_dir,
})
}

114
crates/puffin-cli/tests/pip_install.rs
View file

@ -550,3 +550,117 @@ fn install_editable() -> Result<()> {
Ok(())
}
#[test]
fn install_editable_and_registry() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
// Install the registry-based version of Black.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
.arg("black")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 13 packages in [TIME]
Downloaded 13 packages in [TIME]
Installed 13 packages in [TIME]
+ aiohttp==3.9.1
+ aiosignal==1.3.1
+ attrs==23.1.0
+ black==23.12.0
+ click==8.1.7
+ frozenlist==1.4.1
+ idna==3.6
+ multidict==6.0.4
+ mypy-extensions==1.0.0
+ packaging==23.2
+ pathspec==0.12.1
+ platformdirs==4.1.0
+ yarl==1.9.4
"###);
});
// Install the editable version of Black. This should remove the registry-based version.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
.arg("-e")
.arg("../../scripts/editable-installs/black_editable")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Built 1 editable in [TIME]
Resolved 1 package in [TIME]
Installed 1 package in [TIME]
- black==23.12.0
+ black @ ../../scripts/editable-installs/black_editable
"###);
});
// Re-install the registry-based version of Black. This should be a no-op, since we have a
// version of Black installed (the editable version) that satisfies the requirements.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
.arg("black")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Audited 1 package in [TIME]
"###);
});
// Re-install Black at a specific version. This should replace the editable version.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
.arg("black==23.10.0")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
- black==0.1.0
+ black==23.10.0
"###);
});
Ok(())
}

167
crates/puffin-cli/tests/pip_sync.rs
View file

@ -1903,9 +1903,7 @@ fn duplicate_package_overlap() -> Result<()> {
----- stderr -----
error: Failed to determine installation plan
Caused by: Detected duplicate package in requirements:
markupsafe ==2.1.2
markupsafe ==2.1.3
Caused by: Detected duplicate package in requirements: markupsafe
"###);
});
@ -2127,8 +2125,8 @@ fn sync_editable() -> Result<()> {
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Built 2 editables in [TIME]
Resolved 2 packages in [TIME]
Downloaded 2 packages in [TIME]
Installed 4 packages in [TIME]
+ boltons==23.1.1
@ -2226,3 +2224,164 @@ fn sync_editable() -> Result<()> {
Ok(())
}
#[test]
fn sync_editable_and_registry() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
// Install the registry-based version of Black.
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
black
"
})?;
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg(requirements_txt.path())
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
+ black==24.1a1
warning: The package `black` requires `click >=8.0.0`, but it's not installed.
warning: The package `black` requires `mypy-extensions >=0.4.3`, but it's not installed.
warning: The package `black` requires `packaging >=22.0`, but it's not installed.
warning: The package `black` requires `pathspec >=0.9.0`, but it's not installed.
warning: The package `black` requires `platformdirs >=2`, but it's not installed.
warning: The package `black` requires `aiohttp >=3.7.4 ; sys_platform != 'win32' or (implementation_name != 'pypy' and extra == 'd')`, but it's not installed.
"###);
});
// Install the editable version of Black. This should remove the registry-based version.
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
-e ../../scripts/editable-installs/black_editable
"
})?;
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg(requirements_txt.path())
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Built 1 editable in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- black==24.1a1
+ black @ ../../scripts/editable-installs/black_editable
"###);
});
// Re-install the registry-based version of Black. This should be a no-op, since we have a
// version of Black installed (the editable version) that satisfies the requirements.
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
black
"
})?;
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg(requirements_txt.path())
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Audited 1 package in [TIME]
"###);
});
// Re-install Black at a specific version. This should replace the editable version.
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
black==23.10.0
"
})?;
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg(requirements_txt.path())
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("CARGO_TARGET_DIR", "../../../target/target_install_editable"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- black==0.1.0
+ black==23.10.0
warning: The package `black` requires `click >=8.0.0`, but it's not installed.
warning: The package `black` requires `mypy-extensions >=0.4.3`, but it's not installed.
warning: The package `black` requires `packaging >=22.0`, but it's not installed.
warning: The package `black` requires `pathspec >=0.9.0`, but it's not installed.
warning: The package `black` requires `platformdirs >=2`, but it's not installed.
"###);
});
Ok(())
}

2
crates/puffin-client/src/registry_client.rs
View file

@ -100,7 +100,7 @@ impl RegistryClientBuilder {
}
/// A client for fetching packages from a `PyPI`-compatible index.
// TODO(konstin): Clean up the clients once we moved everything to common caching
// TODO(konstin): Clean up the clients once we moved everything to common caching.
#[derive(Debug, Clone)]
pub struct RegistryClient {
pub(crate) index_urls: IndexUrls,

12
crates/puffin-dispatch/src/lib.rs
View file

@ -16,7 +16,7 @@ use platform_tags::Tags;
use puffin_build::{SourceBuild, SourceBuildContext};
use puffin_cache::Cache;
use puffin_client::RegistryClient;
use puffin_installer::{Downloader, EditableMode, InstallPlan, Installer, Reinstall};
use puffin_installer::{Downloader, InstallPlan, Installer, Reinstall, SitePackages};
use puffin_interpreter::{Interpreter, Virtualenv};
use puffin_resolver::{Manifest, ResolutionOptions, Resolver};
use puffin_traits::{BuildContext, BuildKind, OnceMap};
@ -131,23 +131,27 @@ impl BuildContext for BuildDispatch {
venv.root().display(),
);
// Determine the current environment markers.
let tags = Tags::from_interpreter(&self.interpreter)?;
// Determine the set of installed packages.
let site_packages =
SitePackages::from_executable(venv).context("Failed to list installed packages")?;
let InstallPlan {
local,
remote,
reinstalls,
extraneous,
editables: _,
} = InstallPlan::from_requirements(
&resolution.requirements(),
&[],
Vec::new(),
site_packages,
&Reinstall::None,
&self.index_urls,
self.cache(),
venv,
&tags,
EditableMode::default(),
)?;
// Resolve any registry-based requirements.

52
crates/puffin-installer/src/editable.rs
View file

@ -1,9 +1,59 @@
use distribution_types::{CachedDist, LocalEditable};
use distribution_types::{CachedDist, InstalledDist, LocalEditable, Metadata, VersionOrUrl};
use puffin_normalize::PackageName;
use pypi_types::Metadata21;
/// An editable distribution that has been built.
#[derive(Debug, Clone)]
pub struct BuiltEditable {
pub editable: LocalEditable,
pub wheel: CachedDist,
pub metadata: Metadata21,
}
/// An editable distribution that has been resolved to a concrete distribution.
#[derive(Debug, Clone)]
#[allow(clippy::large_enum_variant)]
pub enum ResolvedEditable {
/// The editable is already installed in the environment.
Installed(InstalledDist),
/// The editable has been built and is ready to be installed.
Built(BuiltEditable),
}
impl Metadata for BuiltEditable {
fn name(&self) -> &PackageName {
&self.metadata.name
}
fn version_or_url(&self) -> VersionOrUrl {
VersionOrUrl::Version(&self.metadata.version)
}
}
impl Metadata for ResolvedEditable {
fn name(&self) -> &PackageName {
match self {
Self::Installed(dist) => dist.name(),
Self::Built(dist) => dist.name(),
}
}
fn version_or_url(&self) -> VersionOrUrl {
match self {
Self::Installed(dist) => dist.version_or_url(),
Self::Built(dist) => dist.version_or_url(),
}
}
}
impl std::fmt::Display for BuiltEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.version_or_url())
}
}
impl std::fmt::Display for ResolvedEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.version_or_url())
}
}

4
crates/puffin-installer/src/lib.rs
View file

@ -1,7 +1,7 @@
pub use downloader::{Downloader, Reporter as DownloadReporter};
pub use editable::BuiltEditable;
pub use editable::{BuiltEditable, ResolvedEditable};
pub use installer::{Installer, Reporter as InstallReporter};
pub use plan::{EditableMode, InstallPlan, Reinstall};
pub use plan::{InstallPlan, Reinstall};
pub use site_packages::SitePackages;
pub use uninstall::uninstall;

106
crates/puffin-installer/src/plan.rs
View file

@ -1,8 +1,8 @@
use std::hash::BuildHasherDefault;
use anyhow::{bail, Context, Result};
use rustc_hash::FxHashMap;
use tracing::debug;
use anyhow::{bail, Result};
use rustc_hash::FxHashSet;
use tracing::{debug, warn};
use distribution_types::direct_url::{git_reference, DirectUrl};
use distribution_types::{BuiltDist, Dist, SourceDist};
@ -14,9 +14,8 @@ use puffin_distribution::{BuiltWheelIndex, RegistryWheelIndex};
use puffin_interpreter::Virtualenv;
use puffin_normalize::PackageName;
use pypi_types::IndexUrls;
use requirements_txt::EditableRequirement;
use crate::SitePackages;
use crate::{ResolvedEditable, SitePackages};
#[derive(Debug, Default)]
pub struct InstallPlan {
@ -35,12 +34,6 @@ pub struct InstallPlan {
/// Any distributions that are already installed in the current environment, and are
/// _not_ necessary to satisfy the requirements.
pub extraneous: Vec<InstalledDist>,
/// Editable installs that are missing in the current environment.
///
/// Since editable installs happen from a path through a non-cacheable wheel, we don't have to
/// divide those into cached and non-cached.
pub editables: Vec<EditableRequirement>,
}
impl InstallPlan {
@ -49,18 +42,14 @@ impl InstallPlan {
#[allow(clippy::too_many_arguments)]
pub fn from_requirements(
requirements: &[Requirement],
editable_requirements: &[EditableRequirement],
editable_requirements: Vec<ResolvedEditable>,
mut site_packages: SitePackages,
reinstall: &Reinstall,
index_urls: &IndexUrls,
cache: &Cache,
venv: &Virtualenv,
tags: &Tags,
editable_mode: EditableMode,
) -> Result<Self> {
// Index all the already-installed packages in site-packages.
let mut site_packages =
SitePackages::from_executable(venv).context("Failed to list installed packages")?;
// Index all the already-downloaded wheels in the cache.
let mut registry_index = RegistryWheelIndex::new(cache, tags, index_urls);
@ -68,43 +57,44 @@ impl InstallPlan {
let mut remote = vec![];
let mut reinstalls = vec![];
let mut extraneous = vec![];
let mut editables = vec![];
let mut seen =
FxHashMap::with_capacity_and_hasher(requirements.len(), BuildHasherDefault::default());
FxHashSet::with_capacity_and_hasher(requirements.len(), BuildHasherDefault::default());
// Remove any editable requirements.
for editable in editable_requirements {
// Check if the package should be reinstalled. A reinstall involves (1) purging any
// cached distributions, and (2) marking any installed distributions as extraneous.
// For editables, we don't cache installations, so there's nothing to purge; and since
// editable installs lack a package name, we first lookup by URL, and then by name.
let reinstall = match reinstall {
Reinstall::None => false,
Reinstall::All => true,
Reinstall::Packages(packages) => site_packages
.get_editable(editable.raw())
.is_some_and(|distribution| packages.contains(distribution.name())),
};
for requirement in editable_requirements {
// If we see the same requirement twice, then we have a conflict.
if !seen.insert(requirement.name().clone()) {
bail!(
"Detected duplicate package in requirements: {}",
requirement.name()
);
}
if reinstall {
if let Some(distribution) = site_packages.remove_editable(editable.raw()) {
reinstalls.push(distribution);
}
editables.push(editable.clone());
} else {
if let Some(dist) = site_packages.remove_editable(editable.raw()) {
match editable_mode {
EditableMode::Immutable => {
debug!("Treating editable requirement as immutable: {editable}");
}
EditableMode::Mutable => {
debug!("Treating editable requirement as mutable: {editable}");
reinstalls.push(dist);
editables.push(editable.clone());
}
match requirement {
ResolvedEditable::Installed(installed) => {
debug!("Treating editable requirement as immutable: {installed}");
// Remove from the site-packages index, to avoid marking as extraneous.
let Some(editable) = installed.editable() else {
warn!("Editable requirement is not editable: {installed}");
continue;
};
if site_packages.remove_editable(editable).is_none() {
warn!("Editable requirement is not installed: {installed}");
continue;
}
} else {
editables.push(editable.clone());
}
ResolvedEditable::Built(built) => {
debug!("Treating editable requirement as mutable: {built}");
if let Some(dist) = site_packages.remove_editable(built.editable.raw()) {
// Remove any editable installs.
reinstalls.push(dist);
} else if let Some(dist) = site_packages.remove(built.name()) {
// Remove any non-editable installs of the same package.
reinstalls.push(dist);
}
local.push(built.wheel);
}
}
}
@ -116,8 +106,11 @@ impl InstallPlan {
}
// If we see the same requirement twice, then we have a conflict.
if let Some(existing) = seen.insert(requirement.name.clone(), requirement) {
bail!("Detected duplicate package in requirements:\n {requirement}\n {existing}");
if !seen.insert(requirement.name.clone()) {
bail!(
"Detected duplicate package in requirements: {}",
requirement.name
);
}
// Check if the package should be reinstalled. A reinstall involves (1) purging any
@ -328,7 +321,6 @@ impl InstallPlan {
remote,
reinstalls,
extraneous,
editables,
})
}
}
@ -362,13 +354,3 @@ impl Reinstall {
matches!(self, Self::None)
}
}
#[derive(Debug, Default, Copy, Clone)]
pub enum EditableMode {
/// Assume that editables are immutable, such that they're left untouched if already present
/// in the environment.
#[default]
Immutable,
/// Assume that editables are mutable, such that they're always reinstalled.
Mutable,
}

2
scripts/editable-installs/black_editable/black/__init__.py Normal file
View file

@ -0,0 +1,2 @@
def a():
pass

12
scripts/editable-installs/black_editable/pyproject.toml Normal file
View file

@ -0,0 +1,12 @@
[tool.poetry]
name = "black"
version = "0.1.0"
description = ""
authors = ["konstin <konstin@mailbox.org>"]
[tool.poetry.dependencies]
python = "^3.10"
[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"