diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index 4e5075963..5c291741e 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -96,7 +96,7 @@ impl std::fmt::Display for VersionOrUrl<'_> { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum InstalledVersion<'a> { /// A PEP 440 version specifier, used to identify a distribution in a registry. Version(&'a Version), diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 464f61d58..cb5169171 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -1,14 +1,15 @@ +use std::collections::hash_map::Entry; use std::hash::BuildHasherDefault; use std::io; use std::path::Path; use anyhow::{bail, Result}; -use rustc_hash::FxHashSet; +use rustc_hash::FxHashMap; use tracing::{debug, warn}; use distribution_types::{ BuiltDist, CachedDirectUrlDist, CachedDist, Dist, IndexLocations, InstalledDirectUrlDist, - InstalledDist, Name, SourceDist, + InstalledDist, InstalledMetadata, InstalledVersion, Name, SourceDist, }; use pep508_rs::{Requirement, VersionOrUrl}; use platform_tags::Tags; @@ -25,7 +26,7 @@ use crate::{ResolvedEditable, SitePackages}; #[derive(Debug)] pub struct Planner<'a> { requirements: &'a [Requirement], - editable_requirements: Vec, + editable_requirements: &'a [ResolvedEditable], } impl<'a> Planner<'a> { @@ -34,13 +35,13 @@ impl<'a> Planner<'a> { pub fn with_requirements(requirements: &'a [Requirement]) -> Self { Self { requirements, - editable_requirements: Vec::new(), + editable_requirements: &[], } } /// Set the editable requirements use in the [`Plan`]. #[must_use] - pub fn with_editable_requirements(self, editable_requirements: Vec) -> Self { + pub fn with_editable_requirements(self, editable_requirements: &'a [ResolvedEditable]) -> Self { Self { editable_requirements, ..self @@ -72,7 +73,7 @@ impl<'a> Planner<'a> { let mut remote = vec![]; let mut reinstalls = vec![]; let mut extraneous = vec![]; - let mut seen = FxHashSet::with_capacity_and_hasher( + let mut seen = FxHashMap::with_capacity_and_hasher( self.requirements.len(), BuildHasherDefault::default(), ); @@ -80,11 +81,20 @@ impl<'a> Planner<'a> { // Remove any editable requirements. for requirement in self.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() - ); + 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 { @@ -111,7 +121,7 @@ impl<'a> Planner<'a> { // Remove any non-editable installs of the same package. reinstalls.push(dist); } - local.push(built.wheel); + local.push(built.wheel.clone()); } } } @@ -123,11 +133,20 @@ impl<'a> Planner<'a> { } // 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 - ); + let specifier = Specifier::NonEditable(requirement.version_or_url.as_ref()); + 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); + } } // Check if the package should be reinstalled. A reinstall involves (1) purging any @@ -382,6 +401,14 @@ 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(Option<&'a VersionOrUrl>), +} + /// Returns `true` if the cache entry linked to the file at the given [`Path`] is not-modified. /// /// A cache entry is not modified if it exists and is newer than the file at the given path. diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index 54c68e5d4..6198a2465 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -506,7 +506,7 @@ async fn install( reinstalls, extraneous: _, } = Planner::with_requirements(&requirements) - .with_editable_requirements(editables) + .with_editable_requirements(&editables) .build( site_packages, reinstall, diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index ed6272fdc..9aea1c4ad 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -142,7 +142,7 @@ pub(crate) async fn pip_sync( reinstalls, extraneous, } = Planner::with_requirements(&requirements) - .with_editable_requirements(resolved_editables.editables) + .with_editable_requirements(&resolved_editables.editables) .build( site_packages, reinstall, diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 9eda54d5a..f363e633d 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2247,7 +2247,7 @@ fn sync_editable() -> Result<()> { "../../scripts/editable-installs/maturin_editable/python/maturin_editable/__init__.py"; let python_version_1 = indoc::indoc! {r" from .maturin_editable import * - + version = 1 "}; fs_err::write(python_source_file, python_version_1)?; @@ -2263,7 +2263,7 @@ fn sync_editable() -> Result<()> { // Edit the sources. let python_version_2 = indoc::indoc! {r" from .maturin_editable import * - + version = 2 "}; fs_err::write(python_source_file, python_version_2)?; @@ -2664,3 +2664,48 @@ fn offline() -> Result<()> { Ok(()) } + +/// Sync with a repeated `anyio` requirement. The second requirement should be ignored. +#[test] +fn repeat_requirement() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("anyio\nanyio")?; + + uv_snapshot!(command(&context) + .arg("requirements.in"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + anyio==4.2.0 + "###); + + Ok(()) +} + +/// Sync with a repeated, but conflicting `anyio` requirement. The second requirement should cause +/// an error. +#[test] +fn conflicting_requirement() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("anyio\nanyio==4.0.0")?; + + uv_snapshot!(command(&context) + .arg("requirements.in"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to determine installation plan + Caused by: Detected duplicate package in requirements: anyio + "###); + + Ok(()) +}