From 6ba30a32a117459523adc808719c6dad3b2af608 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 23 Aug 2024 19:16:38 -0400 Subject: [PATCH] Don't canonicalize paths to user requirements --- crates/distribution-types/src/lib.rs | 43 ++++++++++++++++------------ crates/pep508-rs/src/verbatim_url.rs | 28 ++++++------------ crates/pypi-types/src/parsed_url.rs | 4 +-- crates/pypi-types/src/requirement.rs | 4 +-- crates/uv-installer/src/plan.rs | 38 +++++++++++++----------- 5 files changed, 58 insertions(+), 59 deletions(-) diff --git a/crates/distribution-types/src/lib.rs b/crates/distribution-types/src/lib.rs index b9db7d5fc2fb..471d49b7422c 100644 --- a/crates/distribution-types/src/lib.rs +++ b/crates/distribution-types/src/lib.rs @@ -42,6 +42,7 @@ use distribution_filename::{DistExtension, SourceDistExtension, WheelFilename}; use pep440_rs::Version; use pep508_rs::{Pep508Url, VerbatimUrl}; use pypi_types::{ParsedUrl, VerbatimParsedUrl}; +use uv_fs::normalize_absolute_path; use uv_git::GitUrl; use uv_normalize::PackageName; @@ -234,7 +235,7 @@ pub struct DirectUrlBuiltDist { #[derive(Debug, Clone, Hash)] pub struct PathBuiltDist { pub filename: WheelFilename, - /// The absolute, canonicalized path to the wheel which we use for installing. + /// The absolute path to the wheel which we use for installing. pub install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the wheel /// which we use for locking. Unlike `given` on the verbatim URL all environment variables @@ -295,7 +296,7 @@ pub struct GitSourceDist { #[derive(Debug, Clone, Hash)] pub struct PathSourceDist { pub name: PackageName, - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. pub install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables @@ -311,7 +312,7 @@ pub struct PathSourceDist { #[derive(Debug, Clone, Hash)] pub struct DirectorySourceDist { pub name: PackageName, - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. pub install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables @@ -371,14 +372,16 @@ impl Dist { lock_path: &Path, ext: DistExtension, ) -> Result { - // Store the canonicalized path, which also serves to validate that it exists. - let install_path = match install_path.canonicalize() { - Ok(path) => path, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Err(Error::NotFound(url.to_url())); - } - Err(err) => return Err(err.into()), - }; + // Convert to an absolute path. + let install_path = std::path::absolute(install_path)?; + + // Normalize the path. + let install_path = normalize_absolute_path(&install_path)?; + + // Validate that the path exists. + if !install_path.exists() { + return Err(Error::NotFound(url.to_url())); + } // Determine whether the path represents a built or source distribution. match ext { @@ -417,14 +420,16 @@ impl Dist { lock_path: &Path, editable: bool, ) -> Result { - // Store the canonicalized path, which also serves to validate that it exists. - let install_path = match install_path.canonicalize() { - Ok(path) => path, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Err(Error::NotFound(url.to_url())); - } - Err(err) => return Err(err.into()), - }; + // Convert to an absolute path. + let install_path = std::path::absolute(install_path)?; + + // Normalize the path. + let install_path = normalize_absolute_path(&install_path)?; + + // Validate that the path exists. + if !install_path.exists() { + return Err(Error::NotFound(url.to_url())); + } // Determine whether the path represents an archive or a directory. Ok(Self::Source(SourceDist::Directory(DirectorySourceDist { diff --git a/crates/pep508-rs/src/verbatim_url.rs b/crates/pep508-rs/src/verbatim_url.rs index 63d0c32dbe1c..11de14881fd4 100644 --- a/crates/pep508-rs/src/verbatim_url.rs +++ b/crates/pep508-rs/src/verbatim_url.rs @@ -9,7 +9,7 @@ use std::sync::LazyLock; use thiserror::Error; use url::{ParseError, Url}; -use uv_fs::{normalize_absolute_path, normalize_url_path, Simplified}; +use uv_fs::{normalize_absolute_path, normalize_url_path}; use crate::Pep508Url; @@ -46,12 +46,9 @@ impl VerbatimUrl { pub fn from_path(path: impl AsRef) -> Result { let path = path.as_ref(); - // Normalize the path (and canonicalize it, if possible). - let path = match path.simple_canonicalize() { - Ok(path) => path, - Err(_) => normalize_absolute_path(path) - .map_err(|err| VerbatimUrlError::Normalization(path.to_path_buf(), err))?, - }; + // Normalize the path. + let path = normalize_absolute_path(path) + .map_err(|err| VerbatimUrlError::Normalization(path.to_path_buf(), err))?; // Extract the fragment, if it exists. let (path, fragment) = split_fragment(&path); @@ -90,12 +87,8 @@ impl VerbatimUrl { base_dir.as_ref().join(path) }; - // Normalize the path (and canonicalize it, if possible). - let path = match path.simple_canonicalize() { - Ok(path) => path, - Err(_) => normalize_absolute_path(&path) - .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?, - }; + let path = normalize_absolute_path(&path) + .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?; // Extract the fragment, if it exists. let (path, fragment) = split_fragment(&path); @@ -123,12 +116,9 @@ impl VerbatimUrl { return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf())); }; - // Normalize the path (and canonicalize it, if possible). - let path = match path.simple_canonicalize() { - Ok(path) => path, - Err(_) => normalize_absolute_path(&path) - .map_err(|_| VerbatimUrlError::WorkingDirectory(path))?, - }; + // Normalize the path. + let path = normalize_absolute_path(&path) + .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?; // Extract the fragment, if it exists. let (path, fragment) = split_fragment(&path); diff --git a/crates/pypi-types/src/parsed_url.rs b/crates/pypi-types/src/parsed_url.rs index f39968ede1ba..4dca849ac944 100644 --- a/crates/pypi-types/src/parsed_url.rs +++ b/crates/pypi-types/src/parsed_url.rs @@ -185,7 +185,7 @@ impl ParsedUrl { #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)] pub struct ParsedPathUrl { pub url: Url, - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. pub install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables @@ -219,7 +219,7 @@ impl ParsedPathUrl { #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)] pub struct ParsedDirectoryUrl { pub url: Url, - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. pub install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables diff --git a/crates/pypi-types/src/requirement.rs b/crates/pypi-types/src/requirement.rs index b0d1c079dfd8..d03348721670 100644 --- a/crates/pypi-types/src/requirement.rs +++ b/crates/pypi-types/src/requirement.rs @@ -340,7 +340,7 @@ pub enum RequirementSource { /// be a binary distribution (a `.whl` file) or a source distribution archive (a `.zip` or /// `.tar.gz` file). Path { - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables @@ -355,7 +355,7 @@ pub enum RequirementSource { /// A local source tree (a directory with a pyproject.toml in, or a legacy /// source distribution with only a setup.py but non pyproject.toml in it). Directory { - /// The absolute, canonicalized path to the distribution which we use for installing. + /// The absolute path to the distribution which we use for installing. install_path: PathBuf, /// The absolute path or path relative to the workspace root pointing to the distribution /// which we use for locking. Unlike `given` on the verbatim URL all environment variables diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index d8b823d8dee5..804abc41d568 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -18,7 +18,7 @@ use uv_configuration::{BuildOptions, Reinstall}; use uv_distribution::{ BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex, }; -use uv_fs::Simplified; +use uv_fs::{normalize_absolute_path, Simplified}; use uv_git::GitUrl; use uv_python::PythonEnvironment; use uv_types::HashStrategy; @@ -268,14 +268,16 @@ impl<'a> Planner<'a> { install_path, lock_path, } => { - // Store the canonicalized path, which also serves to validate that it exists. - let install_path = match install_path.canonicalize() { - Ok(path) => path, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Err(Error::NotFound(url.to_url()).into()); - } - Err(err) => return Err(err.into()), - }; + // Convert to an absolute path. + let install_path = std::path::absolute(install_path)?; + + // Normalize the path. + let install_path = normalize_absolute_path(&install_path)?; + + // Validate that the path exists. + if !install_path.exists() { + return Err(Error::NotFound(url.to_url()).into()); + } let sdist = DirectorySourceDist { name: requirement.name.clone(), @@ -305,14 +307,16 @@ impl<'a> Planner<'a> { install_path, lock_path, } => { - // Store the canonicalized path, which also serves to validate that it exists. - let install_path = match install_path.canonicalize() { - Ok(path) => path, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Err(Error::NotFound(url.to_url()).into()); - } - Err(err) => return Err(err.into()), - }; + // Convert to an absolute path. + let install_path = std::path::absolute(install_path)?; + + // Normalize the path. + let install_path = normalize_absolute_path(&install_path)?; + + // Validate that the path exists. + if !install_path.exists() { + return Err(Error::NotFound(url.to_url()).into()); + } match ext { DistExtension::Wheel => {