Skip to content

Commit

Permalink
Converted a lot of call sites to impl AsRef<AbsoluteSystemPath>
Browse files Browse the repository at this point in the history
  • Loading branch information
NicholasLYang committed May 8, 2023
1 parent 08fe50f commit e71fda3
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 30 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/turbopath/Cargo.toml
Expand Up @@ -11,3 +11,6 @@ path-slash = "0.2.1"
# TODO: Make this a crate feature
serde = { workspace = true }
thiserror = { workspace = true }

[dev-dependencies]
anyhow = { workspace = true }
43 changes: 43 additions & 0 deletions crates/turbopath/src/absolute_system_path.rs
Expand Up @@ -29,6 +29,12 @@ impl ToOwned for AbsoluteSystemPath {
}
}

impl AsRef<AbsoluteSystemPath> for AbsoluteSystemPath {
fn as_ref(&self) -> &AbsoluteSystemPath {
self
}
}

impl fmt::Display for AbsoluteSystemPath {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.display().fmt(f)
Expand Down Expand Up @@ -74,6 +80,10 @@ impl AbsoluteSystemPath {
AbsoluteSystemPathBuf(path)
}

pub fn join_literal(&self, segment: &str) -> AbsoluteSystemPathBuf {
AbsoluteSystemPathBuf(self.0.join(segment))
}

pub fn join_unix_path(
&self,
unix_path: &RelativeUnixPath,
Expand Down Expand Up @@ -131,3 +141,36 @@ impl AbsoluteSystemPath {
fs::remove_file(&self.0)
}
}

#[cfg(test)]
mod tests {
use anyhow::Result;

use super::*;

#[test]
fn test_create_absolute_path() -> Result<()> {
let absolute_path = AbsoluteSystemPath::new("/foo/bar")?;

#[cfg(unix)]
{
assert_eq!(absolute_path.to_string(), "/foo/bar");
assert!(absolute_path.is_borrowed());
}

#[cfg(windows)]
{
assert_eq!(absolute_path.to_string(), r"\foo\bar");
assert!(absolute_path.is_owned());
}

#[cfg(windows)]
{
let absolute_path = AbsoluteSystemPath::new(r"\foo\bar")?;
assert_eq!(absolute_path.to_string(), r"\foo\bar");
assert!(absolute_path.is_borrowed());
}

Ok(())
}
}
9 changes: 6 additions & 3 deletions crates/turbopath/src/absolute_system_path_buf.rs
Expand Up @@ -97,8 +97,11 @@ impl AbsoluteSystemPathBuf {
/// assert_eq!(anchored_path.as_path(), Path::new("Documents"));
/// }
/// ```
pub fn anchor(&self, path: &AbsoluteSystemPath) -> Result<AnchoredSystemPathBuf, PathError> {
AnchoredSystemPathBuf::new(self.borrow(), path)
pub fn anchor(
&self,
path: impl AsRef<AbsoluteSystemPath>,
) -> Result<AnchoredSystemPathBuf, PathError> {
AnchoredSystemPathBuf::new(self, path)
}

/// Resolves `path` with `self` as anchor.
Expand Down Expand Up @@ -162,7 +165,7 @@ impl AbsoluteSystemPathBuf {
}

pub fn join_literal(&self, segment: &str) -> Self {
AbsoluteSystemPathBuf(self.0.join(Path::new(segment)))
AbsoluteSystemPathBuf(self.0.join(segment))
}

pub fn ensure_dir(&self) -> Result<(), io::Error> {
Expand Down
7 changes: 6 additions & 1 deletion crates/turbopath/src/anchored_system_path_buf.rs
Expand Up @@ -20,7 +20,12 @@ impl TryFrom<&Path> for AnchoredSystemPathBuf {
}

impl AnchoredSystemPathBuf {
pub fn new(root: &AbsoluteSystemPath, path: &AbsoluteSystemPath) -> Result<Self, PathError> {
pub fn new(
root: impl AsRef<AbsoluteSystemPath>,
path: impl AsRef<AbsoluteSystemPath>,
) -> Result<Self, PathError> {
let root = root.as_ref();
let path = path.as_ref();
let stripped_path = path
.as_path()
.strip_prefix(root.as_path())
Expand Down
1 change: 1 addition & 0 deletions crates/turbopath/src/lib.rs
@@ -1,4 +1,5 @@
#![feature(assert_matches)]
#![feature(cow_is_borrowed)]

mod absolute_system_path;
mod absolute_system_path_buf;
Expand Down
2 changes: 0 additions & 2 deletions crates/turbopath/src/relative_unix_path_buf.rs
Expand Up @@ -42,8 +42,6 @@ impl RelativeUnixPathBuf {

#[cfg(test)]
mod tests {
use std::path::Path;

use super::*;

#[test]
Expand Down
4 changes: 2 additions & 2 deletions crates/turborepo-ffi/src/lib.rs
Expand Up @@ -4,7 +4,7 @@
//! and in ffi.go before modifying this file.
mod lockfile;

use std::{borrow::Borrow, mem::ManuallyDrop, path::PathBuf};
use std::{mem::ManuallyDrop, path::PathBuf};

pub use lockfile::{patches, subgraph, transitive_closure};
use turbopath::AbsoluteSystemPathBuf;
Expand Down Expand Up @@ -155,7 +155,7 @@ pub extern "C" fn recursive_copy(buffer: Buffer) -> Buffer {
}
};

let response = match turborepo_fs::recursive_copy(src.borrow(), dst.borrow()) {
let response = match turborepo_fs::recursive_copy(src, dst) {
Ok(()) => proto::RecursiveCopyResponse { error: None },
Err(e) => proto::RecursiveCopyResponse {
error: Some(e.to_string()),
Expand Down
43 changes: 29 additions & 14 deletions crates/turborepo-fs/src/lib.rs
Expand Up @@ -4,7 +4,12 @@ use anyhow::Result;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
use walkdir::WalkDir;

pub fn recursive_copy(src: &AbsoluteSystemPath, dst: &AbsoluteSystemPath) -> Result<()> {
pub fn recursive_copy(
src: impl AsRef<AbsoluteSystemPath>,
dst: impl AsRef<AbsoluteSystemPath>,
) -> Result<()> {
let src = src.as_ref();
let dst = dst.as_ref();
let src_metadata = src.symlink_metadata()?;
if src_metadata.is_dir() {
let walker = WalkDir::new(src.as_path()).follow_links(false);
Expand Down Expand Up @@ -44,9 +49,9 @@ pub fn recursive_copy(src: &AbsoluteSystemPath, dst: &AbsoluteSystemPath) -> Res
let target = dst.resolve(&suffix);
if is_dir_or_symlink_to_dir {
let src_metadata = entry.metadata()?;
make_dir_copy(target.as_absolute_path(), &src_metadata)?;
make_dir_copy(&target, &src_metadata)?;
} else {
copy_file_with_type(&path, file_type, target.as_absolute_path())?;
copy_file_with_type(&path, file_type, &target)?;
}
}
}
Expand All @@ -57,7 +62,8 @@ pub fn recursive_copy(src: &AbsoluteSystemPath, dst: &AbsoluteSystemPath) -> Res
}
}

fn make_dir_copy(dir: &AbsoluteSystemPath, src_metadata: &Metadata) -> Result<()> {
fn make_dir_copy(dir: impl AsRef<AbsoluteSystemPath>, src_metadata: &Metadata) -> Result<()> {
let dir = dir.as_ref();
let mut builder = DirBuilder::new();
#[cfg(not(windows))]
{
Expand All @@ -69,16 +75,22 @@ fn make_dir_copy(dir: &AbsoluteSystemPath, src_metadata: &Metadata) -> Result<()
Ok(())
}

pub fn copy_file(from: &AbsoluteSystemPath, to: &AbsoluteSystemPath) -> Result<()> {
pub fn copy_file(
from: impl AsRef<AbsoluteSystemPath>,
to: impl AsRef<AbsoluteSystemPath>,
) -> Result<()> {
let from = from.as_ref();
let metadata = from.symlink_metadata()?;
copy_file_with_type(from, metadata.file_type(), to)
}

fn copy_file_with_type(
from: &AbsoluteSystemPath,
from: impl AsRef<AbsoluteSystemPath>,
from_type: fs::FileType,
to: &AbsoluteSystemPath,
to: impl AsRef<AbsoluteSystemPath>,
) -> Result<()> {
let from = from.as_ref();
let to = to.as_ref();
if from_type.is_symlink() {
let target = from.read_link()?;
to.ensure_dir()?;
Expand All @@ -98,13 +110,13 @@ fn copy_file_with_type(
mod tests {
use std::{io, path::Path};

use turbopath::PathError;
use turbopath::{AbsoluteSystemPathBuf, PathError};

use super::*;

fn tmp_dir() -> Result<(tempfile::TempDir, AbsoluteSystemPathBuf)> {
fn tmp_dir<'a>() -> Result<(tempfile::TempDir, AbsoluteSystemPathBuf)> {
let tmp_dir = tempfile::tempdir()?;
let dir = AbsoluteSystemPathBuf::new(tmp_dir.path().to_path_buf())?;
let dir = AbsoluteSystemPathBuf::new(tmp_dir.path())?;
Ok((tmp_dir, dir))
}

Expand All @@ -116,7 +128,7 @@ mod tests {
let (_dst_tmp, dst_dir) = tmp_dir()?;
let dst_file = dst_dir.join_literal("dest");

let err = copy_file(&src_file, &dst_file).unwrap_err();
let err = copy_file(src_file, dst_file).unwrap_err();
let err = err.downcast::<PathError>()?;
assert_eq!(err.is_io_error(io::ErrorKind::NotFound), true);
Ok(())
Expand Down Expand Up @@ -153,7 +165,7 @@ mod tests {
src_target.create_with_contents("target")?;
src_symlink.symlink_to_file(src_target.as_path())?;

copy_file(src_symlink.as_ref(), dst_file.as_ref())?;
copy_file(&src_symlink, &dst_file)?;
assert_target_matches(&dst_file, &src_target);
Ok(())
}
Expand Down Expand Up @@ -238,13 +250,16 @@ mod tests {
Ok(())
}

fn assert_file_matches(a: &AbsoluteSystemPath, b: &AbsoluteSystemPath) {
fn assert_file_matches(a: impl AsRef<AbsoluteSystemPath>, b: impl AsRef<AbsoluteSystemPath>) {
let a = a.as_ref();
let b = b.as_ref();
let a_contents = fs::read_to_string(a.as_path()).unwrap();
let b_contents = fs::read_to_string(b.as_path()).unwrap();
assert_eq!(a_contents, b_contents);
}

fn assert_target_matches<P: AsRef<Path>>(link: &AbsoluteSystemPath, expected: P) {
fn assert_target_matches(link: impl AsRef<AbsoluteSystemPath>, expected: impl AsRef<Path>) {
let link = link.as_ref();
let path = link.read_link().unwrap();
assert_eq!(path.as_path(), expected.as_ref());
}
Expand Down
1 change: 0 additions & 1 deletion crates/turborepo-lib/src/config/repo.rs
Expand Up @@ -180,7 +180,6 @@ mod test {
use std::io::Write;

use tempfile::NamedTempFile;
use turbopath::AbsoluteSystemPath;

use super::*;

Expand Down
16 changes: 9 additions & 7 deletions crates/turborepo-scm/src/git.rs
Expand Up @@ -33,7 +33,7 @@ pub fn changed_files(
) -> Result<HashSet<String>, Error> {
let git_root = AbsoluteSystemPathBuf::new(git_root)?;
let turbo_root = AbsoluteSystemPathBuf::new(turbo_root)?;
let turbo_root_relative_to_git_root = git_root.anchor(turbo_root.borrow())?;
let turbo_root_relative_to_git_root = git_root.anchor(&turbo_root)?;
let pathspec = turbo_root_relative_to_git_root.to_str()?;

let mut files = HashSet::new();
Expand All @@ -44,7 +44,7 @@ pub fn changed_files(
pathspec,
)?;

add_files_from_stdout(&mut files, git_root.borrow(), turbo_root.borrow(), output);
add_files_from_stdout(&mut files, &git_root, &turbo_root, output);

if let Some(from_commit) = from_commit {
let output = execute_git_command(
Expand All @@ -57,7 +57,7 @@ pub fn changed_files(
pathspec,
)?;

add_files_from_stdout(&mut files, git_root.borrow(), turbo_root.borrow(), output);
add_files_from_stdout(&mut files, &git_root, &turbo_root, output);
}

let output = execute_git_command(
Expand All @@ -66,7 +66,7 @@ pub fn changed_files(
pathspec,
)?;

add_files_from_stdout(&mut files, git_root.borrow(), turbo_root.borrow(), output);
add_files_from_stdout(&mut files, &git_root, &turbo_root, output);

Ok(files)
}
Expand Down Expand Up @@ -99,10 +99,12 @@ fn add_pathspec(command: &mut Command, pathspec: &str) {

fn add_files_from_stdout(
files: &mut HashSet<String>,
git_root: &AbsoluteSystemPath,
turbo_root: &AbsoluteSystemPath,
git_root: impl AsRef<AbsoluteSystemPath>,
turbo_root: impl AsRef<AbsoluteSystemPath>,
stdout: Vec<u8>,
) {
let git_root = git_root.as_ref();
let turbo_root = turbo_root.as_ref();
let stdout = String::from_utf8(stdout).unwrap();
for line in stdout.lines() {
let path = RelativeUnixPath::new(&line).unwrap();
Expand Down Expand Up @@ -149,7 +151,7 @@ pub fn previous_content(
// Note that we assume any relative file path is relative to the git root
let anchored_file_path = if file_path.is_absolute() {
let absolute_file_path = AbsoluteSystemPathBuf::new(file_path)?;
git_root.anchor(absolute_file_path.borrow())?
git_root.anchor(&absolute_file_path)?
} else {
file_path.as_path().try_into()?
};
Expand Down

0 comments on commit e71fda3

Please sign in to comment.