From dfec262586c7825775b57ebfff8fe78da6340905 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 31 Jul 2024 12:00:37 -0400 Subject: [PATCH] Capture portable path serialization in a struct (#5652) ## Summary I need to reuse this in #5494, so want to abstract it out and make it reusable. --- Cargo.lock | 5 +- crates/uv-fs/Cargo.toml | 1 + crates/uv-fs/src/path.rs | 94 ++++++++++++++++ crates/uv-resolver/Cargo.toml | 4 +- crates/uv-resolver/src/lock.rs | 130 +++++++++++------------ crates/uv-tool/Cargo.toml | 1 - crates/uv-tool/src/tool.rs | 7 +- crates/uv-workspace/Cargo.toml | 1 - crates/uv-workspace/src/pyproject_mut.rs | 5 +- 9 files changed, 169 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cf2a05ece..47357ca5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4826,6 +4826,7 @@ dependencies = [ "junction", "path-absolutize", "path-slash", + "serde", "tempfile", "tracing", "urlencoding", @@ -5016,7 +5017,6 @@ dependencies = [ "itertools 0.13.0", "once-map", "owo-colors", - "path-slash", "pep440_rs", "pep508_rs", "petgraph", @@ -5040,6 +5040,7 @@ dependencies = [ "uv-client", "uv-configuration", "uv-distribution", + "uv-fs", "uv-git", "uv-normalize", "uv-python", @@ -5117,7 +5118,6 @@ dependencies = [ "dirs-sys", "fs-err", "install-wheel-rs", - "path-slash", "pathdiff", "pep440_rs", "pep508_rs", @@ -5192,7 +5192,6 @@ dependencies = [ "fs-err", "glob", "insta", - "path-slash", "pep440_rs", "pep508_rs", "pypi-types", diff --git a/crates/uv-fs/Cargo.toml b/crates/uv-fs/Cargo.toml index 80fe09773..3b2c58a13 100644 --- a/crates/uv-fs/Cargo.toml +++ b/crates/uv-fs/Cargo.toml @@ -24,6 +24,7 @@ fs-err = { workspace = true } fs2 = { workspace = true } path-absolutize = { workspace = true } path-slash = { workspace = true } +serde = { workspace = true, optional = true } tempfile = { workspace = true } tracing = { workspace = true } urlencoding = { workspace = true } diff --git a/crates/uv-fs/src/path.rs b/crates/uv-fs/src/path.rs index e9ff20dbb..5174817f9 100644 --- a/crates/uv-fs/src/path.rs +++ b/crates/uv-fs/src/path.rs @@ -303,6 +303,100 @@ pub fn relative_to( Ok(up.join(stripped)) } +/// A path that can be serialized and deserialized in a portable way by converting Windows-style +/// backslashes to forward slashes, and using a `.` for an empty path. +/// +/// This implementation assumes that the path is valid UTF-8; otherwise, it won't roundtrip. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PortablePath<'a>(&'a Path); + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PortablePathBuf(PathBuf); + +impl AsRef for PortablePath<'_> { + fn as_ref(&self) -> &Path { + self.0 + } +} + +impl<'a, T> From<&'a T> for PortablePath<'a> +where + T: AsRef + ?Sized, +{ + fn from(path: &'a T) -> Self { + PortablePath(path.as_ref()) + } +} + +impl std::fmt::Display for PortablePath<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let path = self.0.to_slash_lossy(); + if path.is_empty() { + write!(f, ".") + } else { + write!(f, "{path}") + } + } +} + +impl std::fmt::Display for PortablePathBuf { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let path = self.0.to_slash_lossy(); + if path.is_empty() { + write!(f, ".") + } else { + write!(f, "{path}") + } + } +} + +impl From for PathBuf { + fn from(portable: PortablePathBuf) -> Self { + portable.0 + } +} + +impl From for PortablePathBuf { + fn from(path: PathBuf) -> Self { + Self(path) + } +} + +#[cfg(feature = "serde")] +impl serde::Serialize for PortablePathBuf { + fn serialize(&self, serializer: S) -> Result + where + S: serde::ser::Serializer, + { + self.to_string().serialize(serializer) + } +} + +#[cfg(feature = "serde")] +impl serde::Serialize for PortablePath<'_> { + fn serialize(&self, serializer: S) -> Result + where + S: serde::ser::Serializer, + { + self.to_string().serialize(serializer) + } +} + +#[cfg(feature = "serde")] +impl<'de> serde::de::Deserialize<'de> for PortablePathBuf { + fn deserialize(deserializer: D) -> Result + where + D: serde::de::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + if s == "." { + Ok(Self(PathBuf::new())) + } else { + Ok(Self(PathBuf::from(s))) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/uv-resolver/Cargo.toml b/crates/uv-resolver/Cargo.toml index 072438904..cad6be07d 100644 --- a/crates/uv-resolver/Cargo.toml +++ b/crates/uv-resolver/Cargo.toml @@ -26,9 +26,10 @@ requirements-txt = { workspace = true } uv-client = { workspace = true } uv-configuration = { workspace = true } uv-distribution = { workspace = true } +uv-fs = { workspace = true, features = ["serde"] } uv-git = { workspace = true } -uv-python = { workspace = true } uv-normalize = { workspace = true } +uv-python = { workspace = true } uv-types = { workspace = true } uv-warnings = { workspace = true } uv-workspace = { workspace = true } @@ -43,7 +44,6 @@ futures = { workspace = true } indexmap = { workspace = true } itertools = { workspace = true } owo-colors = { workspace = true } -path-slash = { workspace = true } petgraph = { workspace = true } pubgrub = { workspace = true } rkyv = { workspace = true } diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index df9b89adb..f2e05979e 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet, VecDeque}; +use std::convert::Infallible; use std::fmt::{Debug, Display}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -9,10 +10,8 @@ use std::sync::Arc; use either::Either; use itertools::Itertools; -use path_slash::PathExt; use petgraph::visit::EdgeRef; use rustc_hash::{FxHashMap, FxHashSet}; -use serde::{Deserialize, Deserializer}; use toml_edit::{value, Array, ArrayOfTables, InlineTable, Item, Table, Value}; use url::Url; @@ -35,6 +34,7 @@ use pypi_types::{ }; use uv_configuration::{ExtrasSpecification, Upgrade}; use uv_distribution::{ArchiveMetadata, Metadata}; +use uv_fs::{PortablePath, PortablePathBuf}; use uv_git::{GitReference, GitSha, RepositoryReference, ResolvedRepositoryReference}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_workspace::VirtualProject; @@ -1370,19 +1370,6 @@ enum Source { Editable(PathBuf), } -/// A [`PathBuf`], but we show `.` instead of an empty path. -/// -/// We also normalize backslashes to forward slashes on Windows, to ensure -/// that the lockfile contains portable paths. -fn serialize_path_with_dot(path: &Path) -> Cow { - let path = path.to_slash_lossy(); - if path.is_empty() { - Cow::Borrowed(".") - } else { - path - } -} - impl Source { fn from_resolved_dist(resolved_dist: &ResolvedDist) -> Source { match *resolved_dist { @@ -1514,21 +1501,18 @@ impl Source { } } Source::Path(ref path) => { - source_table.insert( - "path", - Value::from(serialize_path_with_dot(path).into_owned()), - ); + source_table.insert("path", Value::from(PortablePath::from(path).to_string())); } Source::Directory(ref path) => { source_table.insert( "directory", - Value::from(serialize_path_with_dot(path).into_owned()), + Value::from(PortablePath::from(path).to_string()), ); } Source::Editable(ref path) => { source_table.insert( "editable", - Value::from(serialize_path_with_dot(path).into_owned()), + Value::from(PortablePath::from(path).to_string()), ); } } @@ -1543,7 +1527,7 @@ impl std::fmt::Display for Source { write!(f, "{}+{}", self.name(), url) } Source::Path(path) | Source::Directory(path) | Source::Editable(path) => { - write!(f, "{}+{}", self.name(), serialize_path_with_dot(path)) + write!(f, "{}+{}", self.name(), PortablePath::from(path)) } } } @@ -1592,16 +1576,13 @@ enum SourceWire { subdirectory: Option, }, Path { - #[serde(deserialize_with = "deserialize_path_with_dot")] - path: PathBuf, + path: PortablePathBuf, }, Directory { - #[serde(deserialize_with = "deserialize_path_with_dot")] - directory: PathBuf, + directory: PortablePathBuf, }, Editable { - #[serde(deserialize_with = "deserialize_path_with_dot")] - editable: PathBuf, + editable: PortablePathBuf, }, } @@ -1634,9 +1615,9 @@ impl TryFrom for Source { Ok(Source::Git(url, git_source)) } Direct { url, subdirectory } => Ok(Source::Direct(url, DirectSource { subdirectory })), - Path { path } => Ok(Source::Path(path)), - Directory { directory } => Ok(Source::Directory(directory)), - Editable { editable } => Ok(Source::Editable(editable)), + Path { path } => Ok(Source::Path(path.into())), + Directory { directory } => Ok(Source::Directory(directory.into())), + Editable { editable } => Ok(Source::Editable(editable.into())), } } } @@ -1719,7 +1700,7 @@ struct SourceDistMetadata { /// future, so this should be treated as only a hint to where to look /// and/or recording where the source dist file originally came from. #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)] -#[serde(untagged)] +#[serde(try_from = "SourceDistWire")] enum SourceDist { Url { url: UrlString, @@ -1727,26 +1708,12 @@ enum SourceDist { metadata: SourceDistMetadata, }, Path { - #[serde(deserialize_with = "deserialize_path_with_dot")] path: PathBuf, #[serde(flatten)] metadata: SourceDistMetadata, }, } -/// A [`PathBuf`], but we show `.` instead of an empty path. -fn deserialize_path_with_dot<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - let path = String::deserialize(deserializer)?; - if path == "." { - Ok(PathBuf::new()) - } else { - Ok(PathBuf::from(path)) - } -} - impl SourceDist { fn filename(&self) -> Option> { match self { @@ -1780,26 +1747,6 @@ impl SourceDist { } impl SourceDist { - /// Returns the TOML representation of this source distribution. - fn to_toml(&self) -> anyhow::Result { - let mut table = InlineTable::new(); - match &self { - SourceDist::Url { url, .. } => { - table.insert("url", Value::from(url.as_ref())); - } - SourceDist::Path { path, .. } => { - table.insert("path", Value::from(serialize_path_with_dot(path).as_ref())); - } - } - if let Some(hash) = self.hash() { - table.insert("hash", Value::from(hash.to_string())); - } - if let Some(size) = self.size() { - table.insert("size", Value::from(i64::try_from(size)?)); - } - Ok(table) - } - fn from_annotated_dist( id: &DistributionId, annotated_dist: &AnnotatedDist, @@ -1890,6 +1837,57 @@ impl SourceDist { } } +#[derive(Clone, Debug, serde::Deserialize)] +#[serde(untagged)] +enum SourceDistWire { + Url { + url: UrlString, + #[serde(flatten)] + metadata: SourceDistMetadata, + }, + Path { + path: PortablePathBuf, + #[serde(flatten)] + metadata: SourceDistMetadata, + }, +} + +impl SourceDist { + /// Returns the TOML representation of this source distribution. + fn to_toml(&self) -> anyhow::Result { + let mut table = InlineTable::new(); + match &self { + SourceDist::Url { url, .. } => { + table.insert("url", Value::from(url.as_ref())); + } + SourceDist::Path { path, .. } => { + table.insert("path", Value::from(PortablePath::from(path).to_string())); + } + } + if let Some(hash) = self.hash() { + table.insert("hash", Value::from(hash.to_string())); + } + if let Some(size) = self.size() { + table.insert("size", Value::from(i64::try_from(size)?)); + } + Ok(table) + } +} + +impl TryFrom for SourceDist { + type Error = Infallible; + + fn try_from(wire: SourceDistWire) -> Result { + match wire { + SourceDistWire::Url { url, metadata } => Ok(SourceDist::Url { url, metadata }), + SourceDistWire::Path { path, metadata } => Ok(SourceDist::Path { + path: path.into(), + metadata, + }), + } + } +} + impl From for GitSourceKind { fn from(value: GitReference) -> Self { match value { diff --git a/crates/uv-tool/Cargo.toml b/crates/uv-tool/Cargo.toml index b1a9264e5..b953fc90a 100644 --- a/crates/uv-tool/Cargo.toml +++ b/crates/uv-tool/Cargo.toml @@ -26,7 +26,6 @@ uv-installer = { workspace = true } dirs-sys = { workspace = true } fs-err = { workspace = true } -path-slash = { workspace = true } pathdiff = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } diff --git a/crates/uv-tool/src/tool.rs b/crates/uv-tool/src/tool.rs index 0b5f4ea11..786ce5355 100644 --- a/crates/uv-tool/src/tool.rs +++ b/crates/uv-tool/src/tool.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; -use path_slash::PathBufExt; -use pypi_types::VerbatimParsedUrl; use serde::Deserialize; use toml_edit::value; use toml_edit::Array; use toml_edit::Table; use toml_edit::Value; +use pypi_types::VerbatimParsedUrl; +use uv_fs::PortablePath; + /// A tool entry. #[allow(dead_code)] #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -127,7 +128,7 @@ impl ToolEntrypoint { table.insert( "install-path", // Use cross-platform slashes so the toml string type does not change - value(self.install_path.to_slash_lossy().to_string()), + value(PortablePath::from(&self.install_path).to_string()), ); table } diff --git a/crates/uv-workspace/Cargo.toml b/crates/uv-workspace/Cargo.toml index 696d1b27b..7b5a40473 100644 --- a/crates/uv-workspace/Cargo.toml +++ b/crates/uv-workspace/Cargo.toml @@ -26,7 +26,6 @@ uv-options-metadata = { workspace = true } either = { workspace = true } fs-err = { workspace = true } glob = { workspace = true } -path-slash = { workspace = true } rustc-hash = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"] } diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index ea6519917..73ab9b62a 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -2,11 +2,11 @@ use std::path::Path; use std::str::FromStr; use std::{fmt, mem}; -use path_slash::PathExt; use thiserror::Error; use toml_edit::{Array, DocumentMut, Item, RawString, Table, TomlError, Value}; use pep508_rs::{ExtraName, PackageName, Requirement, VersionOrUrl}; +use uv_fs::PortablePath; use crate::pyproject::{DependencyType, PyProjectToml, Source}; @@ -65,8 +65,7 @@ impl PyProjectTomlMut { .ok_or(Error::MalformedWorkspace)?; // Add the path to the workspace. - // Use cross-platform slashes so the toml string type does not change - members.push(path.as_ref().to_slash_lossy().to_string()); + members.push(PortablePath::from(path.as_ref()).to_string()); Ok(()) }