Don't canonicalize paths to user requirements (#6560)

This commit is contained in:
Charlie Marsh 2024-08-23 22:02:14 -04:00 committed by GitHub
parent 44e36a7e69
commit 611a9003c9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 58 additions and 59 deletions

View file

@ -42,6 +42,7 @@ use distribution_filename::{DistExtension, SourceDistExtension, WheelFilename};
use pep440_rs::Version; use pep440_rs::Version;
use pep508_rs::{Pep508Url, VerbatimUrl}; use pep508_rs::{Pep508Url, VerbatimUrl};
use pypi_types::{ParsedUrl, VerbatimParsedUrl}; use pypi_types::{ParsedUrl, VerbatimParsedUrl};
use uv_fs::normalize_absolute_path;
use uv_git::GitUrl; use uv_git::GitUrl;
use uv_normalize::PackageName; use uv_normalize::PackageName;
@ -234,7 +235,7 @@ pub struct DirectUrlBuiltDist {
#[derive(Debug, Clone, Hash)] #[derive(Debug, Clone, Hash)]
pub struct PathBuiltDist { pub struct PathBuiltDist {
pub filename: WheelFilename, 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, pub install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the wheel /// 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 /// 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)] #[derive(Debug, Clone, Hash)]
pub struct PathSourceDist { pub struct PathSourceDist {
pub name: PackageName, 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, pub install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// 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)] #[derive(Debug, Clone, Hash)]
pub struct DirectorySourceDist { pub struct DirectorySourceDist {
pub name: PackageName, 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, pub install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// which we use for locking. Unlike `given` on the verbatim URL all environment variables
@ -371,14 +372,16 @@ impl Dist {
lock_path: &Path, lock_path: &Path,
ext: DistExtension, ext: DistExtension,
) -> Result<Dist, Error> { ) -> Result<Dist, Error> {
// Store the canonicalized path, which also serves to validate that it exists. // Convert to an absolute path.
let install_path = match install_path.canonicalize() { let install_path = std::path::absolute(install_path)?;
Ok(path) => path,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // 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())); return Err(Error::NotFound(url.to_url()));
} }
Err(err) => return Err(err.into()),
};
// Determine whether the path represents a built or source distribution. // Determine whether the path represents a built or source distribution.
match ext { match ext {
@ -417,14 +420,16 @@ impl Dist {
lock_path: &Path, lock_path: &Path,
editable: bool, editable: bool,
) -> Result<Dist, Error> { ) -> Result<Dist, Error> {
// Store the canonicalized path, which also serves to validate that it exists. // Convert to an absolute path.
let install_path = match install_path.canonicalize() { let install_path = std::path::absolute(install_path)?;
Ok(path) => path,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // 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())); return Err(Error::NotFound(url.to_url()));
} }
Err(err) => return Err(err.into()),
};
// Determine whether the path represents an archive or a directory. // Determine whether the path represents an archive or a directory.
Ok(Self::Source(SourceDist::Directory(DirectorySourceDist { Ok(Self::Source(SourceDist::Directory(DirectorySourceDist {

View file

@ -9,7 +9,7 @@ use std::sync::LazyLock;
use thiserror::Error; use thiserror::Error;
use url::{ParseError, Url}; 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; use crate::Pep508Url;
@ -46,12 +46,9 @@ impl VerbatimUrl {
pub fn from_path(path: impl AsRef<Path>) -> Result<Self, VerbatimUrlError> { pub fn from_path(path: impl AsRef<Path>) -> Result<Self, VerbatimUrlError> {
let path = path.as_ref(); let path = path.as_ref();
// Normalize the path (and canonicalize it, if possible). // Normalize the path.
let path = match path.simple_canonicalize() { let path = normalize_absolute_path(path)
Ok(path) => path, .map_err(|err| VerbatimUrlError::Normalization(path.to_path_buf(), err))?;
Err(_) => normalize_absolute_path(path)
.map_err(|err| VerbatimUrlError::Normalization(path.to_path_buf(), err))?,
};
// Extract the fragment, if it exists. // Extract the fragment, if it exists.
let (path, fragment) = split_fragment(&path); let (path, fragment) = split_fragment(&path);
@ -90,12 +87,8 @@ impl VerbatimUrl {
base_dir.as_ref().join(path) base_dir.as_ref().join(path)
}; };
// Normalize the path (and canonicalize it, if possible). let path = normalize_absolute_path(&path)
let path = match path.simple_canonicalize() { .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?;
Ok(path) => path,
Err(_) => normalize_absolute_path(&path)
.map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?,
};
// Extract the fragment, if it exists. // Extract the fragment, if it exists.
let (path, fragment) = split_fragment(&path); let (path, fragment) = split_fragment(&path);
@ -123,12 +116,9 @@ impl VerbatimUrl {
return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf())); return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf()));
}; };
// Normalize the path (and canonicalize it, if possible). // Normalize the path.
let path = match path.simple_canonicalize() { let path = normalize_absolute_path(&path)
Ok(path) => path, .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?;
Err(_) => normalize_absolute_path(&path)
.map_err(|_| VerbatimUrlError::WorkingDirectory(path))?,
};
// Extract the fragment, if it exists. // Extract the fragment, if it exists.
let (path, fragment) = split_fragment(&path); let (path, fragment) = split_fragment(&path);

View file

@ -185,7 +185,7 @@ impl ParsedUrl {
#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub struct ParsedPathUrl { pub struct ParsedPathUrl {
pub url: Url, 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, pub install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// 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)] #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
pub struct ParsedDirectoryUrl { pub struct ParsedDirectoryUrl {
pub url: Url, 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, pub install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// which we use for locking. Unlike `given` on the verbatim URL all environment variables

View file

@ -340,7 +340,7 @@ pub enum RequirementSource {
/// be a binary distribution (a `.whl` file) or a source distribution archive (a `.zip` or /// be a binary distribution (a `.whl` file) or a source distribution archive (a `.zip` or
/// `.tar.gz` file). /// `.tar.gz` file).
Path { 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, install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// 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 /// 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). /// source distribution with only a setup.py but non pyproject.toml in it).
Directory { 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, install_path: PathBuf,
/// The absolute path or path relative to the workspace root pointing to the distribution /// 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 /// which we use for locking. Unlike `given` on the verbatim URL all environment variables

View file

@ -18,7 +18,7 @@ use uv_configuration::{BuildOptions, Reinstall};
use uv_distribution::{ use uv_distribution::{
BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex, BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex,
}; };
use uv_fs::Simplified; use uv_fs::{normalize_absolute_path, Simplified};
use uv_git::GitUrl; use uv_git::GitUrl;
use uv_python::PythonEnvironment; use uv_python::PythonEnvironment;
use uv_types::HashStrategy; use uv_types::HashStrategy;
@ -268,14 +268,16 @@ impl<'a> Planner<'a> {
install_path, install_path,
lock_path, lock_path,
} => { } => {
// Store the canonicalized path, which also serves to validate that it exists. // Convert to an absolute path.
let install_path = match install_path.canonicalize() { let install_path = std::path::absolute(install_path)?;
Ok(path) => path,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // 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()); return Err(Error::NotFound(url.to_url()).into());
} }
Err(err) => return Err(err.into()),
};
let sdist = DirectorySourceDist { let sdist = DirectorySourceDist {
name: requirement.name.clone(), name: requirement.name.clone(),
@ -305,14 +307,16 @@ impl<'a> Planner<'a> {
install_path, install_path,
lock_path, lock_path,
} => { } => {
// Store the canonicalized path, which also serves to validate that it exists. // Convert to an absolute path.
let install_path = match install_path.canonicalize() { let install_path = std::path::absolute(install_path)?;
Ok(path) => path,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // 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()); return Err(Error::NotFound(url.to_url()).into());
} }
Err(err) => return Err(err.into()),
};
match ext { match ext {
DistExtension::Wheel => { DistExtension::Wheel => {