From 5021f3f5c895819907ca60011ac1e108c1842c8d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 23 Aug 2024 23:23:10 -0400 Subject: [PATCH 1/4] Use relative paths for --find-links and local registries --- crates/distribution-types/src/index_url.rs | 6 +- crates/uv-resolver/src/lock.rs | 348 +++++++++++++++++---- crates/uv/tests/lock.rs | 24 +- 3 files changed, 299 insertions(+), 79 deletions(-) diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index 6ffc6a1a2bd9..15cd574a0fee 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -112,7 +112,8 @@ impl FromStr for IndexUrl { type Err = IndexUrlError; fn from_str(s: &str) -> Result { - let url = if let Ok(path) = Path::new(s).canonicalize() { + let path = Path::new(s); + let url = if path.exists() { VerbatimUrl::from_path(path)? } else { VerbatimUrl::parse_url(s)? @@ -247,7 +248,8 @@ impl FromStr for FlatIndexLocation { type Err = IndexUrlError; fn from_str(s: &str) -> Result { - let url = if let Ok(path) = Path::new(s).canonicalize() { + let path = Path::new(s); + let url = if path.exists() { VerbatimUrl::from_path(path)? } else { VerbatimUrl::parse_url(s)? diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index e29e434651f6..c7215183a663 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -1128,7 +1128,7 @@ impl Package { ) -> Result { let id = PackageId::from_annotated_dist(annotated_dist, root)?; let sdist = SourceDist::from_annotated_dist(&id, annotated_dist)?; - let wheels = Wheel::from_annotated_dist(annotated_dist)?; + let wheels = Wheel::from_annotated_dist(annotated_dist, root)?; let requires_dist = if id.source.is_immutable() { BTreeSet::default() } else { @@ -1245,7 +1245,20 @@ impl Package { let wheels = self .wheels .iter() - .map(|wheel| wheel.to_registry_dist(url.to_url())) + .map(|wheel| wheel.to_remote_registry_dist(url.to_url())) + .collect::>()?; + let reg_built_dist = RegistryBuiltDist { + wheels, + best_wheel_index, + sdist: None, + }; + Ok(Dist::Built(BuiltDist::Registry(reg_built_dist))) + } + Source::Local(path) => { + let wheels = self + .wheels + .iter() + .map(|wheel| wheel.to_local_registry_dist(path, workspace_root)) .collect::>()?; let reg_built_dist = RegistryBuiltDist { wheels, @@ -1295,7 +1308,7 @@ impl Package { } .into()), }; - } + }; if let Some(sdist) = self.to_source_dist(workspace_root)? { return Ok(Dist::Source(sdist)); @@ -1418,6 +1431,51 @@ impl Package { }); let index = IndexUrl::Url(VerbatimUrl::from_url(url.to_url())); + let reg_dist = RegistrySourceDist { + name: self.id.name.clone(), + version: self.id.version.clone(), + file, + ext, + index, + wheels: vec![], + }; + distribution_types::SourceDist::Registry(reg_dist) + } + Source::Local(path) => { + let Some(ref sdist) = self.sdist else { + return Ok(None); + }; + + let file_path = sdist.path().ok_or_else(|| LockErrorKind::MissingPath { + name: self.id.name.clone(), + version: self.id.version.clone(), + })?; + let file_path = workspace_root.join(path).join(file_path); + let file_url = Url::from_file_path(&file_path).unwrap(); + let filename = sdist + .filename() + .ok_or_else(|| LockErrorKind::MissingFilename { + id: self.id.clone(), + })?; + let ext = SourceDistExtension::from_path(filename.as_ref())?; + let file = Box::new(distribution_types::File { + dist_info_metadata: false, + filename: filename.to_string(), + hashes: sdist + .hash() + .map(|hash| vec![hash.0.clone()]) + .unwrap_or_default(), + requires_python: None, + size: sdist.size(), + upload_time_utc_ms: None, + url: FileLocation::AbsoluteUrl(UrlString::from(file_url)), + yanked: None, + }); + let index = IndexUrl::Path( + VerbatimUrl::from_path(workspace_root.join(path)) + .map_err(LockErrorKind::RegistryVerbatimUrl)?, + ); + let reg_dist = RegistrySourceDist { name: self.id.name.clone(), version: self.id.version.clone(), @@ -1836,12 +1894,23 @@ impl From for PackageIdForDependency { #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, serde::Deserialize)] #[serde(try_from = "SourceWire")] enum Source { + /// A remote registry of `--find-links` index. Registry(UrlString), + /// A Git repository. Git(UrlString, GitSource), + /// A direct HTTP(S) URL. Direct(UrlString, DirectSource), + /// A path to a local source or built archive. Path(PathBuf), + /// A path to a local directory. Directory(PathBuf), + /// A path to a local directory that should be installed as editable. Editable(PathBuf), + /// A local registry of `--find-links` index. + /// + /// STOPSHIP(charlie): We should just use `Registry` for this, and have serialization that + /// allows either a URL or a path. + Local(PathBuf), } impl Source { @@ -1862,7 +1931,7 @@ impl Source { fn from_built_dist(built_dist: &BuiltDist, root: &Path) -> Result { match *built_dist { - BuiltDist::Registry(ref reg_dist) => Ok(Source::from_registry_built_dist(reg_dist)), + BuiltDist::Registry(ref reg_dist) => Source::from_registry_built_dist(reg_dist, root), BuiltDist::DirectUrl(ref direct_dist) => { Ok(Source::from_direct_built_dist(direct_dist)) } @@ -1876,7 +1945,7 @@ impl Source { ) -> Result { match *source_dist { distribution_types::SourceDist::Registry(ref reg_dist) => { - Ok(Source::from_registry_source_dist(reg_dist)) + Source::from_registry_source_dist(reg_dist, root) } distribution_types::SourceDist::DirectUrl(ref direct_dist) => { Ok(Source::from_direct_source_dist(direct_dist)) @@ -1893,12 +1962,18 @@ impl Source { } } - fn from_registry_built_dist(reg_dist: &RegistryBuiltDist) -> Source { - Source::from_index_url(®_dist.best_wheel().index) + fn from_registry_built_dist( + reg_dist: &RegistryBuiltDist, + root: &Path, + ) -> Result { + Source::from_index_url(®_dist.best_wheel().index, root) } - fn from_registry_source_dist(reg_dist: &RegistrySourceDist) -> Source { - Source::from_index_url(®_dist.index) + fn from_registry_source_dist( + reg_dist: &RegistrySourceDist, + root: &Path, + ) -> Result { + Source::from_index_url(®_dist.index, root) } fn from_direct_built_dist(direct_dist: &DirectUrlBuiltDist) -> Source { @@ -1947,10 +2022,23 @@ impl Source { } } - fn from_index_url(index_url: &IndexUrl) -> Source { - // Remove any sensitive credentials from the index URL. - let redacted = index_url.redacted(); - Source::Registry(UrlString::from(redacted.as_ref())) + fn from_index_url(index_url: &IndexUrl, root: &Path) -> Result { + match index_url { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + // Remove any sensitive credentials from the index URL. + let redacted = index_url.redacted(); + Ok(Source::Registry(UrlString::from(redacted.as_ref()))) + } + IndexUrl::Path(url) => { + let path = relative_to( + url.to_file_path() + .expect("Path registry should be a file path"), + root, + ) + .map_err(LockErrorKind::IndexRelativePath)?; + Ok(Source::Local(path)) + } + } } fn from_git_dist(git_dist: &GitSourceDist) -> Source { @@ -2010,6 +2098,9 @@ impl Source { Value::from(PortablePath::from(path).to_string()), ); } + Source::Local(ref path) => { + source_table.insert("local", Value::from(PortablePath::from(path).to_string())); + } } table.insert("source", value(source_table)); } @@ -2021,7 +2112,10 @@ impl std::fmt::Display for Source { Source::Registry(url) | Source::Git(url, _) | Source::Direct(url, _) => { write!(f, "{}+{}", self.name(), url) } - Source::Path(path) | Source::Directory(path) | Source::Editable(path) => { + Source::Path(path) + | Source::Directory(path) + | Source::Editable(path) + | Source::Local(path) => { write!(f, "{}+{}", self.name(), PortablePath::from(path)) } } @@ -2037,6 +2131,7 @@ impl Source { Self::Path(..) => "path", Self::Directory(..) => "directory", Self::Editable(..) => "editable", + Self::Local(..) => "local", } } @@ -2049,7 +2144,7 @@ impl Source { /// Returns `None` to indicate that the source kind _may_ include a hash. fn requires_hash(&self) -> Option { match *self { - Self::Registry(..) => None, + Self::Registry(..) | Self::Local(..) => None, Self::Direct(..) | Self::Path(..) => Some(true), Self::Git(..) | Self::Directory(..) | Self::Editable(..) => Some(false), } @@ -2079,6 +2174,9 @@ enum SourceWire { Editable { editable: PortablePathBuf, }, + Local { + local: PortablePathBuf, + }, } impl TryFrom for Source { @@ -2115,6 +2213,7 @@ impl TryFrom for Source { Path { path } => Ok(Source::Path(path.into())), Directory { directory } => Ok(Source::Directory(directory.into())), Editable { editable } => Ok(Source::Editable(editable.into())), + Local { local } => Ok(Source::Local(local.into())), } } } @@ -2223,6 +2322,13 @@ impl SourceDist { } } + fn path(&self) -> Option<&Path> { + match &self { + SourceDist::Url { .. } => None, + SourceDist::Path { path, .. } => Some(path), + } + } + fn hash(&self) -> Option<&Hash> { match &self { SourceDist::Url { metadata, .. } => metadata.hash.as_ref(), @@ -2304,15 +2410,31 @@ impl SourceDist { return Ok(None); } - let url = normalize_file_location(®_dist.file.url) - .map_err(LockErrorKind::InvalidFileUrl) - .map_err(LockError::from)?; - let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from); - let size = reg_dist.file.size; - Ok(Some(SourceDist::Url { - url, - metadata: SourceDistMetadata { hash, size }, - })) + match ®_dist.index { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + let url = normalize_file_location(®_dist.file.url) + .map_err(LockErrorKind::InvalidFileUrl) + .map_err(LockError::from)?; + let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from); + let size = reg_dist.file.size; + Ok(Some(SourceDist::Url { + url, + metadata: SourceDistMetadata { hash, size }, + })) + } + IndexUrl::Path(path) => { + let index_path = path.to_file_path().unwrap(); + let reg_dist_path = reg_dist.file.url.to_url().unwrap().to_file_path().unwrap(); + let path = relative_to(reg_dist_path, index_path) + .map_err(LockErrorKind::DistributionRelativePath)?; + let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from); + let size = reg_dist.file.size; + Ok(Some(SourceDist::Path { + path, + metadata: SourceDistMetadata { hash, size }, + })) + } + } } fn from_direct_dist( @@ -2495,12 +2617,15 @@ struct Wheel { } impl Wheel { - fn from_annotated_dist(annotated_dist: &AnnotatedDist) -> Result, LockError> { + fn from_annotated_dist( + annotated_dist: &AnnotatedDist, + root: &Path, + ) -> Result, LockError> { match annotated_dist.dist { // We pass empty installed packages for locking. ResolvedDist::Installed(_) => unreachable!(), ResolvedDist::Installable(ref dist) => { - Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index()) + Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index(), root) } } } @@ -2509,9 +2634,10 @@ impl Wheel { dist: &Dist, hashes: &[HashDigest], index: Option<&IndexUrl>, + root: &Path, ) -> Result, LockError> { match *dist { - Dist::Built(ref built_dist) => Wheel::from_built_dist(built_dist, hashes, index), + Dist::Built(ref built_dist) => Wheel::from_built_dist(built_dist, hashes, index, root), Dist::Source(distribution_types::SourceDist::Registry(ref source_dist)) => source_dist .wheels .iter() @@ -2530,13 +2656,16 @@ impl Wheel { built_dist: &BuiltDist, hashes: &[HashDigest], index: Option<&IndexUrl>, + root: &Path, ) -> Result, LockError> { match *built_dist { BuiltDist::Registry(ref reg_dist) => Wheel::from_registry_dist(reg_dist, index), BuiltDist::DirectUrl(ref direct_dist) => { Ok(vec![Wheel::from_direct_dist(direct_dist, hashes)]) } - BuiltDist::Path(ref path_dist) => Ok(vec![Wheel::from_path_dist(path_dist, hashes)]), + BuiltDist::Path(ref path_dist) => { + Ok(vec![Wheel::from_path_dist(path_dist, hashes, root)?]) + } } } @@ -2558,17 +2687,33 @@ impl Wheel { fn from_registry_wheel(wheel: &RegistryBuiltWheel) -> Result { let filename = wheel.filename.clone(); - let url = normalize_file_location(&wheel.file.url) - .map_err(LockErrorKind::InvalidFileUrl) - .map_err(LockError::from)?; - let hash = wheel.file.hashes.iter().max().cloned().map(Hash::from); - let size = wheel.file.size; - Ok(Wheel { - url: WheelWireSource::Url { url }, - hash, - size, - filename, - }) + match &wheel.index { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + let url = normalize_file_location(&wheel.file.url) + .map_err(LockErrorKind::InvalidFileUrl) + .map_err(LockError::from)?; + let hash = wheel.file.hashes.iter().max().cloned().map(Hash::from); + let size = wheel.file.size; + Ok(Wheel { + url: WheelWireSource::Url { url }, + hash, + size, + filename, + }) + } + IndexUrl::Path(path) => { + let index_path = path.to_file_path().unwrap(); + let wheel_path = wheel.file.url.to_url().unwrap().to_file_path().unwrap(); + let path = relative_to(wheel_path, index_path) + .map_err(LockErrorKind::DistributionRelativePath)?; + Ok(Wheel { + url: WheelWireSource::Path { path }, + hash: None, + size: None, + filename, + }) + } + } } fn from_direct_dist(direct_dist: &DirectUrlBuiltDist, hashes: &[HashDigest]) -> Wheel { @@ -2582,28 +2727,26 @@ impl Wheel { } } - fn from_path_dist(path_dist: &PathBuiltDist, hashes: &[HashDigest]) -> Wheel { - Wheel { - url: WheelWireSource::Filename { - filename: path_dist.filename.clone(), - }, + fn from_path_dist( + path_dist: &PathBuiltDist, + hashes: &[HashDigest], + root: &Path, + ) -> Result { + let path = relative_to(&path_dist.install_path, root) + .map_err(LockErrorKind::DistributionRelativePath)?; + Ok(Wheel { + url: WheelWireSource::Path { path }, hash: hashes.iter().max().cloned().map(Hash::from), size: None, filename: path_dist.filename.clone(), - } + }) } - fn to_registry_dist(&self, url: Url) -> Result { + fn to_remote_registry_dist(&self, index_url: Url) -> Result { let filename: WheelFilename = self.filename.clone(); - let url_string = match &self.url { - WheelWireSource::Url { url } => url.clone(), - WheelWireSource::Filename { .. } => { - return Err(LockErrorKind::MissingUrl { - name: self.filename.name.clone(), - version: self.filename.version.clone(), - } - .into()) - } + let file_url = match &self.url { + WheelWireSource::Url { url } => url, + WheelWireSource::Path { .. } => unreachable!(), }; let file = Box::new(distribution_types::File { dist_info_metadata: false, @@ -2612,10 +2755,43 @@ impl Wheel { requires_python: None, size: self.size, upload_time_utc_ms: None, - url: FileLocation::AbsoluteUrl(url_string), + url: FileLocation::AbsoluteUrl(file_url.clone()), yanked: None, }); - let index = IndexUrl::Url(VerbatimUrl::from_url(url)); + let index = IndexUrl::Url(VerbatimUrl::from_url(index_url)); + Ok(RegistryBuiltWheel { + filename, + file, + index, + }) + } + + fn to_local_registry_dist( + &self, + index_path: &Path, + root: &Path, + ) -> Result { + let filename: WheelFilename = self.filename.clone(); + let file_path = match &self.url { + WheelWireSource::Path { path } => path, + WheelWireSource::Url { .. } => unreachable!(), + }; + let file_path = root.join(index_path).join(file_path); + let file_url = Url::from_file_path(&file_path).unwrap(); + let file = Box::new(distribution_types::File { + dist_info_metadata: false, + filename: filename.to_string(), + hashes: self.hash.iter().map(|h| h.0.clone()).collect(), + requires_python: None, + size: self.size, + upload_time_utc_ms: None, + url: FileLocation::AbsoluteUrl(UrlString::from(file_url)), + yanked: None, + }); + let index = IndexUrl::Path( + VerbatimUrl::from_path(root.join(index_path)) + .map_err(LockErrorKind::RegistryVerbatimUrl)?, + ); Ok(RegistryBuiltWheel { filename, file, @@ -2652,12 +2828,14 @@ enum WheelWireSource { url: UrlString, }, /// Used for path wheels. - /// - /// We only store the filename for path wheel, since we can't store a relative path in the url - Filename { - /// We duplicate the filename since a lot of code relies on having the filename on the - /// wheel entry. - filename: WheelFilename, + Path { + /// The filename of the wheel. + /// + /// This isn't part of the wire format since it's redundant with the + /// URL. But we do use it for various things, and thus compute it at + /// deserialization time. Not being able to extract a wheel filename from a + /// wheel URL is thus a deserialization error. + path: PathBuf, }, } @@ -2669,8 +2847,8 @@ impl Wheel { WheelWireSource::Url { url } => { table.insert("url", Value::from(url.as_ref())); } - WheelWireSource::Filename { filename } => { - table.insert("filename", Value::from(filename.to_string())); + WheelWireSource::Path { path } => { + table.insert("path", Value::from(PortablePath::from(path).to_string())); } } if let Some(ref hash) = self.hash { @@ -2687,7 +2865,6 @@ impl TryFrom for Wheel { type Error = String; fn try_from(wire: WheelWire) -> Result { - // If necessary, extract the filename from the URL. let filename = match &wire.url { WheelWireSource::Url { url } => { let filename = url.filename().map_err(|err| err.to_string())?; @@ -2695,7 +2872,17 @@ impl TryFrom for Wheel { format!("failed to parse `{filename}` as wheel filename: {err}") })? } - WheelWireSource::Filename { filename } => filename.clone(), + WheelWireSource::Path { path } => { + let filename = path + .file_name() + .and_then(|file_name| file_name.to_str()) + .ok_or_else(|| { + format!("path `{}` has no filename component", path.display()) + })?; + filename.parse::().map_err(|err| { + format!("failed to parse `{filename}` as wheel filename: {err}") + })? + } }; Ok(Wheel { @@ -3098,8 +3285,8 @@ enum LockErrorKind { /// The kind of the invalid source. source_type: &'static str, }, - /// An error that occurs when a distribution indicates that it is sourced from a registry, but - /// is missing a URL. + /// An error that occurs when a distribution indicates that it is sourced from a remote + /// registry, but is missing a URL. #[error("found registry distribution {name}=={version} without a valid URL")] MissingUrl { /// The name of the distribution that is missing a URL. @@ -3107,6 +3294,15 @@ enum LockErrorKind { /// The version of the distribution that is missing a URL. version: Version, }, + /// An error that occurs when a distribution indicates that it is sourced from a local registry, + /// but is missing a path. + #[error("found local registry distribution {name}=={version} without a valid path")] + MissingPath { + /// The name of the distribution that is missing a URL. + name: PackageName, + /// The version of the distribution that is missing a URL. + version: Version, + }, /// An error that occurs when a distribution indicates that it is sourced from a registry, but /// is missing a filename. #[error("found registry distribution {id} without a valid filename")] @@ -3137,6 +3333,13 @@ enum LockErrorKind { #[source] std::io::Error, ), + /// An error that occurs when converting an index URL to a relative path + #[error("could not compute relative path between workspace and index")] + IndexRelativePath( + /// The inner error we forward. + #[source] + std::io::Error, + ), /// An error that occurs when an ambiguous `package.dependency` is /// missing a `version` field. #[error( @@ -3171,6 +3374,13 @@ enum LockErrorKind { #[source] VerbatimUrlError, ), + /// An error that occurs when parsing an registry's index URL. + #[error("could not convert between URL and path")] + RegistryVerbatimUrl( + /// The inner error we forward. + #[source] + VerbatimUrlError, + ), } /// An error that occurs when a source string could not be parsed. diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index 71b8bffba003..18e83ea41534 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -6541,9 +6541,9 @@ fn lock_find_links_local_wheel() -> Result<()> { [[package]] name = "tqdm" version = "1000.0.0" - source = { registry = "file://[WORKSPACE]/scripts/links" } + source = { local = "../../../../../../workspace/puffin/scripts/links" } wheels = [ - { url = "file://[WORKSPACE]/scripts/links/tqdm-1000.0.0-py3-none-any.whl" }, + { path = "tqdm-1000.0.0-py3-none-any.whl" }, ] "### ); @@ -6630,8 +6630,8 @@ fn lock_find_links_local_sdist() -> Result<()> { [[package]] name = "tqdm" version = "999.0.0" - source = { registry = "file://[WORKSPACE]/scripts/links" } - sdist = { url = "file://[WORKSPACE]/scripts/links/tqdm-999.0.0.tar.gz" } + source = { local = "../../../../../../workspace/puffin/scripts/links" } + sdist = { path = "tqdm-999.0.0.tar.gz" } "### ); }); @@ -6855,6 +6855,14 @@ fn lock_local_index() -> Result<()> { let tqdm = root.child("tqdm"); fs_err::create_dir_all(&tqdm)?; + let wheel = tqdm.child("tqdm-1000.0.0-py3-none-any.whl"); + fs_err::copy( + context + .workspace_root + .join("scripts/links/tqdm-1000.0.0-py3-none-any.whl"), + &wheel, + )?; + let index = tqdm.child("index.html"); index.write_str(&formatdoc! {r#" @@ -6865,14 +6873,14 @@ fn lock_local_index() -> Result<()> {

Links for tqdm

tqdm-1000.0.0-py3-none-any.whl - "#, Url::from_directory_path(context.workspace_root.join("scripts/links/")).unwrap().as_str()})?; + "#, Url::from_file_path(wheel).unwrap().as_str()})?; let context = TestContext::new("3.12"); @@ -6929,9 +6937,9 @@ fn lock_local_index() -> Result<()> { [[package]] name = "tqdm" version = "1000.0.0" - source = { registry = "file://[TMP]" } + source = { local = "../../[TMP]/simple-html" } wheels = [ - { url = "file://[WORKSPACE]/scripts/links//tqdm-1000.0.0-py3-none-any.whl" }, + { path = "tqdm/tqdm-1000.0.0-py3-none-any.whl" }, ] "### ); From 70f6efe47d50da594eb97de6210896c7639d99cb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 24 Aug 2024 13:46:15 -0400 Subject: [PATCH 2/4] Use a single type --- crates/distribution-types/src/file.rs | 20 +- crates/distribution-types/src/index_url.rs | 10 +- crates/requirements-txt/src/lib.rs | 15 +- crates/uv-resolver/src/lock.rs | 448 +++++++++++------- ...r__lock__tests__hash_optional_missing.snap | 12 +- ...r__lock__tests__hash_optional_present.snap | 12 +- ...missing_dependency_source_unambiguous.snap | 30 +- ...dependency_source_version_unambiguous.snap | 30 +- ...issing_dependency_version_unambiguous.snap | 30 +- crates/uv/src/commands/project/lock.rs | 10 +- crates/uv/tests/lock.rs | 209 +++++++- 11 files changed, 593 insertions(+), 233 deletions(-) diff --git a/crates/distribution-types/src/file.rs b/crates/distribution-types/src/file.rs index f55b75e13f51..427e72dc6083 100644 --- a/crates/distribution-types/src/file.rs +++ b/crates/distribution-types/src/file.rs @@ -143,8 +143,6 @@ impl Display for FileLocation { PartialOrd, Ord, Hash, - Serialize, - Deserialize, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize, @@ -153,6 +151,24 @@ impl Display for FileLocation { #[archive_attr(derive(Debug))] pub struct UrlString(String); +impl serde::Serialize for UrlString { + fn serialize(&self, serializer: S) -> Result + where + S: serde::ser::Serializer, + { + String::serialize(&self.0, serializer) + } +} + +impl<'de> serde::de::Deserialize<'de> for UrlString { + fn deserialize(deserializer: D) -> Result + where + D: serde::de::Deserializer<'de>, + { + String::deserialize(deserializer).map(UrlString) + } +} + impl UrlString { /// Converts a [`UrlString`] to a [`Url`]. pub fn to_url(&self) -> Url { diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index 15cd574a0fee..bb6d5e481821 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -112,9 +112,8 @@ impl FromStr for IndexUrl { type Err = IndexUrlError; fn from_str(s: &str) -> Result { - let path = Path::new(s); - let url = if path.exists() { - VerbatimUrl::from_path(path)? + let url = if Path::new(s).exists() { + VerbatimUrl::from_path(std::path::absolute(s)?)? } else { VerbatimUrl::parse_url(s)? }; @@ -248,9 +247,8 @@ impl FromStr for FlatIndexLocation { type Err = IndexUrlError; fn from_str(s: &str) -> Result { - let path = Path::new(s); - let url = if path.exists() { - VerbatimUrl::from_path(path)? + let url = if Path::new(s).exists() { + VerbatimUrl::from_path(std::path::absolute(s)?)? } else { VerbatimUrl::parse_url(s)? }; diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index 3d00cb485494..4cdd35f7f3a1 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -484,7 +484,10 @@ fn parse_entry( } else if s.eat_if("-i") || s.eat_if("--index-url") { let given = parse_value(content, s, |c: char| !['\n', '\r', '#'].contains(&c))?; let expanded = expand_env_vars(given); - let url = if let Ok(path) = Path::new(expanded.as_ref()).canonicalize() { + let url = if let Some(path) = std::path::absolute(expanded.as_ref()) + .ok() + .filter(|path| path.exists()) + { VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { source: err, url: given.to_string(), @@ -505,7 +508,10 @@ fn parse_entry( } else if s.eat_if("--extra-index-url") { let given = parse_value(content, s, |c: char| !['\n', '\r', '#'].contains(&c))?; let expanded = expand_env_vars(given); - let url = if let Ok(path) = Path::new(expanded.as_ref()).canonicalize() { + let url = if let Some(path) = std::path::absolute(expanded.as_ref()) + .ok() + .filter(|path| path.exists()) + { VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { source: err, url: given.to_string(), @@ -528,7 +534,10 @@ fn parse_entry( } else if s.eat_if("--find-links") || s.eat_if("-f") { let given = parse_value(content, s, |c: char| !['\n', '\r', '#'].contains(&c))?; let expanded = expand_env_vars(given); - let url = if let Ok(path) = Path::new(expanded.as_ref()).canonicalize() { + let url = if let Some(path) = std::path::absolute(expanded.as_ref()) + .ok() + .filter(|path| path.exists()) + { VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { source: err, url: given.to_string(), diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index c7215183a663..66014249aada 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -23,7 +23,7 @@ use distribution_types::{ UrlString, }; use pep440_rs::Version; -use pep508_rs::{MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError}; +use pep508_rs::{split_scheme, MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError}; use platform_tags::{TagCompatibility, TagPriority, Tags}; use pypi_types::{ redact_git_credentials, HashDigest, ParsedArchiveUrl, ParsedGitUrl, Requirement, @@ -762,12 +762,51 @@ impl Lock { } // Collect the set of available indexes (both `--index-url` and `--find-links` entries). - let indexes = indexes.map(|locations| { + let remotes = indexes.map(|locations| { locations .indexes() - .map(IndexUrl::redacted) - .chain(locations.flat_index().map(FlatIndexLocation::redacted)) - .map(UrlString::from) + .filter_map(|index_url| match index_url { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => { + Some(UrlString::from(index_url.redacted())) + } + IndexUrl::Path(_) => None, + }) + .chain( + locations + .flat_index() + .filter_map(|index_url| match index_url { + FlatIndexLocation::Url(_) => { + Some(UrlString::from(index_url.redacted())) + } + FlatIndexLocation::Path(_) => None, + }), + ) + .collect::>() + }); + + let locals = indexes.map(|locations| { + locations + .indexes() + .filter_map(|index_url| match index_url { + IndexUrl::Pypi(_) | IndexUrl::Url(_) => None, + IndexUrl::Path(index_url) => { + let path = index_url.to_file_path().ok()?; + let path = relative_to(&path, workspace.install_path()).ok()?; + Some(path) + } + }) + .chain( + locations + .flat_index() + .filter_map(|index_url| match index_url { + FlatIndexLocation::Url(_) => None, + FlatIndexLocation::Path(index_url) => { + let path = index_url.to_file_path().ok()?; + let path = relative_to(&path, workspace.install_path()).ok()?; + Some(path) + } + }), + ) .collect::>() }); @@ -789,16 +828,29 @@ impl Lock { while let Some(package) = queue.pop_front() { // If the lockfile references an index that was not provided, we can't validate it. if let Source::Registry(index) = &package.id.source { - if indexes - .as_ref() - .is_some_and(|indexes| !indexes.contains(index)) - { - return Ok(SatisfiesResult::MissingIndex( - &package.id.name, - &package.id.version, - index, - )); - } + match index { + RegistrySource::Url(url) => { + if remotes + .as_ref() + .is_some_and(|remotes| !remotes.contains(url)) + { + return Ok(SatisfiesResult::MissingRemoteIndex( + &package.id.name, + &package.id.version, + url, + )); + } + } + RegistrySource::Path(path) => { + if locals.as_ref().is_some_and(|locals| !locals.contains(path)) { + return Ok(SatisfiesResult::MissingLocalIndex( + &package.id.name, + &package.id.version, + path, + )); + } + } + }; } // If the package is immutable, we don't need to validate it (or its dependencies). @@ -935,8 +987,10 @@ pub enum SatisfiesResult<'lock> { MismatchedOverrides(BTreeSet, BTreeSet), /// The lockfile is missing a workspace member. MissingRoot(PackageName), - /// The lockfile referenced an index that was not provided - MissingIndex(&'lock PackageName, &'lock Version, &'lock UrlString), + /// The lockfile referenced a remote index that was not provided + MissingRemoteIndex(&'lock PackageName, &'lock Version, &'lock UrlString), + /// The lockfile referenced a local index that was not provided + MissingLocalIndex(&'lock PackageName, &'lock Version, &'lock PathBuf), /// The resolver failed to generate metadata for a given package. MissingMetadata(&'lock PackageName, &'lock Version), /// A package in the lockfile contains different `requires-dist` metadata than expected. @@ -1128,7 +1182,7 @@ impl Package { ) -> Result { let id = PackageId::from_annotated_dist(annotated_dist, root)?; let sdist = SourceDist::from_annotated_dist(&id, annotated_dist)?; - let wheels = Wheel::from_annotated_dist(annotated_dist, root)?; + let wheels = Wheel::from_annotated_dist(annotated_dist)?; let requires_dist = if id.source.is_immutable() { BTreeSet::default() } else { @@ -1241,24 +1295,11 @@ impl Package { fn to_dist(&self, workspace_root: &Path, tags: &Tags) -> Result { if let Some(best_wheel_index) = self.find_best_wheel(tags) { return match &self.id.source { - Source::Registry(url) => { - let wheels = self - .wheels - .iter() - .map(|wheel| wheel.to_remote_registry_dist(url.to_url())) - .collect::>()?; - let reg_built_dist = RegistryBuiltDist { - wheels, - best_wheel_index, - sdist: None, - }; - Ok(Dist::Built(BuiltDist::Registry(reg_built_dist))) - } - Source::Local(path) => { + Source::Registry(source) => { let wheels = self .wheels .iter() - .map(|wheel| wheel.to_local_registry_dist(path, workspace_root)) + .map(|wheel| wheel.to_registry_dist(source, workspace_root)) .collect::>()?; let reg_built_dist = RegistryBuiltDist { wheels, @@ -1401,7 +1442,7 @@ impl Package { }; distribution_types::SourceDist::DirectUrl(direct_dist) } - Source::Registry(url) => { + Source::Registry(RegistrySource::Url(url)) => { let Some(ref sdist) = self.sdist else { return Ok(None); }; @@ -1441,7 +1482,7 @@ impl Package { }; distribution_types::SourceDist::Registry(reg_dist) } - Source::Local(path) => { + Source::Registry(RegistrySource::Path(path)) => { let Some(ref sdist) = self.sdist else { return Ok(None); }; @@ -1450,8 +1491,8 @@ impl Package { name: self.id.name.clone(), version: self.id.version.clone(), })?; - let file_path = workspace_root.join(path).join(file_path); - let file_url = Url::from_file_path(&file_path).unwrap(); + let file_url = Url::from_file_path(workspace_root.join(path).join(file_path)) + .map_err(|()| LockErrorKind::PathToUrl)?; let filename = sdist .filename() .ok_or_else(|| LockErrorKind::MissingFilename { @@ -1655,14 +1696,6 @@ impl Package { self.fork_markers.as_slice() } - /// Return the index URL for this package, if it is a registry source. - pub fn index(&self) -> Option<&UrlString> { - match &self.id.source { - Source::Registry(url) => Some(url), - _ => None, - } - } - /// Returns all the hashes associated with this [`Package`]. fn hashes(&self) -> Vec { let mut hashes = Vec::new(); @@ -1894,8 +1927,8 @@ impl From for PackageIdForDependency { #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, serde::Deserialize)] #[serde(try_from = "SourceWire")] enum Source { - /// A remote registry of `--find-links` index. - Registry(UrlString), + /// A registry or `--find-links` index. + Registry(RegistrySource), /// A Git repository. Git(UrlString, GitSource), /// A direct HTTP(S) URL. @@ -1906,11 +1939,6 @@ enum Source { Directory(PathBuf), /// A path to a local directory that should be installed as editable. Editable(PathBuf), - /// A local registry of `--find-links` index. - /// - /// STOPSHIP(charlie): We should just use `Registry` for this, and have serialization that - /// allows either a URL or a path. - Local(PathBuf), } impl Source { @@ -2027,16 +2055,17 @@ impl Source { IndexUrl::Pypi(_) | IndexUrl::Url(_) => { // Remove any sensitive credentials from the index URL. let redacted = index_url.redacted(); - Ok(Source::Registry(UrlString::from(redacted.as_ref()))) + let source = RegistrySource::Url(UrlString::from(redacted.as_ref())); + Ok(Source::Registry(source)) } IndexUrl::Path(url) => { let path = relative_to( - url.to_file_path() - .expect("Path registry should be a file path"), + url.to_file_path().map_err(|()| LockErrorKind::UrlToPath)?, root, ) .map_err(LockErrorKind::IndexRelativePath)?; - Ok(Source::Local(path)) + let source = RegistrySource::Path(path); + Ok(Source::Registry(source)) } } } @@ -2071,9 +2100,17 @@ impl Source { fn to_toml(&self, table: &mut Table) { let mut source_table = InlineTable::new(); match *self { - Source::Registry(ref url) => { - source_table.insert("registry", Value::from(url.as_ref())); - } + Source::Registry(ref source) => match source { + RegistrySource::Url(url) => { + source_table.insert("registry", Value::from(url.as_ref())); + } + RegistrySource::Path(path) => { + source_table.insert( + "registry", + Value::from(PortablePath::from(path).to_string()), + ); + } + }, Source::Git(ref url, _) => { source_table.insert("git", Value::from(url.as_ref())); } @@ -2098,9 +2135,6 @@ impl Source { Value::from(PortablePath::from(path).to_string()), ); } - Source::Local(ref path) => { - source_table.insert("local", Value::from(PortablePath::from(path).to_string())); - } } table.insert("source", value(source_table)); } @@ -2109,13 +2143,15 @@ impl Source { impl std::fmt::Display for Source { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { - Source::Registry(url) | Source::Git(url, _) | Source::Direct(url, _) => { + Source::Registry(RegistrySource::Url(url)) + | Source::Git(url, _) + | Source::Direct(url, _) => { write!(f, "{}+{}", self.name(), url) } - Source::Path(path) + Source::Registry(RegistrySource::Path(path)) + | Source::Path(path) | Source::Directory(path) - | Source::Editable(path) - | Source::Local(path) => { + | Source::Editable(path) => { write!(f, "{}+{}", self.name(), PortablePath::from(path)) } } @@ -2131,7 +2167,6 @@ impl Source { Self::Path(..) => "path", Self::Directory(..) => "directory", Self::Editable(..) => "editable", - Self::Local(..) => "local", } } @@ -2144,7 +2179,7 @@ impl Source { /// Returns `None` to indicate that the source kind _may_ include a hash. fn requires_hash(&self) -> Option { match *self { - Self::Registry(..) | Self::Local(..) => None, + Self::Registry(..) => None, Self::Direct(..) | Self::Path(..) => Some(true), Self::Git(..) | Self::Directory(..) | Self::Editable(..) => Some(false), } @@ -2155,7 +2190,7 @@ impl Source { #[serde(untagged)] enum SourceWire { Registry { - registry: UrlString, + registry: RegistrySource, }, Git { git: String, @@ -2174,9 +2209,6 @@ enum SourceWire { Editable { editable: PortablePathBuf, }, - Local { - local: PortablePathBuf, - }, } impl TryFrom for Source { @@ -2213,11 +2245,68 @@ impl TryFrom for Source { Path { path } => Ok(Source::Path(path.into())), Directory { directory } => Ok(Source::Directory(directory.into())), Editable { editable } => Ok(Source::Editable(editable.into())), - Local { local } => Ok(Source::Local(local.into())), } } } +/// The source for a registry, which could be a URL or a relative path. +#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] +enum RegistrySource { + /// Ex) `https://pypi.org/simple` + Url(UrlString), + /// Ex) `../path/to/local/index` + Path(PathBuf), +} + +impl std::fmt::Display for RegistrySource { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + RegistrySource::Url(url) => write!(f, "{url}"), + RegistrySource::Path(path) => write!(f, "{}", path.display()), + } + } +} + +impl<'de> serde::de::Deserialize<'de> for RegistrySource { + fn deserialize(deserializer: D) -> Result + where + D: serde::de::Deserializer<'de>, + { + struct Visitor; + + impl<'de> serde::de::Visitor<'de> for Visitor { + type Value = RegistrySource; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a valid URL or a file path") + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + if split_scheme(value).is_some() { + Ok( + serde::Deserialize::deserialize(serde::de::value::StrDeserializer::new( + value, + )) + .map(RegistrySource::Url)?, + ) + } else { + Ok( + serde::Deserialize::deserialize(serde::de::value::StrDeserializer::new( + value, + )) + .map(RegistrySource::Path)?, + ) + } + } + } + + deserializer.deserialize_str(Visitor) + } +} + #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, serde::Deserialize)] struct DirectSource { subdirectory: Option, @@ -2423,8 +2512,14 @@ impl SourceDist { })) } IndexUrl::Path(path) => { - let index_path = path.to_file_path().unwrap(); - let reg_dist_path = reg_dist.file.url.to_url().unwrap().to_file_path().unwrap(); + let index_path = path.to_file_path().map_err(|()| LockErrorKind::UrlToPath)?; + let reg_dist_path = reg_dist + .file + .url + .to_url() + .map_err(LockErrorKind::InvalidFileUrl)? + .to_file_path() + .map_err(|()| LockErrorKind::UrlToPath)?; let path = relative_to(reg_dist_path, index_path) .map_err(LockErrorKind::DistributionRelativePath)?; let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from); @@ -2617,15 +2712,12 @@ struct Wheel { } impl Wheel { - fn from_annotated_dist( - annotated_dist: &AnnotatedDist, - root: &Path, - ) -> Result, LockError> { + fn from_annotated_dist(annotated_dist: &AnnotatedDist) -> Result, LockError> { match annotated_dist.dist { // We pass empty installed packages for locking. ResolvedDist::Installed(_) => unreachable!(), ResolvedDist::Installable(ref dist) => { - Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index(), root) + Wheel::from_dist(dist, &annotated_dist.hashes, annotated_dist.index()) } } } @@ -2634,10 +2726,9 @@ impl Wheel { dist: &Dist, hashes: &[HashDigest], index: Option<&IndexUrl>, - root: &Path, ) -> Result, LockError> { match *dist { - Dist::Built(ref built_dist) => Wheel::from_built_dist(built_dist, hashes, index, root), + Dist::Built(ref built_dist) => Wheel::from_built_dist(built_dist, hashes, index), Dist::Source(distribution_types::SourceDist::Registry(ref source_dist)) => source_dist .wheels .iter() @@ -2656,16 +2747,13 @@ impl Wheel { built_dist: &BuiltDist, hashes: &[HashDigest], index: Option<&IndexUrl>, - root: &Path, ) -> Result, LockError> { match *built_dist { BuiltDist::Registry(ref reg_dist) => Wheel::from_registry_dist(reg_dist, index), BuiltDist::DirectUrl(ref direct_dist) => { Ok(vec![Wheel::from_direct_dist(direct_dist, hashes)]) } - BuiltDist::Path(ref path_dist) => { - Ok(vec![Wheel::from_path_dist(path_dist, hashes, root)?]) - } + BuiltDist::Path(ref path_dist) => Ok(vec![Wheel::from_path_dist(path_dist, hashes)]), } } @@ -2702,8 +2790,14 @@ impl Wheel { }) } IndexUrl::Path(path) => { - let index_path = path.to_file_path().unwrap(); - let wheel_path = wheel.file.url.to_url().unwrap().to_file_path().unwrap(); + let index_path = path.to_file_path().map_err(|()| LockErrorKind::UrlToPath)?; + let wheel_path = wheel + .file + .url + .to_url() + .map_err(LockErrorKind::InvalidFileUrl)? + .to_file_path() + .map_err(|()| LockErrorKind::UrlToPath)?; let path = relative_to(wheel_path, index_path) .map_err(LockErrorKind::DistributionRelativePath)?; Ok(Wheel { @@ -2727,76 +2821,87 @@ impl Wheel { } } - fn from_path_dist( - path_dist: &PathBuiltDist, - hashes: &[HashDigest], - root: &Path, - ) -> Result { - let path = relative_to(&path_dist.install_path, root) - .map_err(LockErrorKind::DistributionRelativePath)?; - Ok(Wheel { - url: WheelWireSource::Path { path }, + fn from_path_dist(path_dist: &PathBuiltDist, hashes: &[HashDigest]) -> Wheel { + Wheel { + url: WheelWireSource::Filename { + filename: path_dist.filename.clone(), + }, hash: hashes.iter().max().cloned().map(Hash::from), size: None, filename: path_dist.filename.clone(), - }) - } - - fn to_remote_registry_dist(&self, index_url: Url) -> Result { - let filename: WheelFilename = self.filename.clone(); - let file_url = match &self.url { - WheelWireSource::Url { url } => url, - WheelWireSource::Path { .. } => unreachable!(), - }; - let file = Box::new(distribution_types::File { - dist_info_metadata: false, - filename: filename.to_string(), - hashes: self.hash.iter().map(|h| h.0.clone()).collect(), - requires_python: None, - size: self.size, - upload_time_utc_ms: None, - url: FileLocation::AbsoluteUrl(file_url.clone()), - yanked: None, - }); - let index = IndexUrl::Url(VerbatimUrl::from_url(index_url)); - Ok(RegistryBuiltWheel { - filename, - file, - index, - }) + } } - fn to_local_registry_dist( + fn to_registry_dist( &self, - index_path: &Path, + source: &RegistrySource, root: &Path, ) -> Result { let filename: WheelFilename = self.filename.clone(); - let file_path = match &self.url { - WheelWireSource::Path { path } => path, - WheelWireSource::Url { .. } => unreachable!(), - }; - let file_path = root.join(index_path).join(file_path); - let file_url = Url::from_file_path(&file_path).unwrap(); - let file = Box::new(distribution_types::File { - dist_info_metadata: false, - filename: filename.to_string(), - hashes: self.hash.iter().map(|h| h.0.clone()).collect(), - requires_python: None, - size: self.size, - upload_time_utc_ms: None, - url: FileLocation::AbsoluteUrl(UrlString::from(file_url)), - yanked: None, - }); - let index = IndexUrl::Path( - VerbatimUrl::from_path(root.join(index_path)) - .map_err(LockErrorKind::RegistryVerbatimUrl)?, - ); - Ok(RegistryBuiltWheel { - filename, - file, - index, - }) + + match source { + RegistrySource::Url(index_url) => { + let file_url = match &self.url { + WheelWireSource::Url { url } => url, + WheelWireSource::Path { .. } | WheelWireSource::Filename { .. } => { + return Err(LockErrorKind::MissingUrl { + name: filename.name, + version: filename.version, + } + .into()) + } + }; + let file = Box::new(distribution_types::File { + dist_info_metadata: false, + filename: filename.to_string(), + hashes: self.hash.iter().map(|h| h.0.clone()).collect(), + requires_python: None, + size: self.size, + upload_time_utc_ms: None, + url: FileLocation::AbsoluteUrl(file_url.clone()), + yanked: None, + }); + let index = IndexUrl::Url(VerbatimUrl::from_url(index_url.to_url())); + Ok(RegistryBuiltWheel { + filename, + file, + index, + }) + } + RegistrySource::Path(index_path) => { + let file_path = match &self.url { + WheelWireSource::Path { path } => path, + WheelWireSource::Url { .. } | WheelWireSource::Filename { .. } => { + return Err(LockErrorKind::MissingPath { + name: filename.name, + version: filename.version, + } + .into()) + } + }; + let file_url = Url::from_file_path(root.join(index_path).join(file_path)) + .map_err(|()| LockErrorKind::PathToUrl)?; + let file = Box::new(distribution_types::File { + dist_info_metadata: false, + filename: filename.to_string(), + hashes: self.hash.iter().map(|h| h.0.clone()).collect(), + requires_python: None, + size: self.size, + upload_time_utc_ms: None, + url: FileLocation::AbsoluteUrl(UrlString::from(file_url)), + yanked: None, + }); + let index = IndexUrl::Path( + VerbatimUrl::from_path(root.join(index_path)) + .map_err(LockErrorKind::RegistryVerbatimUrl)?, + ); + Ok(RegistryBuiltWheel { + filename, + file, + index, + }) + } + } } } @@ -2819,24 +2924,27 @@ struct WheelWire { #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)] #[serde(untagged)] enum WheelWireSource { - /// Used for all wheels except path wheels. + /// Used for all wheels that come from remote sources. Url { - /// A URL or file path (via `file://`) where the wheel that was locked - /// against was found. The location does not need to exist in the future, - /// so this should be treated as only a hint to where to look and/or - /// recording where the wheel file originally came from. + /// A URL where the wheel that was locked against was found. The location + /// does not need to exist in the future, so this should be treated as + /// only a hint to where to look and/or recording where the wheel file + /// originally came from. url: UrlString, }, - /// Used for path wheels. + /// Used for wheels that come from local registries (like `--find-links`). Path { - /// The filename of the wheel. - /// - /// This isn't part of the wire format since it's redundant with the - /// URL. But we do use it for various things, and thus compute it at - /// deserialization time. Not being able to extract a wheel filename from a - /// wheel URL is thus a deserialization error. + /// The path to the wheel, relative to the index. path: PathBuf, }, + /// Used for path wheels. + /// + /// We only store the filename for path wheel, since we can't store a relative path in the url + Filename { + /// We duplicate the filename since a lot of code relies on having the filename on the + /// wheel entry. + filename: WheelFilename, + }, } impl Wheel { @@ -2850,6 +2958,9 @@ impl Wheel { WheelWireSource::Path { path } => { table.insert("path", Value::from(PortablePath::from(path).to_string())); } + WheelWireSource::Filename { filename } => { + table.insert("filename", Value::from(filename.to_string())); + } } if let Some(ref hash) = self.hash { table.insert("hash", Value::from(hash.to_string())); @@ -2883,6 +2994,7 @@ impl TryFrom for Wheel { format!("failed to parse `{filename}` as wheel filename: {err}") })? } + WheelWireSource::Filename { filename } => filename.clone(), }; Ok(Wheel { @@ -3296,11 +3408,11 @@ enum LockErrorKind { }, /// An error that occurs when a distribution indicates that it is sourced from a local registry, /// but is missing a path. - #[error("found local registry distribution {name}=={version} without a valid path")] + #[error("found registry distribution {name}=={version} without a valid path")] MissingPath { - /// The name of the distribution that is missing a URL. + /// The name of the distribution that is missing a path. name: PackageName, - /// The version of the distribution that is missing a URL. + /// The version of the distribution that is missing a path. version: Version, }, /// An error that occurs when a distribution indicates that it is sourced from a registry, but @@ -3374,13 +3486,19 @@ enum LockErrorKind { #[source] VerbatimUrlError, ), - /// An error that occurs when parsing an registry's index URL. + /// An error that occurs when parsing a registry's index URL. #[error("could not convert between URL and path")] RegistryVerbatimUrl( /// The inner error we forward. #[source] VerbatimUrlError, ), + /// An error that occurs when converting a path to a URL. + #[error("failed to convert path to URL")] + PathToUrl, + /// An error that occurs when converting a URL to a path + #[error("failed to convert URL to path")] + UrlToPath, } /// An error that occurs when a source string could not be parsed. diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap index f5a25df78a97..d82a1092dcaa 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_missing.snap @@ -21,8 +21,10 @@ Ok( ), version: "4.3.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -71,8 +73,10 @@ Ok( ), version: "4.3.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 0, diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap index 3ad72e2ee30e..b2d54362b67b 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__hash_optional_present.snap @@ -21,8 +21,10 @@ Ok( ), version: "4.3.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -78,8 +80,10 @@ Ok( ), version: "4.3.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 0, diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap index 52e0bdd962f6..3ecdda5f8679 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap @@ -21,8 +21,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -63,8 +65,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -98,8 +102,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -122,8 +128,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 0, @@ -133,8 +141,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 1, diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap index 52e0bdd962f6..3ecdda5f8679 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap @@ -21,8 +21,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -63,8 +65,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -98,8 +102,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -122,8 +128,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 0, @@ -133,8 +141,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 1, diff --git a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap index 52e0bdd962f6..3ecdda5f8679 100644 --- a/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap +++ b/crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap @@ -21,8 +21,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -63,8 +65,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -98,8 +102,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }, @@ -122,8 +128,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 0, @@ -133,8 +141,10 @@ Ok( ), version: "0.1.0", source: Registry( - UrlString( - "https://pypi.org/simple", + Url( + UrlString( + "https://pypi.org/simple", + ), ), ), }: 1, diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 21b11ac5b674..81e78834a6ec 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -723,9 +723,15 @@ impl ValidatedLock { debug!("Ignoring existing lockfile due to missing root package: `{name}`"); Ok(Self::Preferable(lock)) } - SatisfiesResult::MissingIndex(name, version, index) => { + SatisfiesResult::MissingRemoteIndex(name, version, index) => { debug!( - "Ignoring existing lockfile due to missing index: `{name}` `{version}` from `{index}`" + "Ignoring existing lockfile due to missing remote index: `{name}` `{version}` from `{index}`" + ); + Ok(Self::Preferable(lock)) + } + SatisfiesResult::MissingLocalIndex(name, version, index) => { + debug!( + "Ignoring existing lockfile due to missing local index: `{name}` `{version}` from `{}`", index.display() ); Ok(Self::Preferable(lock)) } diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index 18e83ea41534..2e10a59f3e8d 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -6491,7 +6491,28 @@ fn lock_warn_missing_transitive_lower_bounds() -> Result<()> { fn lock_find_links_local_wheel() -> Result<()> { let context = TestContext::new("3.12"); - let pyproject_toml = context.temp_dir.child("pyproject.toml"); + // Populate the `--find-links` entries. + fs_err::create_dir_all(context.temp_dir.join("links"))?; + + for entry in fs_err::read_dir(context.workspace_root.join("scripts/links"))? { + let entry = entry?; + let path = entry.path(); + if path + .file_name() + .and_then(|file_name| file_name.to_str()) + .is_some_and(|file_name| file_name.starts_with("tqdm-")) + { + let dest = context + .temp_dir + .join("links") + .join(path.file_name().unwrap()); + fs_err::copy(&path, &dest)?; + } + } + + let workspace = context.temp_dir.child("workspace"); + + let pyproject_toml = workspace.child("pyproject.toml"); pyproject_toml.write_str(&formatdoc! { r#" [project] name = "project" @@ -6502,19 +6523,20 @@ fn lock_find_links_local_wheel() -> Result<()> { [tool.uv] find-links = ["{}"] "#, - context.workspace_root.join("scripts/links/").portable_display(), + context.temp_dir.join("links/").portable_display(), })?; - uv_snapshot!(context.filters(), context.lock(), @r###" + uv_snapshot!(context.filters(), context.lock().current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Resolved 2 packages in [TIME] "###); - let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); + let lock = fs_err::read_to_string(workspace.join("uv.lock")).unwrap(); insta::with_settings!({ filters => context.filters(), @@ -6541,7 +6563,7 @@ fn lock_find_links_local_wheel() -> Result<()> { [[package]] name = "tqdm" version = "1000.0.0" - source = { local = "../../../../../../workspace/puffin/scripts/links" } + source = { registry = "../links" } wheels = [ { path = "tqdm-1000.0.0-py3-none-any.whl" }, ] @@ -6550,25 +6572,28 @@ fn lock_find_links_local_wheel() -> Result<()> { }); // Re-run with `--locked`. - uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + uv_snapshot!(context.filters(), context.lock().arg("--locked").current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Resolved 2 packages in [TIME] "###); // Install from the lockfile. - uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + uv_snapshot!(context.filters(), context.sync().arg("--frozen").current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtualenv at: .venv Prepared 1 package in [TIME] Installed 2 packages in [TIME] - + project==0.1.0 (from file://[TEMP_DIR]/) + + project==0.1.0 (from file://[TEMP_DIR]/workspace) + tqdm==1000.0.0 "###); @@ -6580,7 +6605,28 @@ fn lock_find_links_local_wheel() -> Result<()> { fn lock_find_links_local_sdist() -> Result<()> { let context = TestContext::new("3.12"); - let pyproject_toml = context.temp_dir.child("pyproject.toml"); + // Populate the `--find-links` entries. + fs_err::create_dir_all(context.temp_dir.join("links"))?; + + for entry in fs_err::read_dir(context.workspace_root.join("scripts/links"))? { + let entry = entry?; + let path = entry.path(); + if path + .file_name() + .and_then(|file_name| file_name.to_str()) + .is_some_and(|file_name| file_name.starts_with("tqdm-")) + { + let dest = context + .temp_dir + .join("links") + .join(path.file_name().unwrap()); + fs_err::copy(&path, &dest)?; + } + } + + let workspace = context.temp_dir.child("workspace"); + + let pyproject_toml = workspace.child("pyproject.toml"); pyproject_toml.write_str(&formatdoc! { r#" [project] name = "project" @@ -6591,19 +6637,20 @@ fn lock_find_links_local_sdist() -> Result<()> { [tool.uv] find-links = ["{}"] "#, - context.workspace_root.join("scripts/links/").portable_display(), + context.temp_dir.join("links/").portable_display(), })?; - uv_snapshot!(context.filters(), context.lock(), @r###" + uv_snapshot!(context.filters(), context.lock().current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Resolved 2 packages in [TIME] "###); - let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); + let lock = fs_err::read_to_string(workspace.join("uv.lock")).unwrap(); insta::with_settings!({ filters => context.filters(), @@ -6630,32 +6677,35 @@ fn lock_find_links_local_sdist() -> Result<()> { [[package]] name = "tqdm" version = "999.0.0" - source = { local = "../../../../../../workspace/puffin/scripts/links" } + source = { registry = "../links" } sdist = { path = "tqdm-999.0.0.tar.gz" } "### ); }); // Re-run with `--locked`. - uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + uv_snapshot!(context.filters(), context.lock().arg("--locked").current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Resolved 2 packages in [TIME] "###); // Install from the lockfile. - uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + uv_snapshot!(context.filters(), context.sync().arg("--frozen").current_dir(&workspace), @r###" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] + Creating virtualenv at: .venv Prepared 2 packages in [TIME] Installed 2 packages in [TIME] - + project==0.1.0 (from file://[TEMP_DIR]/) + + project==0.1.0 (from file://[TEMP_DIR]/workspace) + tqdm==999.0.0 "###); @@ -6937,7 +6987,7 @@ fn lock_local_index() -> Result<()> { [[package]] name = "tqdm" version = "1000.0.0" - source = { local = "../../[TMP]/simple-html" } + source = { registry = "../../[TMP]/simple-html" } wheels = [ { path = "tqdm/tqdm-1000.0.0-py3-none-any.whl" }, ] @@ -10429,3 +10479,128 @@ fn lock_dropped_dev_extra() -> Result<()> { Ok(()) } + +/// Use a trailing slash on the declared index. +#[test] +fn lock_trailing_slash() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["anyio==3.7.0"] + + [tool.uv] + index-url = "https://pypi.org/simple/" + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25T00:00:00Z" + + [[package]] + name = "anyio" + version = "3.7.0" + source = { registry = "https://pypi.org/simple/" } + dependencies = [ + { name = "idna" }, + { name = "sniffio" }, + ] + sdist = { url = "https://files.pythonhosted.org/packages/c6/b3/fefbf7e78ab3b805dec67d698dc18dd505af7a18a8dd08868c9b4fa736b5/anyio-3.7.0.tar.gz", hash = "sha256:275d9973793619a5374e1c89a4f4ad3f4b0a5510a2b5b939444bee8f4c4d37ce", size = 142737 } + wheels = [ + { url = "https://files.pythonhosted.org/packages/68/fe/7ce1926952c8a403b35029e194555558514b365ad77d75125f521a2bec62/anyio-3.7.0-py3-none-any.whl", hash = "sha256:eddca883c4175f14df8aedce21054bfca3adb70ffe76a9f607aef9d7fa2ea7f0", size = 80873 }, + ] + + [[package]] + name = "idna" + version = "3.6" + source = { registry = "https://pypi.org/simple/" } + sdist = { url = "https://files.pythonhosted.org/packages/bf/3f/ea4b9117521a1e9c50344b909be7886dd00a519552724809bb1f486986c2/idna-3.6.tar.gz", hash = "sha256:9ecdbbd083b06798ae1e86adcbfe8ab1479cf864e4ee30fe4e46a003d12491ca", size = 175426 } + wheels = [ + { url = "https://files.pythonhosted.org/packages/c2/e7/a82b05cf63a603df6e68d59ae6a68bf5064484a0718ea5033660af4b54a9/idna-3.6-py3-none-any.whl", hash = "sha256:c05567e9c24a6b9faaa835c4821bad0590fbb9d5779e7caa6e1cc4978e7eb24f", size = 61567 }, + ] + + [[package]] + name = "project" + version = "0.1.0" + source = { editable = "." } + dependencies = [ + { name = "anyio" }, + ] + + [package.metadata] + requires-dist = [{ name = "anyio", specifier = "==3.7.0" }] + + [[package]] + name = "sniffio" + version = "1.3.1" + source = { registry = "https://pypi.org/simple/" } + sdist = { url = "https://files.pythonhosted.org/packages/a2/87/a6771e1546d97e7e041b6ae58d80074f81b7d5121207425c964ddf5cfdbd/sniffio-1.3.1.tar.gz", hash = "sha256:f4324edc670a0f49750a81b895f35c3adb843cca46f0530f79fc1babb23789dc", size = 20372 } + wheels = [ + { url = "https://files.pythonhosted.org/packages/e9/44/75a9c9421471a6c4805dbf2356f7c181a29c1879239abab1ea2cc8f38b40/sniffio-1.3.1-py3-none-any.whl", hash = "sha256:2f6da418d1f1e0fddd844478f41680e794e6051915791a034ff65e5f100525a2", size = 10235 }, + ] + "### + ); + }); + + // Re-run with `--locked`. + uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + + // Re-run with `--offline`. We shouldn't need a network connection to validate an + // already-correct lockfile with immutable metadata. + uv_snapshot!(context.filters(), context.lock().arg("--locked").arg("--offline").arg("--no-cache"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + + // Install from the lockfile. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + project==0.1.0 (from file://[TEMP_DIR]/) + + sniffio==1.3.1 + "###); + + Ok(()) +} From ea636b18b5ae1c190f1405a770be2966dce1c33a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 24 Aug 2024 15:50:51 -0400 Subject: [PATCH 3/4] Remove from_path --- crates/distribution-types/src/index_url.rs | 4 +- crates/pep508-rs/src/unnamed.rs | 4 +- crates/pep508-rs/src/verbatim_url.rs | 58 +++++-------------- crates/pypi-types/src/parsed_url.rs | 4 +- crates/pypi-types/src/requirement.rs | 10 ++-- crates/requirements-txt/src/lib.rs | 36 +++++++----- .../uv-distribution/src/metadata/lowering.rs | 4 +- crates/uv-resolver/src/lock.rs | 10 ++-- crates/uv-workspace/src/workspace.rs | 2 +- 9 files changed, 55 insertions(+), 77 deletions(-) diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index bb6d5e481821..0f16c932aab4 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -113,7 +113,7 @@ impl FromStr for IndexUrl { fn from_str(s: &str) -> Result { let url = if Path::new(s).exists() { - VerbatimUrl::from_path(std::path::absolute(s)?)? + VerbatimUrl::from_absolute_path(std::path::absolute(s)?)? } else { VerbatimUrl::parse_url(s)? }; @@ -248,7 +248,7 @@ impl FromStr for FlatIndexLocation { fn from_str(s: &str) -> Result { let url = if Path::new(s).exists() { - VerbatimUrl::from_path(std::path::absolute(s)?)? + VerbatimUrl::from_absolute_path(std::path::absolute(s)?)? } else { VerbatimUrl::parse_url(s)? }; diff --git a/crates/pep508-rs/src/unnamed.rs b/crates/pep508-rs/src/unnamed.rs index e64d763e4a1d..06c4757438c3 100644 --- a/crates/pep508-rs/src/unnamed.rs +++ b/crates/pep508-rs/src/unnamed.rs @@ -40,11 +40,11 @@ impl UnnamedRequirementUrl for VerbatimUrl { path: impl AsRef, working_dir: impl AsRef, ) -> Result { - Self::parse_path(path, working_dir) + Self::from_path(path, working_dir) } fn parse_absolute_path(path: impl AsRef) -> Result { - Self::parse_absolute_path(path) + Self::from_absolute_path(path) } fn parse_unnamed_url(given: impl AsRef) -> Result { diff --git a/crates/pep508-rs/src/verbatim_url.rs b/crates/pep508-rs/src/verbatim_url.rs index 11de14881fd4..9445b13cfd78 100644 --- a/crates/pep508-rs/src/verbatim_url.rs +++ b/crates/pep508-rs/src/verbatim_url.rs @@ -40,31 +40,6 @@ impl VerbatimUrl { Self { url, given: None } } - /// Create a [`VerbatimUrl`] from a file path. - /// - /// Assumes that the path is absolute. - pub fn from_path(path: impl AsRef) -> Result { - let path = path.as_ref(); - - // 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); - - // Convert to a URL. - let mut url = Url::from_file_path(path.clone()) - .map_err(|()| VerbatimUrlError::UrlConversion(path.to_path_buf()))?; - - // Set the fragment, if it exists. - if let Some(fragment) = fragment { - url.set_fragment(Some(fragment)); - } - - Ok(Self { url, given: None }) - } - /// Parse a URL from a string, expanding any environment variables. pub fn parse_url(given: impl AsRef) -> Result { let url = Url::parse(given.as_ref())?; @@ -73,7 +48,7 @@ impl VerbatimUrl { /// Parse a URL from an absolute or relative path. #[cfg(feature = "non-pep508-extensions")] // PEP 508 arguably only allows absolute file URLs. - pub fn parse_path( + pub fn from_path( path: impl AsRef, base_dir: impl AsRef, ) -> Result { @@ -82,13 +57,13 @@ impl VerbatimUrl { // Convert the path to an absolute path, if necessary. let path = if path.is_absolute() { - path.to_path_buf() + Cow::Borrowed(path) } else { - base_dir.as_ref().join(path) + Cow::Owned(base_dir.as_ref().join(path)) }; let path = normalize_absolute_path(&path) - .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?; + .map_err(|err| VerbatimUrlError::Normalization(path.to_path_buf(), err))?; // Extract the fragment, if it exists. let (path, fragment) = split_fragment(&path); @@ -106,19 +81,19 @@ impl VerbatimUrl { } /// Parse a URL from an absolute path. - pub fn parse_absolute_path(path: impl AsRef) -> Result { + pub fn from_absolute_path(path: impl AsRef) -> Result { let path = path.as_ref(); - // Convert the path to an absolute path, if necessary. + // Error if the path is relative. let path = if path.is_absolute() { - path.to_path_buf() + path } else { return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf())); }; // Normalize the path. - let path = normalize_absolute_path(&path) - .map_err(|err| VerbatimUrlError::Normalization(path.clone(), err))?; + 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); @@ -252,14 +227,11 @@ impl Pep508Url for VerbatimUrl { #[cfg(feature = "non-pep508-extensions")] if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::parse_path(path.as_ref(), working_dir)? + return Ok(VerbatimUrl::from_path(path.as_ref(), working_dir)? .with_given(url.to_string())); } - Ok( - VerbatimUrl::parse_absolute_path(path.as_ref())? - .with_given(url.to_string()), - ) + Ok(VerbatimUrl::from_absolute_path(path.as_ref())?.with_given(url.to_string())) } // Ex) `https://download.pytorch.org/whl/torch_stable.html` @@ -272,11 +244,11 @@ impl Pep508Url for VerbatimUrl { _ => { #[cfg(feature = "non-pep508-extensions")] if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::parse_path(expanded.as_ref(), working_dir)? + return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? .with_given(url.to_string())); } - Ok(VerbatimUrl::parse_absolute_path(expanded.as_ref())? + Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())? .with_given(url.to_string())) } } @@ -284,11 +256,11 @@ impl Pep508Url for VerbatimUrl { // Ex) `../editable/` #[cfg(feature = "non-pep508-extensions")] if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::parse_path(expanded.as_ref(), working_dir)? + return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? .with_given(url.to_string())); } - Ok(VerbatimUrl::parse_absolute_path(expanded.as_ref())?.with_given(url.to_string())) + Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())?.with_given(url.to_string())) } } } diff --git a/crates/pypi-types/src/parsed_url.rs b/crates/pypi-types/src/parsed_url.rs index 614fc487b301..fd5ad4350bbe 100644 --- a/crates/pypi-types/src/parsed_url.rs +++ b/crates/pypi-types/src/parsed_url.rs @@ -60,7 +60,7 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl { path: impl AsRef, working_dir: impl AsRef, ) -> Result { - let verbatim = VerbatimUrl::parse_path(&path, &working_dir)?; + let verbatim = VerbatimUrl::from_path(&path, &working_dir)?; let verbatim_path = verbatim.as_path()?; let is_dir = if let Ok(metadata) = verbatim_path.metadata() { metadata.is_dir() @@ -89,7 +89,7 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl { } fn parse_absolute_path(path: impl AsRef) -> Result { - let verbatim = VerbatimUrl::parse_absolute_path(&path)?; + let verbatim = VerbatimUrl::from_absolute_path(&path)?; let verbatim_path = verbatim.as_path()?; let is_dir = if let Ok(metadata) = verbatim_path.metadata() { metadata.is_dir() diff --git a/crates/pypi-types/src/requirement.rs b/crates/pypi-types/src/requirement.rs index b98e834bde2e..3952af9777e5 100644 --- a/crates/pypi-types/src/requirement.rs +++ b/crates/pypi-types/src/requirement.rs @@ -744,7 +744,7 @@ impl TryFrom for RequirementSource { // sources in the lockfile, we replace the URL anyway. RequirementSourceWire::Path { path } => { let path = PathBuf::from(path); - let url = VerbatimUrl::parse_path(&path, &*CWD)?; + let url = VerbatimUrl::from_path(&path, &*CWD)?; Ok(Self::Path { ext: DistExtension::from_path(path.as_path()) .map_err(|err| ParsedUrlError::MissingExtensionPath(path.clone(), err))?, @@ -754,7 +754,7 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Directory { directory } => { let directory = PathBuf::from(directory); - let url = VerbatimUrl::parse_path(&directory, &*CWD)?; + let url = VerbatimUrl::from_path(&directory, &*CWD)?; Ok(Self::Directory { install_path: directory, editable: false, @@ -763,7 +763,7 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Editable { editable } => { let editable = PathBuf::from(editable); - let url = VerbatimUrl::parse_path(&editable, &*CWD)?; + let url = VerbatimUrl::from_path(&editable, &*CWD)?; Ok(Self::Directory { install_path: editable, editable: true, @@ -788,7 +788,7 @@ pub fn redact_git_credentials(url: &mut Url) { #[cfg(test)] mod tests { - use std::path::{Path, PathBuf}; + use std::path::PathBuf; use pep508_rs::{MarkerTree, VerbatimUrl}; @@ -823,7 +823,7 @@ mod tests { source: RequirementSource::Directory { install_path: PathBuf::from(path), editable: false, - url: VerbatimUrl::from_path(Path::new(path)).unwrap(), + url: VerbatimUrl::from_absolute_path(path).unwrap(), }, origin: None, }; diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index 4cdd35f7f3a1..38db8a75f943 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -488,11 +488,13 @@ fn parse_entry( .ok() .filter(|path| path.exists()) { - VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { - source: err, - url: given.to_string(), - start, - end: s.cursor(), + VerbatimUrl::from_absolute_path(path).map_err(|err| { + RequirementsTxtParserError::VerbatimUrl { + source: err, + url: given.to_string(), + start, + end: s.cursor(), + } })? } else { VerbatimUrl::parse_url(expanded.as_ref()).map_err(|err| { @@ -512,11 +514,13 @@ fn parse_entry( .ok() .filter(|path| path.exists()) { - VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { - source: err, - url: given.to_string(), - start, - end: s.cursor(), + VerbatimUrl::from_absolute_path(path).map_err(|err| { + RequirementsTxtParserError::VerbatimUrl { + source: err, + url: given.to_string(), + start, + end: s.cursor(), + } })? } else { VerbatimUrl::parse_url(expanded.as_ref()).map_err(|err| { @@ -538,11 +542,13 @@ fn parse_entry( .ok() .filter(|path| path.exists()) { - VerbatimUrl::from_path(path).map_err(|err| RequirementsTxtParserError::VerbatimUrl { - source: err, - url: given.to_string(), - start, - end: s.cursor(), + VerbatimUrl::from_absolute_path(path).map_err(|err| { + RequirementsTxtParserError::VerbatimUrl { + source: err, + url: given.to_string(), + start, + end: s.cursor(), + } })? } else { VerbatimUrl::parse_url(expanded.as_ref()).map_err(|err| { diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs index 669ebcfe3be5..c0caf41190ba 100644 --- a/crates/uv-distribution/src/metadata/lowering.rs +++ b/crates/uv-distribution/src/metadata/lowering.rs @@ -142,7 +142,7 @@ impl LoweredRequirement { // relative to workspace: `packages/current_project` // workspace lock root: `../current_workspace` // relative to main workspace: `../current_workspace/packages/current_project` - let url = VerbatimUrl::parse_absolute_path(member.root())?; + let url = VerbatimUrl::from_absolute_path(member.root())?; let install_path = url.to_file_path().map_err(|()| { LoweringError::RelativeTo(io::Error::new( io::ErrorKind::Other, @@ -360,7 +360,7 @@ fn path_source( Origin::Project => project_dir, Origin::Workspace => workspace_root, }; - let url = VerbatimUrl::parse_path(path, base)?.with_given(path.to_string_lossy()); + let url = VerbatimUrl::from_path(path, base)?.with_given(path.to_string_lossy()); let install_path = url.to_file_path().map_err(|()| { LoweringError::RelativeTo(io::Error::new( io::ErrorKind::Other, diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index 66014249aada..38eb47f1a3fc 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -1513,7 +1513,7 @@ impl Package { yanked: None, }); let index = IndexUrl::Path( - VerbatimUrl::from_path(workspace_root.join(path)) + VerbatimUrl::from_absolute_path(workspace_root.join(path)) .map_err(LockErrorKind::RegistryVerbatimUrl)?, ); @@ -1727,7 +1727,7 @@ impl Package { /// Attempts to construct a `VerbatimUrl` from the given `Path`. fn verbatim_url(path: PathBuf, id: &PackageId) -> Result { - let url = VerbatimUrl::from_path(path).map_err(|err| LockErrorKind::VerbatimUrl { + let url = VerbatimUrl::from_absolute_path(path).map_err(|err| LockErrorKind::VerbatimUrl { id: id.clone(), err, })?; @@ -2892,7 +2892,7 @@ impl Wheel { yanked: None, }); let index = IndexUrl::Path( - VerbatimUrl::from_path(root.join(index_path)) + VerbatimUrl::from_absolute_path(root.join(index_path)) .map_err(LockErrorKind::RegistryVerbatimUrl)?, ); Ok(RegistryBuiltWheel { @@ -3213,7 +3213,7 @@ fn normalize_requirement( url: _, } => { let install_path = uv_fs::normalize_path(&workspace.install_path().join(&install_path)); - let url = VerbatimUrl::from_path(&install_path) + let url = VerbatimUrl::from_absolute_path(&install_path) .map_err(LockErrorKind::RequirementVerbatimUrl)?; Ok(Requirement { @@ -3234,7 +3234,7 @@ fn normalize_requirement( url: _, } => { let install_path = uv_fs::normalize_path(&workspace.install_path().join(&install_path)); - let url = VerbatimUrl::from_path(&install_path) + let url = VerbatimUrl::from_absolute_path(&install_path) .map_err(LockErrorKind::RequirementVerbatimUrl)?; Ok(Requirement { diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs index 6a1c69a6afc4..dce3ddc098a2 100644 --- a/crates/uv-workspace/src/workspace.rs +++ b/crates/uv-workspace/src/workspace.rs @@ -270,7 +270,7 @@ impl Workspace { }) .unwrap_or_default(); - let url = VerbatimUrl::from_path(&member.root) + let url = VerbatimUrl::from_absolute_path(&member.root) .expect("path is valid URL") .with_given(member.root.to_string_lossy()); Some(Requirement { From 99f628332961ff460b8120e98e8549d150fb21a6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 24 Aug 2024 22:25:09 -0400 Subject: [PATCH 4/4] Allow relative --- crates/pypi-types/src/requirement.rs | 6 ++++-- crates/uv-fs/src/path.rs | 3 +++ crates/uv-resolver/src/lock.rs | 27 +++++++++++++++++---------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/pypi-types/src/requirement.rs b/crates/pypi-types/src/requirement.rs index 3952af9777e5..594b187cf1bc 100644 --- a/crates/pypi-types/src/requirement.rs +++ b/crates/pypi-types/src/requirement.rs @@ -507,7 +507,8 @@ impl RequirementSource { ext, url, } => Ok(Self::Path { - install_path: relative_to(&install_path, path)?, + install_path: relative_to(&install_path, path) + .or_else(|_| std::path::absolute(install_path))?, ext, url, }), @@ -516,7 +517,8 @@ impl RequirementSource { editable, url, } => Ok(Self::Directory { - install_path: relative_to(&install_path, path)?, + install_path: relative_to(&install_path, path) + .or_else(|_| std::path::absolute(install_path))?, editable, url, }), diff --git a/crates/uv-fs/src/path.rs b/crates/uv-fs/src/path.rs index 5174817f9877..25e435b92a6a 100644 --- a/crates/uv-fs/src/path.rs +++ b/crates/uv-fs/src/path.rs @@ -270,6 +270,9 @@ pub fn canonicalize_executable(path: impl AsRef) -> std::io::Result `foo/__init__.py` /// `lib/marker.txt` and `lib/python/site-packages` -> `../../marker.txt` /// `bin/foo_launcher` and `lib/python/site-packages` -> `../../../bin/foo_launcher` +/// +/// Returns `Err` if there is no relative path between `path` and `base` (for example, if the paths +/// are on different drives on Windows). pub fn relative_to( path: impl AsRef, base: impl AsRef, diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index 38eb47f1a3fc..3c78c664a0bd 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -791,7 +791,9 @@ impl Lock { IndexUrl::Pypi(_) | IndexUrl::Url(_) => None, IndexUrl::Path(index_url) => { let path = index_url.to_file_path().ok()?; - let path = relative_to(&path, workspace.install_path()).ok()?; + let path = relative_to(&path, workspace.install_path()) + .or_else(|_| std::path::absolute(path)) + .ok()?; Some(path) } }) @@ -802,7 +804,9 @@ impl Lock { FlatIndexLocation::Url(_) => None, FlatIndexLocation::Path(index_url) => { let path = index_url.to_file_path().ok()?; - let path = relative_to(&path, workspace.install_path()).ok()?; + let path = relative_to(&path, workspace.install_path()) + .or_else(|_| std::path::absolute(path)) + .ok()?; Some(path) } }), @@ -2026,14 +2030,15 @@ impl Source { fn from_path_built_dist(path_dist: &PathBuiltDist, root: &Path) -> Result { let path = relative_to(&path_dist.install_path, root) + .or_else(|_| std::path::absolute(&path_dist.install_path)) .map_err(LockErrorKind::DistributionRelativePath)?; Ok(Source::Path(path)) } fn from_path_source_dist(path_dist: &PathSourceDist, root: &Path) -> Result { let path = relative_to(&path_dist.install_path, root) + .or_else(|_| std::path::absolute(&path_dist.install_path)) .map_err(LockErrorKind::DistributionRelativePath)?; - Ok(Source::Path(path)) } @@ -2042,6 +2047,7 @@ impl Source { root: &Path, ) -> Result { let path = relative_to(&directory_dist.install_path, root) + .or_else(|_| std::path::absolute(&directory_dist.install_path)) .map_err(LockErrorKind::DistributionRelativePath)?; if directory_dist.editable { Ok(Source::Editable(path)) @@ -2059,11 +2065,10 @@ impl Source { Ok(Source::Registry(source)) } IndexUrl::Path(url) => { - let path = relative_to( - url.to_file_path().map_err(|()| LockErrorKind::UrlToPath)?, - root, - ) - .map_err(LockErrorKind::IndexRelativePath)?; + let path = url.to_file_path().map_err(|()| LockErrorKind::UrlToPath)?; + let path = relative_to(&path, root) + .or_else(|_| std::path::absolute(&path)) + .map_err(LockErrorKind::IndexRelativePath)?; let source = RegistrySource::Path(path); Ok(Source::Registry(source)) } @@ -2520,7 +2525,8 @@ impl SourceDist { .map_err(LockErrorKind::InvalidFileUrl)? .to_file_path() .map_err(|()| LockErrorKind::UrlToPath)?; - let path = relative_to(reg_dist_path, index_path) + let path = relative_to(®_dist_path, index_path) + .or_else(|_| std::path::absolute(®_dist_path)) .map_err(LockErrorKind::DistributionRelativePath)?; let hash = reg_dist.file.hashes.iter().max().cloned().map(Hash::from); let size = reg_dist.file.size; @@ -2798,7 +2804,8 @@ impl Wheel { .map_err(LockErrorKind::InvalidFileUrl)? .to_file_path() .map_err(|()| LockErrorKind::UrlToPath)?; - let path = relative_to(wheel_path, index_path) + let path = relative_to(&wheel_path, index_path) + .or_else(|_| std::path::absolute(&wheel_path)) .map_err(LockErrorKind::DistributionRelativePath)?; Ok(Wheel { url: WheelWireSource::Path { path },