Remove special-casing for editable requirements (#3869)

## Summary

There are a few behavior changes in here:

- We now enforce `--require-hashes` for editables, like pip. So if you
use `--require-hashes` with an editable requirement, we'll reject it. I
could change this if it seems off.
- We now treat source tree requirements, editable or not (e.g., both `-e
./black` and `./black`) as if `--refresh` is always enabled. This
doesn't mean that we _always_ rebuild them; but if you pass
`--reinstall`, then yes, we always rebuild them. I think this is an
improvement and is close to how editables work today.

Closes #3844.

Closes #2695.
This commit is contained in:
Charlie Marsh 2024-05-28 11:49:34 -04:00 committed by GitHub
parent 063a0a4384
commit 1fc6a59707
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
64 changed files with 583 additions and 1813 deletions

View file

@ -21,7 +21,6 @@ pep440_rs = { workspace = true }
pep508_rs = { workspace = true }
platform-tags = { workspace = true }
pypi-types = { workspace = true }
requirements-txt = { workspace = true }
uv-cache = { workspace = true }
uv-configuration = { workspace = true }
uv-distribution = { workspace = true }

View file

@ -1,24 +1,18 @@
use std::cmp::Reverse;
use std::path::Path;
use std::sync::Arc;
use futures::{stream::FuturesUnordered, FutureExt, Stream, StreamExt, TryFutureExt, TryStreamExt};
use futures::{stream::FuturesUnordered, FutureExt, Stream, TryFutureExt, TryStreamExt};
use pep508_rs::PackageName;
use tokio::task::JoinError;
use tracing::instrument;
use url::Url;
use distribution_types::{
BuildableSource, CachedDist, Dist, Hashed, Identifier, LocalEditable, LocalEditables,
RemoteSource,
};
use distribution_types::{BuildableSource, CachedDist, Dist, Hashed, Identifier, RemoteSource};
use platform_tags::Tags;
use uv_cache::Cache;
use uv_distribution::{DistributionDatabase, LocalWheel};
use uv_types::{BuildContext, HashStrategy, InFlight};
use crate::editable::BuiltEditable;
#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("Failed to unzip wheel: {0}")]
@ -115,55 +109,6 @@ impl<'a, Context: BuildContext> Downloader<'a, Context> {
Ok(wheels)
}
/// Build a set of editables
#[instrument(skip_all)]
pub async fn build_editables(
&self,
editables: LocalEditables,
editable_wheel_dir: &Path,
) -> Result<Vec<BuiltEditable>, Error> {
// Build editables in parallel
let mut results = Vec::with_capacity(editables.len());
let mut fetches = editables
.into_iter()
.map(|editable| async move {
let task_id = self
.reporter
.as_ref()
.map(|reporter| reporter.on_editable_build_start(&editable));
let (local_wheel, metadata) = self
.database
.build_wheel_editable(&editable, editable_wheel_dir)
.await
.map_err(Error::Editable)?;
let cached_dist = CachedDist::from(local_wheel);
if let Some(task_id) = task_id {
if let Some(reporter) = &self.reporter {
reporter.on_editable_build_complete(&editable, task_id);
}
}
Ok::<_, Error>((editable, cached_dist, metadata))
})
.collect::<FuturesUnordered<_>>();
while let Some((editable, wheel, metadata)) = fetches.next().await.transpose()? {
if let Some(reporter) = self.reporter.as_ref() {
reporter.on_progress(&wheel);
}
results.push(BuiltEditable {
editable,
wheel,
metadata,
});
}
if let Some(reporter) = self.reporter.as_ref() {
reporter.on_complete();
}
Ok(results)
}
/// Download, build, and unzip a single wheel.
#[instrument(skip_all, fields(name = % dist, size = ? dist.size(), url = dist.file().map(| file | file.url.to_string()).unwrap_or_default()))]
pub async fn get_wheel(&self, dist: Dist, in_flight: &InFlight) -> Result<CachedDist, Error> {
@ -241,12 +186,6 @@ pub trait Reporter: Send + Sync {
/// Callback to invoke when a source distribution build is complete.
fn on_build_complete(&self, source: &BuildableSource, id: usize);
/// Callback to invoke when a editable build is kicked off.
fn on_editable_build_start(&self, dist: &LocalEditable) -> usize;
/// Callback to invoke when a editable build is complete.
fn on_editable_build_complete(&self, dist: &LocalEditable, id: usize);
/// Callback to invoke when a repository checkout begins.
fn on_checkout_start(&self, url: &Url, rev: &str) -> usize;

View file

@ -1,158 +0,0 @@
use serde::Deserialize;
use std::path::Path;
use distribution_types::{
CachedDist, InstalledDist, InstalledMetadata, InstalledVersion, LocalEditable, Name,
};
use pypi_types::Metadata23;
use uv_normalize::PackageName;
/// An editable distribution that has been installed.
#[derive(Debug, Clone)]
pub struct InstalledEditable {
pub editable: LocalEditable,
pub wheel: InstalledDist,
pub metadata: Metadata23,
}
/// An editable distribution that has been built.
#[derive(Debug, Clone)]
pub struct BuiltEditable {
pub editable: LocalEditable,
pub wheel: CachedDist,
pub metadata: Metadata23,
}
/// 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(InstalledEditable),
/// The editable has been built and is ready to be installed.
Built(BuiltEditable),
}
impl ResolvedEditable {
/// Return the [`LocalEditable`] for the distribution.
pub fn local(&self) -> &LocalEditable {
match self {
Self::Installed(dist) => &dist.editable,
Self::Built(dist) => &dist.editable,
}
}
/// Return the [`Metadata23`] for the distribution.
pub fn metadata(&self) -> &Metadata23 {
match self {
Self::Installed(dist) => &dist.metadata,
Self::Built(dist) => &dist.metadata,
}
}
}
impl Name for InstalledEditable {
fn name(&self) -> &PackageName {
&self.metadata.name
}
}
impl Name for BuiltEditable {
fn name(&self) -> &PackageName {
&self.metadata.name
}
}
impl Name for ResolvedEditable {
fn name(&self) -> &PackageName {
match self {
Self::Installed(dist) => dist.name(),
Self::Built(dist) => dist.name(),
}
}
}
impl InstalledMetadata for InstalledEditable {
fn installed_version(&self) -> InstalledVersion {
self.wheel.installed_version()
}
}
impl InstalledMetadata for BuiltEditable {
fn installed_version(&self) -> InstalledVersion {
self.wheel.installed_version()
}
}
impl InstalledMetadata for ResolvedEditable {
fn installed_version(&self) -> InstalledVersion {
match self {
Self::Installed(dist) => dist.installed_version(),
Self::Built(dist) => dist.installed_version(),
}
}
}
impl std::fmt::Display for InstalledEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.installed_version())
}
}
impl std::fmt::Display for BuiltEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.installed_version())
}
}
impl std::fmt::Display for ResolvedEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.installed_version())
}
}
/// Returns `true` if the source tree at the given path contains dynamic metadata.
pub fn is_dynamic(path: &Path) -> bool {
// If there's no `pyproject.toml`, we assume it's dynamic.
let Ok(contents) = fs_err::read_to_string(path.join("pyproject.toml")) else {
return true;
};
let Ok(pyproject_toml) = toml::from_str::<PyProjectToml>(&contents) else {
return true;
};
// If `[project]` is not present, we assume it's dynamic.
let Some(project) = pyproject_toml.project else {
// ...unless it appears to be a Poetry project.
return pyproject_toml
.tool
.map_or(true, |tool| tool.poetry.is_none());
};
// `[project.dynamic]` must be present and non-empty.
project.dynamic.is_some_and(|dynamic| !dynamic.is_empty())
}
/// A pyproject.toml as specified in PEP 517.
#[derive(Deserialize, Debug)]
#[serde(rename_all = "kebab-case")]
struct PyProjectToml {
project: Option<Project>,
tool: Option<Tool>,
}
#[derive(Deserialize, Debug)]
#[serde(rename_all = "kebab-case")]
struct Project {
dynamic: Option<Vec<String>>,
}
#[derive(Deserialize, Debug)]
struct Tool {
poetry: Option<ToolPoetry>,
}
#[derive(Deserialize, Debug)]
struct ToolPoetry {
#[allow(dead_code)]
name: Option<String>,
}

View file

@ -1,6 +1,5 @@
pub use compile::{compile_tree, CompileError};
pub use downloader::{Downloader, Reporter as DownloadReporter};
pub use editable::{is_dynamic, BuiltEditable, InstalledEditable, ResolvedEditable};
pub use installer::{Installer, Reporter as InstallReporter};
pub use plan::{Plan, Planner};
pub use site_packages::{SatisfiesResult, SitePackages, SitePackagesDiagnostic};
@ -8,7 +7,7 @@ pub use uninstall::{uninstall, UninstallError};
mod compile;
mod downloader;
mod editable;
mod installer;
mod plan;
mod satisfies;

View file

@ -5,14 +5,13 @@ use std::str::FromStr;
use anyhow::{bail, Result};
use rustc_hash::FxHashMap;
use tracing::{debug, warn};
use tracing::debug;
use distribution_filename::WheelFilename;
use distribution_types::{
CachedDirectUrlDist, CachedDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist,
Error, GitSourceDist, Hashed, IndexLocations, InstalledDist, InstalledMetadata,
InstalledVersion, Name, PathBuiltDist, PathSourceDist, RemoteSource, Requirement,
RequirementSource, Verbatim,
Error, GitSourceDist, Hashed, IndexLocations, InstalledDist, Name, PathBuiltDist,
PathSourceDist, RemoteSource, Requirement, RequirementSource, Verbatim,
};
use platform_tags::Tags;
use uv_cache::{ArchiveTimestamp, Cache, CacheBucket, WheelCache};
@ -26,32 +25,18 @@ use uv_interpreter::PythonEnvironment;
use uv_types::HashStrategy;
use crate::satisfies::RequirementSatisfaction;
use crate::{ResolvedEditable, SitePackages};
use crate::SitePackages;
/// A planner to generate an [`Plan`] based on a set of requirements.
#[derive(Debug)]
pub struct Planner<'a> {
requirements: &'a [Requirement],
editable_requirements: &'a [ResolvedEditable],
}
impl<'a> Planner<'a> {
/// Set the requirements use in the [`Plan`].
#[must_use]
pub fn with_requirements(requirements: &'a [Requirement]) -> Self {
Self {
requirements,
editable_requirements: &[],
}
}
/// Set the editable requirements use in the [`Plan`].
#[must_use]
pub fn with_editable_requirements(self, editable_requirements: &'a [ResolvedEditable]) -> Self {
Self {
editable_requirements,
..self
}
pub fn new(requirements: &'a [Requirement]) -> Self {
Self { requirements }
}
/// Partition a set of requirements into those that should be linked from the cache, those that
@ -90,56 +75,6 @@ impl<'a> Planner<'a> {
BuildHasherDefault::default(),
);
// Remove any editable requirements.
for requirement in self.editable_requirements {
// If we see the same requirement twice, then we have a conflict.
let specifier = Specifier::Editable(requirement.installed_version());
match seen.entry(requirement.name().clone()) {
Entry::Occupied(value) => {
if value.get() == &specifier {
continue;
}
bail!(
"Detected duplicate package in requirements: {}",
requirement.name()
);
}
Entry::Vacant(entry) => {
entry.insert(specifier);
}
}
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.wheel.as_editable() else {
warn!("Editable requirement is not editable: {installed}");
continue;
};
let existing = site_packages.remove_editables(editable);
if existing.is_empty() {
warn!("Editable requirement is not installed: {installed}");
continue;
}
}
ResolvedEditable::Built(built) => {
debug!("Treating editable requirement as mutable: {built}");
// Remove any editables.
let existing = site_packages.remove_editables(built.editable.raw());
reinstalls.extend(existing);
// Remove any non-editable installs of the same package.
let existing = site_packages.remove_packages(built.name());
reinstalls.extend(existing);
cached.push(built.wheel.clone());
}
}
}
for requirement in self.requirements {
// Filter out incompatible requirements.
if !requirement.evaluate_markers(Some(venv.interpreter().markers()), &[]) {
@ -147,10 +82,9 @@ impl<'a> Planner<'a> {
}
// If we see the same requirement twice, then we have a conflict.
let specifier = Specifier::NonEditable(&requirement.source);
match seen.entry(requirement.name.clone()) {
Entry::Occupied(value) => {
if value.get() == &specifier {
if value.get() == &&requirement.source {
continue;
}
bail!(
@ -159,7 +93,7 @@ impl<'a> Planner<'a> {
);
}
Entry::Vacant(entry) => {
entry.insert(specifier);
entry.insert(&requirement.source);
}
}
@ -195,6 +129,9 @@ impl<'a> Planner<'a> {
RequirementSatisfaction::OutOfDate => {
debug!("Requirement installed, but not fresh: {distribution}");
}
RequirementSatisfaction::Dynamic => {
debug!("Requirement installed, but dynamic: {distribution}");
}
}
reinstalls.push(distribution.clone());
}
@ -332,7 +269,7 @@ impl<'a> Planner<'a> {
continue;
}
}
RequirementSource::Path { url, .. } => {
RequirementSource::Path { url, editable, .. } => {
// Store the canonicalized path, which also serves to validate that it exists.
let path = match url
.to_file_path()
@ -352,7 +289,7 @@ impl<'a> Planner<'a> {
name: requirement.name.clone(),
url: url.clone(),
path,
editable: false,
editable: *editable,
};
// Find the most-compatible wheel from the cache, since we don't know
@ -478,14 +415,6 @@ impl<'a> Planner<'a> {
}
}
#[derive(Debug, PartialEq, Eq)]
enum Specifier<'a> {
/// An editable requirement, marked by the installed version of the package.
Editable(InstalledVersion<'a>),
/// A non-editable requirement, marked by the version or URL specifier.
NonEditable(&'a RequirementSource),
}
#[derive(Debug, Default)]
pub struct Plan {
/// The distributions that are not already installed in the current environment, but are

View file

@ -1,8 +1,11 @@
use anyhow::Result;
use cache_key::{CanonicalUrl, RepositoryUrl};
use std::fmt::Debug;
use std::path::Path;
use anyhow::Result;
use serde::Deserialize;
use tracing::{debug, trace};
use cache_key::{CanonicalUrl, RepositoryUrl};
use distribution_types::{InstalledDirectUrlDist, InstalledDist, RequirementSource};
use pypi_types::{DirInfo, DirectUrl, VcsInfo, VcsKind};
use uv_cache::{ArchiveTarget, ArchiveTimestamp};
@ -12,6 +15,7 @@ pub(crate) enum RequirementSatisfaction {
Mismatch,
Satisfied,
OutOfDate,
Dynamic,
}
impl RequirementSatisfaction {
@ -187,9 +191,59 @@ impl RequirementSatisfaction {
return Ok(Self::OutOfDate);
}
// Does the package have dynamic metadata?
if is_dynamic(path) {
return Ok(Self::Dynamic);
}
// Otherwise, assume the requirement is up-to-date.
Ok(Self::Satisfied)
}
}
}
}
/// Returns `true` if the source tree at the given path contains dynamic metadata.
fn is_dynamic(path: &Path) -> bool {
// If there's no `pyproject.toml`, we assume it's dynamic.
let Ok(contents) = fs_err::read_to_string(path.join("pyproject.toml")) else {
return true;
};
let Ok(pyproject_toml) = toml::from_str::<PyProjectToml>(&contents) else {
return true;
};
// If `[project]` is not present, we assume it's dynamic.
let Some(project) = pyproject_toml.project else {
// ...unless it appears to be a Poetry project.
return pyproject_toml
.tool
.map_or(true, |tool| tool.poetry.is_none());
};
// `[project.dynamic]` must be present and non-empty.
project.dynamic.is_some_and(|dynamic| !dynamic.is_empty())
}
/// A pyproject.toml as specified in PEP 517.
#[derive(Deserialize, Debug)]
#[serde(rename_all = "kebab-case")]
struct PyProjectToml {
project: Option<Project>,
tool: Option<Tool>,
}
#[derive(Deserialize, Debug)]
#[serde(rename_all = "kebab-case")]
struct Project {
dynamic: Option<Vec<String>>,
}
#[derive(Deserialize, Debug)]
struct Tool {
poetry: Option<ToolPoetry>,
}
#[derive(Deserialize, Debug)]
struct ToolPoetry {
#[allow(dead_code)]
name: Option<String>,
}

View file

@ -13,13 +13,10 @@ use distribution_types::{
};
use pep440_rs::{Version, VersionSpecifiers};
use pypi_types::VerbatimParsedUrl;
use requirements_txt::EditableRequirement;
use uv_cache::{ArchiveTarget, ArchiveTimestamp};
use uv_interpreter::PythonEnvironment;
use uv_normalize::PackageName;
use uv_types::InstalledPackagesProvider;
use crate::is_dynamic;
use crate::satisfies::RequirementSatisfaction;
/// An index over the packages installed in an environment.
@ -289,7 +286,6 @@ impl SitePackages {
pub fn satisfies(
&self,
requirements: &[UnresolvedRequirementSpecification],
editables: &[EditableRequirement],
constraints: &[Requirement],
) -> Result<SatisfiesResult> {
let mut stack = Vec::with_capacity(requirements.len());
@ -308,58 +304,6 @@ impl SitePackages {
}
}
// Verify that all editable requirements are met.
for requirement in editables {
let installed = self.get_editables(requirement.raw());
match installed.as_slice() {
[] => {
// The package isn't installed.
return Ok(SatisfiesResult::Unsatisfied(requirement.to_string()));
}
[distribution] => {
// Is the editable out-of-date?
if !ArchiveTimestamp::up_to_date_with(
&requirement.path,
ArchiveTarget::Install(distribution),
)? {
return Ok(SatisfiesResult::Unsatisfied(requirement.to_string()));
}
// Does the editable have dynamic metadata?
if is_dynamic(&requirement.path) {
return Ok(SatisfiesResult::Unsatisfied(requirement.to_string()));
}
// Recurse into the dependencies.
let metadata = distribution
.metadata()
.with_context(|| format!("Failed to read metadata for: {distribution}"))?;
// Add the dependencies to the queue.
for dependency in metadata.requires_dist {
if dependency.evaluate_markers(
self.venv.interpreter().markers(),
&requirement.extras,
) {
let dependency = UnresolvedRequirementSpecification {
requirement: UnresolvedRequirement::Named(Requirement::from(
dependency,
)),
hashes: vec![],
};
if seen.insert(dependency.clone()) {
stack.push(dependency);
}
}
}
}
_ => {
// There are multiple installed distributions for the same package.
return Ok(SatisfiesResult::Unsatisfied(requirement.to_string()));
}
}
}
// Verify that all non-editable requirements are met.
while let Some(entry) = stack.pop() {
let installed = match &entry.requirement {
@ -378,7 +322,9 @@ impl SitePackages {
distribution,
entry.requirement.source().as_ref(),
)? {
RequirementSatisfaction::Mismatch | RequirementSatisfaction::OutOfDate => {
RequirementSatisfaction::Mismatch
| RequirementSatisfaction::OutOfDate
| RequirementSatisfaction::Dynamic => {
return Ok(SatisfiesResult::Unsatisfied(entry.requirement.to_string()))
}
RequirementSatisfaction::Satisfied => {}
@ -387,7 +333,8 @@ impl SitePackages {
for constraint in constraints {
match RequirementSatisfaction::check(distribution, &constraint.source)? {
RequirementSatisfaction::Mismatch
| RequirementSatisfaction::OutOfDate => {
| RequirementSatisfaction::OutOfDate
| RequirementSatisfaction::Dynamic => {
return Ok(SatisfiesResult::Unsatisfied(
entry.requirement.to_string(),
))