From 7f27e6eaa8021c62f16571d809d7c845b8178773 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Wed, 3 May 2023 20:23:50 -0700 Subject: [PATCH] Stripped down unix path (#4767) ### Description - Experiment with a very stripped down `RelativeUnixPath` implementation - Test out using it in `git.rs` ### Testing Instructions Existing tests in `git.rs` --- .../turbopath/src/absolute_system_path_buf.rs | 40 ++++++++---- .../turbopath/src/anchored_system_path_buf.rs | 12 ++-- crates/turbopath/src/lib.rs | 4 ++ .../turbopath/src/relative_system_path_buf.rs | 4 ++ crates/turbopath/src/relative_unix_path.rs | 25 ++++++++ .../turbopath/src/relative_unix_path_buf.rs | 63 ++----------------- crates/turborepo-scm/src/git.rs | 23 +++---- crates/turborepo-scm/src/lib.rs | 7 +-- 8 files changed, 83 insertions(+), 95 deletions(-) create mode 100644 crates/turbopath/src/relative_unix_path.rs diff --git a/crates/turbopath/src/absolute_system_path_buf.rs b/crates/turbopath/src/absolute_system_path_buf.rs index 654132172d497..3cc18b348ef41 100644 --- a/crates/turbopath/src/absolute_system_path_buf.rs +++ b/crates/turbopath/src/absolute_system_path_buf.rs @@ -17,6 +17,7 @@ use serde::Serialize; use crate::{ AnchoredSystemPathBuf, IntoSystem, PathError, PathValidationError, RelativeSystemPathBuf, + RelativeUnixPath, }; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize)] @@ -51,10 +52,10 @@ impl AbsoluteSystemPathBuf { /// #[cfg(not(windows))] /// assert_eq!(absolute_path.as_path(), Path::new("/Users/user")); /// ``` - pub fn new(unchecked_path: impl Into) -> Result { + pub fn new(unchecked_path: impl Into) -> Result { let unchecked_path = unchecked_path.into(); if !unchecked_path.is_absolute() { - return Err(PathValidationError::NotAbsolute(unchecked_path)); + return Err(PathValidationError::NotAbsolute(unchecked_path).into()); } let system_path = unchecked_path.into_system()?; @@ -90,10 +91,7 @@ impl AbsoluteSystemPathBuf { /// assert_eq!(anchored_path.as_path(), Path::new("Documents")); /// } /// ``` - pub fn anchor( - &self, - path: &AbsoluteSystemPathBuf, - ) -> Result { + pub fn anchor(&self, path: &AbsoluteSystemPathBuf) -> Result { AnchoredSystemPathBuf::new(self, path) } @@ -127,6 +125,14 @@ impl AbsoluteSystemPathBuf { AbsoluteSystemPathBuf(self.0.join(path.as_path())) } + pub fn join_unix_path( + &self, + unix_path: &RelativeUnixPath, + ) -> Result { + let tail = unix_path.to_system_path()?; + Ok(AbsoluteSystemPathBuf(self.0.join(tail.as_path()))) + } + pub fn as_path(&self) -> &Path { self.0.as_path() } @@ -263,7 +269,7 @@ impl AsRef for AbsoluteSystemPathBuf { mod tests { use std::assert_matches::assert_matches; - use crate::{AbsoluteSystemPathBuf, PathValidationError}; + use crate::{AbsoluteSystemPathBuf, PathError, PathValidationError}; #[cfg(not(windows))] #[test] @@ -271,12 +277,16 @@ mod tests { assert!(AbsoluteSystemPathBuf::new("/Users/user").is_ok()); assert_matches!( AbsoluteSystemPathBuf::new("./Users/user/"), - Err(PathValidationError::NotAbsolute(_)) + Err(PathError::PathValidationError( + PathValidationError::NotAbsolute(_) + )) ); assert_matches!( AbsoluteSystemPathBuf::new("Users"), - Err(PathValidationError::NotAbsolute(_)) + Err(PathError::PathValidationError( + PathValidationError::NotAbsolute(_) + )) ); } @@ -286,15 +296,21 @@ mod tests { assert!(AbsoluteSystemPathBuf::new("C:\\Users\\user").is_ok()); assert_matches!( AbsoluteSystemPathBuf::new(".\\Users\\user\\"), - Err(PathValidationError::NotAbsolute(_)) + Err(PathError::PathValidationError( + PathValidationError::NotAbsolute(_) + )) ); assert_matches!( AbsoluteSystemPathBuf::new("Users"), - Err(PathValidationError::NotAbsolute(_)) + Err(PathError::PathValidationError( + PathValidationError::NotAbsolute(_) + )) ); assert_matches!( AbsoluteSystemPathBuf::new("/Users/home"), - Err(PathValidationError::NotAbsolute(_)) + Err(PathError::PathValidationError( + PathValidationError::NotAbsolute(_) + )) ) } } diff --git a/crates/turbopath/src/anchored_system_path_buf.rs b/crates/turbopath/src/anchored_system_path_buf.rs index 410458ca96e0c..ab309a134f055 100644 --- a/crates/turbopath/src/anchored_system_path_buf.rs +++ b/crates/turbopath/src/anchored_system_path_buf.rs @@ -2,17 +2,17 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; -use crate::{AbsoluteSystemPathBuf, IntoSystem, PathValidationError}; +use crate::{AbsoluteSystemPathBuf, IntoSystem, PathError, PathValidationError}; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize, Deserialize)] pub struct AnchoredSystemPathBuf(PathBuf); impl TryFrom<&Path> for AnchoredSystemPathBuf { - type Error = PathValidationError; + type Error = PathError; fn try_from(path: &Path) -> Result { if path.is_absolute() { - return Err(PathValidationError::NotRelative(path.to_path_buf())); + return Err(PathValidationError::NotRelative(path.to_path_buf()).into()); } Ok(AnchoredSystemPathBuf(path.into_system()?)) @@ -23,7 +23,7 @@ impl AnchoredSystemPathBuf { pub fn new( root: &AbsoluteSystemPathBuf, path: &AbsoluteSystemPathBuf, - ) -> Result { + ) -> Result { let stripped_path = path .as_path() .strip_prefix(root.as_path()) @@ -37,10 +37,10 @@ impl AnchoredSystemPathBuf { self.0.as_path() } - pub fn to_str(&self) -> Result<&str, PathValidationError> { + pub fn to_str(&self) -> Result<&str, PathError> { self.0 .to_str() - .ok_or_else(|| PathValidationError::InvalidUnicode(self.0.clone())) + .ok_or_else(|| PathValidationError::InvalidUnicode(self.0.clone()).into()) } } diff --git a/crates/turbopath/src/lib.rs b/crates/turbopath/src/lib.rs index 48e65f7979803..c5bca98cd60a1 100644 --- a/crates/turbopath/src/lib.rs +++ b/crates/turbopath/src/lib.rs @@ -3,6 +3,7 @@ mod absolute_system_path_buf; mod anchored_system_path_buf; mod relative_system_path_buf; +mod relative_unix_path; mod relative_unix_path_buf; use std::{ @@ -14,6 +15,7 @@ pub use absolute_system_path_buf::AbsoluteSystemPathBuf; pub use anchored_system_path_buf::AnchoredSystemPathBuf; use path_slash::{PathBufExt, PathExt}; pub use relative_system_path_buf::RelativeSystemPathBuf; +pub use relative_unix_path::RelativeUnixPath; pub use relative_unix_path_buf::RelativeUnixPathBuf; #[derive(Debug, thiserror::Error)] @@ -44,6 +46,8 @@ pub enum PathValidationError { NotRelative(PathBuf), #[error("Path {0} is not parent of {1}")] NotParent(String, String), + #[error("Path {0} is not a unix path")] + NotUnix(String), } trait IntoSystem { diff --git a/crates/turbopath/src/relative_system_path_buf.rs b/crates/turbopath/src/relative_system_path_buf.rs index 663c18692815c..ef47cc6e24622 100644 --- a/crates/turbopath/src/relative_system_path_buf.rs +++ b/crates/turbopath/src/relative_system_path_buf.rs @@ -43,6 +43,10 @@ impl RelativeSystemPathBuf { Ok(RelativeSystemPathBuf(system_path)) } + pub(crate) fn new_unchecked(unchecked_path: impl Into) -> Self { + Self(unchecked_path.into()) + } + pub fn as_path(&self) -> &Path { &self.0 } diff --git a/crates/turbopath/src/relative_unix_path.rs b/crates/turbopath/src/relative_unix_path.rs new file mode 100644 index 0000000000000..a8adfc244d016 --- /dev/null +++ b/crates/turbopath/src/relative_unix_path.rs @@ -0,0 +1,25 @@ +use std::path::Path; + +use crate::{IntoSystem, PathError, PathValidationError, RelativeSystemPathBuf}; + +pub struct RelativeUnixPath { + inner: Path, +} + +impl RelativeUnixPath { + pub fn new>(value: &P) -> Result<&Self, PathError> { + let path = value.as_ref(); + if path.is_absolute() { + return Err(PathValidationError::NotRelative(path.to_owned()).into()); + } + // copied from stdlib path.rs: relies on the representation of + // RelativeUnixPath being just a Path, the same way Path relies on + // just being an OsStr + Ok(unsafe { &*(path as *const Path as *const Self) }) + } + + pub fn to_system_path(&self) -> Result { + let system_path = self.inner.into_system()?; + Ok(RelativeSystemPathBuf::new_unchecked(system_path)) + } +} diff --git a/crates/turbopath/src/relative_unix_path_buf.rs b/crates/turbopath/src/relative_unix_path_buf.rs index c20505dddd4e7..95306ee28b763 100644 --- a/crates/turbopath/src/relative_unix_path_buf.rs +++ b/crates/turbopath/src/relative_unix_path_buf.rs @@ -1,7 +1,4 @@ -use std::{ - ffi::OsStr, - path::{Components, Path, PathBuf}, -}; +use std::path::PathBuf; use serde::Serialize; @@ -36,81 +33,29 @@ impl RelativeUnixPathBuf { Ok(RelativeUnixPathBuf(path.into_unix()?)) } - pub fn as_path(&self) -> &Path { - &self.0 - } - - pub fn components(&self) -> Components<'_> { - self.0.components() - } - - pub fn parent(&self) -> Option { - self.0 - .parent() - .map(|p| RelativeUnixPathBuf(p.to_path_buf())) - } - - pub fn starts_with>(&self, base: P) -> bool { - self.0.starts_with(base.as_ref()) - } - - pub fn ends_with>(&self, child: P) -> bool { - self.0.ends_with(child.as_ref()) - } - - pub fn join>(&self, path: P) -> RelativeUnixPathBuf { - RelativeUnixPathBuf(self.0.join(path)) - } - pub fn to_str(&self) -> Result<&str, PathValidationError> { self.0 .to_str() .ok_or_else(|| PathValidationError::InvalidUnicode(self.0.clone())) } - - pub fn file_name(&self) -> Option<&OsStr> { - self.0.file_name() - } - - pub fn extension(&self) -> Option<&OsStr> { - self.0.extension() - } - - pub fn into_path_buf(self) -> PathBuf { - self.0 - } } #[cfg(test)] mod tests { + use std::path::Path; + use super::*; #[test] fn test_relative_unix_path_buf() { let path = RelativeUnixPathBuf::new(PathBuf::from("foo/bar")).unwrap(); - assert_eq!(path.as_path(), Path::new("foo/bar")); - assert_eq!(path.components().count(), 2); - assert_eq!(path.parent().unwrap().as_path(), Path::new("foo")); - assert!(path.starts_with("foo")); - assert!(path.ends_with("bar")); - assert_eq!(path.join("baz").as_path(), Path::new("foo/bar/baz")); assert_eq!(path.to_str().unwrap(), "foo/bar"); - assert_eq!(path.file_name(), Some(OsStr::new("bar"))); - assert_eq!(path.extension(), None); } #[test] fn test_relative_unix_path_buf_with_extension() { let path = RelativeUnixPathBuf::new(PathBuf::from("foo/bar.txt")).unwrap(); - assert_eq!(path.as_path(), Path::new("foo/bar.txt")); - assert_eq!(path.components().count(), 2); - assert_eq!(path.parent().unwrap().as_path(), Path::new("foo")); - assert!(path.starts_with("foo")); - assert!(path.ends_with("bar.txt")); - assert_eq!(path.join("baz").as_path(), Path::new("foo/bar.txt/baz")); assert_eq!(path.to_str().unwrap(), "foo/bar.txt"); - assert_eq!(path.file_name(), Some(OsStr::new("bar.txt"))); - assert_eq!(path.extension(), Some(OsStr::new("txt"))); } #[test] @@ -125,6 +70,6 @@ mod tests { #[test] fn test_convert_from_windows_path() { let path = RelativeUnixPathBuf::new(PathBuf::from("foo\\bar")).unwrap(); - assert_eq!(path.as_path(), Path::new("foo/bar")); + assert_eq!(path.0.as_path(), Path::new("foo/bar")); } } diff --git a/crates/turborepo-scm/src/git.rs b/crates/turborepo-scm/src/git.rs index 7ad5648d24b5c..b15718b4a6ad4 100644 --- a/crates/turborepo-scm/src/git.rs +++ b/crates/turborepo-scm/src/git.rs @@ -1,11 +1,6 @@ -use std::{ - backtrace::Backtrace, - collections::HashSet, - path::{Path, PathBuf}, - process::Command, -}; +use std::{backtrace::Backtrace, collections::HashSet, path::PathBuf, process::Command}; -use turbopath::{AbsoluteSystemPathBuf, AnchoredSystemPathBuf}; +use turbopath::{AbsoluteSystemPathBuf, AnchoredSystemPathBuf, RelativeUnixPath}; use crate::Error; @@ -102,7 +97,7 @@ fn add_files_from_stdout( ) { let stdout = String::from_utf8(stdout).unwrap(); for line in stdout.lines() { - let path = Path::new(line); + let path = RelativeUnixPath::new(&line).unwrap(); let anchored_to_turbo_root_file_path = reanchor_path_from_git_root_to_turbo_root(git_root, turbo_root, path).unwrap(); files.insert( @@ -117,10 +112,9 @@ fn add_files_from_stdout( fn reanchor_path_from_git_root_to_turbo_root( git_root: &AbsoluteSystemPathBuf, turbo_root: &AbsoluteSystemPathBuf, - path: &Path, + path: &RelativeUnixPath, ) -> Result { - let anchored_to_git_root_file_path: AnchoredSystemPathBuf = path.try_into()?; - let absolute_file_path = git_root.resolve(&anchored_to_git_root_file_path); + let absolute_file_path = git_root.join_unix_path(path)?; let anchored_to_turbo_root_file_path = turbo_root.anchor(&absolute_file_path)?; Ok(anchored_to_turbo_root_file_path) } @@ -185,7 +179,7 @@ mod tests { use git2::{Oid, Repository}; use tempfile::TempDir; - use turbopath::PathValidationError; + use turbopath::{PathError, PathValidationError}; use super::previous_content; use crate::{git::changed_files, Error}; @@ -615,7 +609,10 @@ mod tests { assert_matches!( turbo_root_is_not_subdir_of_git_root, - Err(Error::Path(PathValidationError::NotParent(_, _), _)) + Err(Error::Path( + PathError::PathValidationError(PathValidationError::NotParent(_, _)), + _ + )) ); Ok(()) diff --git a/crates/turborepo-scm/src/lib.rs b/crates/turborepo-scm/src/lib.rs index 0f4328f6a5591..86abdfc38d4e5 100644 --- a/crates/turborepo-scm/src/lib.rs +++ b/crates/turborepo-scm/src/lib.rs @@ -5,7 +5,7 @@ use std::backtrace; use thiserror::Error; -use turbopath::PathValidationError; +use turbopath::PathError; pub mod git; @@ -18,8 +18,5 @@ pub enum Error { #[error("io error: {0}")] Io(#[from] std::io::Error, #[backtrace] backtrace::Backtrace), #[error("path error: {0}")] - Path( - #[from] PathValidationError, - #[backtrace] backtrace::Backtrace, - ), + Path(#[from] PathError, #[backtrace] backtrace::Backtrace), }