Only use relative paths in lockfile (#6490)

For users who were using absolute paths in the `pyproject.toml`
previously, this is a behavior change: We now convert all absolute paths
in `path` entries to relative paths. Since i assume that no-one relies
on absolute path in their lockfiles - they are intended to be portable -
I'm tagging this as a bugfix.

Closes https://github.com/astral-sh/uv/pull/6438
Fixes https://github.com/astral-sh/uv/issues/6371
This commit is contained in:
Charlie Marsh 2024-08-23 22:19:10 -04:00 committed by GitHub
parent 611a9003c9
commit f7835243c5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 329 additions and 383 deletions

View file

@ -71,14 +71,12 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl {
ParsedUrl::Directory(ParsedDirectoryUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
lock_path: path.as_ref().to_path_buf(),
editable: false,
})
} else {
ParsedUrl::Path(ParsedPathUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
lock_path: path.as_ref().to_path_buf(),
ext: DistExtension::from_path(&path).map_err(|err| {
ParsedUrlError::MissingExtensionPath(path.as_ref().to_path_buf(), err)
})?,
@ -102,14 +100,12 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl {
ParsedUrl::Directory(ParsedDirectoryUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
lock_path: path.as_ref().to_path_buf(),
editable: false,
})
} else {
ParsedUrl::Path(ParsedPathUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
lock_path: path.as_ref().to_path_buf(),
ext: DistExtension::from_path(&path).map_err(|err| {
ParsedUrlError::MissingExtensionPath(path.as_ref().to_path_buf(), err)
})?,
@ -187,26 +183,16 @@ pub struct ParsedPathUrl {
pub url: Url,
/// 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
/// are resolved, and unlike the install path, we did not yet join it on the base directory.
pub lock_path: PathBuf,
/// The file extension, e.g. `tar.gz`, `zip`, etc.
pub ext: DistExtension,
}
impl ParsedPathUrl {
/// Construct a [`ParsedPathUrl`] from a path requirement source.
pub fn from_source(
install_path: PathBuf,
lock_path: PathBuf,
ext: DistExtension,
url: Url,
) -> Self {
pub fn from_source(install_path: PathBuf, ext: DistExtension, url: Url) -> Self {
Self {
url,
install_path,
lock_path,
ext,
}
}
@ -221,25 +207,15 @@ pub struct ParsedDirectoryUrl {
pub url: Url,
/// 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
/// are resolved, and unlike the install path, we did not yet join it on the base directory.
pub lock_path: PathBuf,
pub editable: bool,
}
impl ParsedDirectoryUrl {
/// Construct a [`ParsedDirectoryUrl`] from a path requirement source.
pub fn from_source(
install_path: PathBuf,
lock_path: PathBuf,
editable: bool,
url: Url,
) -> Self {
pub fn from_source(install_path: PathBuf, editable: bool, url: Url) -> Self {
Self {
url,
install_path,
lock_path,
editable,
}
}
@ -393,7 +369,6 @@ impl TryFrom<Url> for ParsedUrl {
Ok(Self::Directory(ParsedDirectoryUrl {
url,
install_path: path.clone(),
lock_path: path,
editable: false,
}))
} else {
@ -402,7 +377,6 @@ impl TryFrom<Url> for ParsedUrl {
ext: DistExtension::from_path(&path)
.map_err(|err| ParsedUrlError::MissingExtensionPath(path.clone(), err))?,
install_path: path.clone(),
lock_path: path,
}))
}
} else {

View file

@ -1,4 +1,5 @@
use std::fmt::{Display, Formatter};
use std::io;
use std::path::{Path, PathBuf};
use std::str::FromStr;
@ -10,7 +11,7 @@ use pep440_rs::VersionSpecifiers;
use pep508_rs::{
marker, MarkerEnvironment, MarkerTree, RequirementOrigin, VerbatimUrl, VersionOrUrl,
};
use uv_fs::{PortablePathBuf, CWD};
use uv_fs::{relative_to, PortablePathBuf, CWD};
use uv_git::{GitReference, GitSha, GitUrl};
use uv_normalize::{ExtraName, PackageName};
@ -105,6 +106,14 @@ impl Requirement {
_ => self,
}
}
/// Convert the requirement to a [`Requirement`] relative to the given path.
pub fn relative_to(self, path: &Path) -> Result<Self, io::Error> {
Ok(Self {
source: self.source.relative_to(path)?,
..self
})
}
}
impl From<Requirement> for pep508_rs::Requirement<VerbatimUrl> {
@ -175,28 +184,24 @@ impl From<Requirement> for pep508_rs::Requirement<VerbatimParsedUrl> {
}
RequirementSource::Path {
install_path,
lock_path,
ext,
url,
} => Some(VersionOrUrl::Url(VerbatimParsedUrl {
parsed_url: ParsedUrl::Path(ParsedPathUrl {
url: url.to_url(),
install_path,
lock_path,
ext,
}),
verbatim: url,
})),
RequirementSource::Directory {
install_path,
lock_path,
editable,
url,
} => Some(VersionOrUrl::Url(VerbatimParsedUrl {
parsed_url: ParsedUrl::Directory(ParsedDirectoryUrl {
url: url.to_url(),
install_path,
lock_path,
editable,
}),
verbatim: url,
@ -342,10 +347,6 @@ pub enum RequirementSource {
Path {
/// 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
/// are resolved, and unlike the install path, we did not yet join it on the base directory.
lock_path: PathBuf,
/// The file extension, e.g. `tar.gz`, `zip`, etc.
ext: DistExtension,
/// The PEP 508 style URL in the format
@ -357,10 +358,6 @@ pub enum RequirementSource {
Directory {
/// 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
/// are resolved, and unlike the install path, we did not yet join it on the base directory.
lock_path: PathBuf,
/// For a source tree (a directory), whether to install as an editable.
editable: bool,
/// The PEP 508 style URL in the format
@ -376,13 +373,11 @@ impl RequirementSource {
match parsed_url {
ParsedUrl::Path(local_file) => RequirementSource::Path {
install_path: local_file.install_path.clone(),
lock_path: local_file.lock_path.clone(),
ext: local_file.ext,
url,
},
ParsedUrl::Directory(directory) => RequirementSource::Directory {
install_path: directory.install_path.clone(),
lock_path: directory.lock_path.clone(),
editable: directory.editable,
url,
},
@ -427,13 +422,11 @@ impl RequirementSource {
}),
Self::Path {
install_path,
lock_path,
ext,
url,
} => Some(VerbatimParsedUrl {
parsed_url: ParsedUrl::Path(ParsedPathUrl::from_source(
install_path.clone(),
lock_path.clone(),
*ext,
url.to_url(),
)),
@ -441,13 +434,11 @@ impl RequirementSource {
}),
Self::Directory {
install_path,
lock_path,
editable,
url,
} => Some(VerbatimParsedUrl {
parsed_url: ParsedUrl::Directory(ParsedDirectoryUrl::from_source(
install_path.clone(),
lock_path.clone(),
*editable,
url.to_url(),
)),
@ -504,6 +495,33 @@ impl RequirementSource {
| RequirementSource::Directory { .. } => None,
}
}
/// Convert the source to a [`RequirementSource`] relative to the given path.
pub fn relative_to(self, path: &Path) -> Result<Self, io::Error> {
match self {
RequirementSource::Registry { .. }
| RequirementSource::Url { .. }
| RequirementSource::Git { .. } => Ok(self),
RequirementSource::Path {
install_path,
ext,
url,
} => Ok(Self::Path {
install_path: relative_to(&install_path, path)?,
ext,
url,
}),
RequirementSource::Directory {
install_path,
editable,
url,
} => Ok(Self::Directory {
install_path: relative_to(&install_path, path)?,
editable,
url,
}),
}
}
}
impl Display for RequirementSource {
@ -639,26 +657,24 @@ impl From<RequirementSource> for RequirementSourceWire {
}
}
RequirementSource::Path {
install_path: _,
lock_path,
install_path,
ext: _,
url: _,
} => Self::Path {
path: PortablePathBuf::from(lock_path),
path: PortablePathBuf::from(install_path),
},
RequirementSource::Directory {
install_path: _,
lock_path,
install_path,
editable,
url: _,
} => {
if editable {
Self::Editable {
editable: PortablePathBuf::from(lock_path),
editable: PortablePathBuf::from(install_path),
}
} else {
Self::Directory {
directory: PortablePathBuf::from(lock_path),
directory: PortablePathBuf::from(install_path),
}
}
}
@ -732,8 +748,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
Ok(Self::Path {
ext: DistExtension::from_path(path.as_path())
.map_err(|err| ParsedUrlError::MissingExtensionPath(path.clone(), err))?,
install_path: path.clone(),
lock_path: path,
install_path: path,
url,
})
}
@ -741,8 +756,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
let directory = PathBuf::from(directory);
let url = VerbatimUrl::parse_path(&directory, &*CWD)?;
Ok(Self::Directory {
install_path: directory.clone(),
lock_path: directory,
install_path: directory,
editable: false,
url,
})
@ -751,8 +765,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
let editable = PathBuf::from(editable);
let url = VerbatimUrl::parse_path(&editable, &*CWD)?;
Ok(Self::Directory {
install_path: editable.clone(),
lock_path: editable,
install_path: editable,
editable: true,
url,
})
@ -809,7 +822,6 @@ mod tests {
marker: MarkerTree::TRUE,
source: RequirementSource::Directory {
install_path: PathBuf::from(path),
lock_path: PathBuf::from(path),
editable: false,
url: VerbatimUrl::from_path(Path::new(path)).unwrap(),
},