From 3e6e5f77e378968139510275b1bd7d7ab9b8bd2b Mon Sep 17 00:00:00 2001 From: Noam Yorav-Raphael Date: Mon, 1 Jul 2024 01:26:30 +0300 Subject: [PATCH] Add -L / --dereference option (#36) Co-authored-by: Noam Yorav-Raphael Co-authored-by: Alex Saveau --- cpz/README.md | 3 + cpz/command-reference-short.golden | 2 + cpz/command-reference.golden | 3 + cpz/src/main.rs | 9 +++ fuc_engine/api.golden | 2 +- fuc_engine/src/ops/copy.rs | 97 +++++++++++++++++++++--------- fuc_engine/src/ops/mod.rs | 8 ++- fuc_engine/src/ops/remove.rs | 4 +- fuc_engine/tests/copy.rs | 88 +++++++++++++++++++++++++++ 9 files changed, 184 insertions(+), 32 deletions(-) diff --git a/cpz/README.md b/cpz/README.md index 0f14e19..d8c6ba9 100644 --- a/cpz/README.md +++ b/cpz/README.md @@ -80,6 +80,9 @@ Options: -t, --reverse-args Reverse the argument order so that it becomes `cpz ...` + -L, --dereference + Follow symlinks in the files to be copied rather than copying the symlinks themselves + -h, --help Print help (use `-h` for a summary) diff --git a/cpz/command-reference-short.golden b/cpz/command-reference-short.golden index 6a8044a..9e79fc7 100644 --- a/cpz/command-reference-short.golden +++ b/cpz/command-reference-short.golden @@ -9,5 +9,7 @@ Arguments: Options: -f, --force Overwrite existing files -t, --reverse-args Reverse the argument order so that it becomes `cpz ...` + -L, --dereference Follow symlinks in the files to be copied rather than copying the symlinks + themselves -h, --help Print help (use `--help` for more detail) -V, --version Print version diff --git a/cpz/command-reference.golden b/cpz/command-reference.golden index 895a6ad..d2880f6 100644 --- a/cpz/command-reference.golden +++ b/cpz/command-reference.golden @@ -20,6 +20,9 @@ Options: -t, --reverse-args Reverse the argument order so that it becomes `cpz ...` + -L, --dereference + Follow symlinks in the files to be copied rather than copying the symlinks themselves + -h, --help Print help (use `-h` for a summary) diff --git a/cpz/src/main.rs b/cpz/src/main.rs index e796ff0..2809559 100644 --- a/cpz/src/main.rs +++ b/cpz/src/main.rs @@ -43,6 +43,12 @@ struct Cpz { #[arg(short = 't', long, default_value_t = false)] reverse_args: bool, + /// Follow symlinks in the files to be copied rather than copying the + /// symlinks themselves + #[arg(short = 'L', long, default_value_t = false)] + #[arg(aliases = ["follow-symlinks"])] + dereference: bool, + #[arg(short, long, short_alias = '?', global = true)] #[arg(action = ArgAction::Help, help = "Print help (use `--help` for more detail)")] #[arg(long_help = "Print help (use `-h` for a summary)")] @@ -203,6 +209,7 @@ fn copy( mut to, force, reverse_args, + dereference, help: _, }: Cpz, ) -> Result<(), Error> { @@ -243,6 +250,7 @@ fn copy( (path, to) })) .force(force) + .follow_symlinks(dereference) .build() .run() } else { @@ -261,6 +269,7 @@ fn copy( (from, to) }]) .force(force) + .follow_symlinks(dereference) .build() .run() } diff --git a/fuc_engine/api.golden b/fuc_engine/api.golden index 89c08ed..72d4149 100644 --- a/fuc_engine/api.golden +++ b/fuc_engine/api.golden @@ -46,7 +46,7 @@ pub struct fuc_engine::CopyOp<'a, 'b, I1: core::convert::Into> + 'a, I2: core::convert::Into> + 'b, F: core::iter::traits::collect::IntoIterator> fuc_engine::CopyOp<'a, 'b, I1, I2, F> pub fn fuc_engine::CopyOp<'a, 'b, I1, I2, F>::run(self) -> core::result::Result<(), fuc_engine::Error> impl<'a, 'b, I1: core::convert::Into> + 'a, I2: core::convert::Into> + 'b, F: core::iter::traits::collect::IntoIterator> fuc_engine::CopyOp<'a, 'b, I1, I2, F> -pub fn fuc_engine::CopyOp<'a, 'b, I1, I2, F>::builder() -> CopyOpBuilder<'a, 'b, I1, I2, F, ((), (), (), ())> +pub fn fuc_engine::CopyOp<'a, 'b, I1, I2, F>::builder() -> CopyOpBuilder<'a, 'b, I1, I2, F, ((), (), (), (), ())> impl<'a, 'b, I1: core::fmt::Debug + core::convert::Into> + 'a, I2: core::fmt::Debug + core::convert::Into> + 'b, F: core::fmt::Debug + core::iter::traits::collect::IntoIterator> core::fmt::Debug for fuc_engine::CopyOp<'a, 'b, I1, I2, F> pub fn fuc_engine::CopyOp<'a, 'b, I1, I2, F>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl<'a, 'b, I1, I2, F> core::marker::Freeze for fuc_engine::CopyOp<'a, 'b, I1, I2, F> where F: core::marker::Freeze diff --git a/fuc_engine/src/ops/copy.rs b/fuc_engine/src/ops/copy.rs index 7a96825..f76a60f 100644 --- a/fuc_engine/src/ops/copy.rs +++ b/fuc_engine/src/ops/copy.rs @@ -30,6 +30,8 @@ pub struct CopyOp< files: F, #[builder(default = false)] force: bool, + #[builder(default = false)] + follow_symlinks: bool, #[builder(default)] _marker1: PhantomData<&'a I1>, #[builder(default)] @@ -50,7 +52,7 @@ impl< /// /// Returns the underlying I/O errors that occurred. pub fn run(self) -> Result<(), Error> { - let copy = compat::copy_impl(); + let copy = compat::copy_impl(self.follow_symlinks); let result = schedule_copies(self, ©); copy.finish().and(result) } @@ -70,6 +72,7 @@ fn schedule_copies< CopyOp { files, force, + follow_symlinks, _marker1: _, _marker2: _, }: CopyOp<'a, 'b, I1, I2, F>, @@ -94,9 +97,12 @@ fn schedule_copies< } } - let from_metadata = from - .symlink_metadata() - .map_io_err(|| format!("Failed to read metadata for file: {from:?}"))?; + let from_metadata = if follow_symlinks { + from.metadata() + } else { + from.symlink_metadata() + } + .map_io_err(|| format!("Failed to read metadata for file: {from:?}"))?; if let Some(parent) = to.parent() { fs::create_dir_all(parent) @@ -106,14 +112,7 @@ fn schedule_copies< #[cfg(unix)] if from_metadata.is_dir() { use std::os::unix::fs::{DirBuilderExt, MetadataExt}; - match fs::DirBuilder::new() - .mode( - fs::symlink_metadata(&from) - .map_io_err(|| format!("Failed to stat directory: {from:?}"))? - .mode(), - ) - .create(&to) - { + match fs::DirBuilder::new().mode(from_metadata.mode()).create(&to) { Err(e) if force && e.kind() == io::ErrorKind::AlreadyExists => {} r => r.map_io_err(|| format!("Failed to create directory: {to:?}"))?, }; @@ -178,10 +177,15 @@ mod compat { scheduling: LazyCell<(Sender, JoinHandle>), LF>, } - pub fn copy_impl<'a, 'b>() -> impl DirectoryOp<(Cow<'a, Path>, Cow<'b, Path>)> { - let scheduling = LazyCell::new(|| { + pub fn copy_impl<'a, 'b>( + follow_symlinks: bool, + ) -> impl DirectoryOp<(Cow<'a, Path>, Cow<'b, Path>)> { + let scheduling = LazyCell::new(move || { let (tx, rx) = crossbeam_channel::unbounded(); - (tx, thread::spawn(|| root_worker_thread(rx))) + ( + tx, + thread::spawn(move || root_worker_thread(rx, follow_symlinks)), + ) }); Impl { scheduling } @@ -222,7 +226,7 @@ mod compat { } #[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(tasks)))] - fn root_worker_thread(tasks: Receiver) -> Result<(), Error> { + fn root_worker_thread(tasks: Receiver, follow_symlinks: bool) -> Result<(), Error> { unshare_files()?; let mut available_parallelism = thread::available_parallelism() @@ -266,7 +270,7 @@ mod compat { available_parallelism -= 1; threads.push(scope.spawn({ let tasks = tasks.clone(); - move || worker_thread(tasks, root_to_inode) + move || worker_thread(tasks, root_to_inode, follow_symlinks) })); } }; @@ -275,6 +279,7 @@ mod compat { copy_dir( node, root_to_inode, + follow_symlinks, &mut buf, &symlink_buf_cache, maybe_spawn, @@ -290,13 +295,24 @@ mod compat { } #[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(tasks)))] - fn worker_thread(tasks: Receiver, root_to_inode: u64) -> Result<(), Error> { + fn worker_thread( + tasks: Receiver, + root_to_inode: u64, + follow_symlinks: bool, + ) -> Result<(), Error> { unshare_files()?; let mut buf = [MaybeUninit::::uninit(); 8192]; let symlink_buf_cache = Cell::new(Vec::new()); for node in tasks { - copy_dir(node, root_to_inode, &mut buf, &symlink_buf_cache, || {})?; + copy_dir( + node, + root_to_inode, + follow_symlinks, + &mut buf, + &symlink_buf_cache, + || {}, + )?; } Ok(()) } @@ -308,6 +324,7 @@ mod compat { fn copy_dir( TreeNode { from, to, messages }: TreeNode, root_to_inode: u64, + follow_symlinks: bool, buf: &mut [MaybeUninit], symlink_buf_cache: &Cell>, mut maybe_spawn: impl FnMut(), @@ -315,7 +332,13 @@ mod compat { let from_dir = openat( CWD, &from, - OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW, + OFlags::RDONLY + | OFlags::DIRECTORY + | if follow_symlinks { + OFlags::empty() + } else { + OFlags::NOFOLLOW + }, Mode::empty(), ) .map_io_err(|| format!("Failed to open directory: {from:?}"))?; @@ -341,10 +364,12 @@ mod compat { } } - let file_type = match file.file_type() { - FileType::Unknown => get_file_type(&from_dir, file.file_name(), &from)?, - t => t, - }; + let mut file_type = file.file_type(); + if file_type == FileType::Unknown || (follow_symlinks && file_type == FileType::Symlink) + { + file_type = get_file_type(&from_dir, file.file_name(), &from, follow_symlinks)?; + } + let file_type = file_type; if file_type == FileType::Directory { let from = concat_cstrs(&from, file.file_name()); let to = concat_cstrs(&to, file.file_name()); @@ -586,10 +611,15 @@ mod compat { Error, }; - struct Impl; + struct Impl { + #[allow(dead_code)] + follow_symlinks: bool, + } - pub fn copy_impl<'a, 'b>() -> impl DirectoryOp<(Cow<'a, Path>, Cow<'b, Path>)> { - Impl + pub fn copy_impl<'a, 'b>( + follow_symlinks: bool, + ) -> impl DirectoryOp<(Cow<'a, Path>, Cow<'b, Path>)> { + Impl { follow_symlinks } } impl DirectoryOp<(Cow<'_, Path>, Cow<'_, Path>)> for Impl { @@ -598,6 +628,8 @@ mod compat { &from, to, #[cfg(unix)] + self.follow_symlinks, + #[cfg(unix)] None, ) .map_io_err(|| format!("Failed to copy directory: {from:?}")) @@ -611,6 +643,7 @@ mod compat { fn copy_dir, Q: AsRef>( from: P, to: Q, + #[cfg(unix)] follow_symlinks: bool, #[cfg(unix)] root_to_inode: Option, ) -> Result<(), io::Error> { let to = to.as_ref(); @@ -636,11 +669,17 @@ mod compat { } let to = to.join(dir_entry.file_name()); - let file_type = dir_entry.file_type()?; + #[allow(unused_mut)] + let mut file_type = dir_entry.file_type()?; + #[cfg(unix)] + if follow_symlinks && file_type.is_symlink() { + file_type = fs::metadata(dir_entry.path())?.file_type(); + } + let file_type = file_type; #[cfg(unix)] if file_type.is_dir() { - copy_dir(dir_entry.path(), to, root_to_inode)?; + copy_dir(dir_entry.path(), to, follow_symlinks, root_to_inode)?; } else if file_type.is_symlink() { std::os::unix::fs::symlink(fs::read_link(dir_entry.path())?, to)?; } else { diff --git a/fuc_engine/src/ops/mod.rs b/fuc_engine/src/ops/mod.rs index b1a5c34..d8b6269 100644 --- a/fuc_engine/src/ops/mod.rs +++ b/fuc_engine/src/ops/mod.rs @@ -81,8 +81,14 @@ mod linux { dir: impl AsFd, file_name: &CStr, path: &CString, + follow_symlinks: bool, ) -> Result { - statx(dir, file_name, AtFlags::SYMLINK_NOFOLLOW, StatxFlags::TYPE) + let flags = if follow_symlinks { + AtFlags::empty() + } else { + AtFlags::SYMLINK_NOFOLLOW + }; + statx(dir, file_name, flags, StatxFlags::TYPE) .map_io_err(|| { format!( "Failed to stat file: {:?}", diff --git a/fuc_engine/src/ops/remove.rs b/fuc_engine/src/ops/remove.rs index aaf80ca..eac6bc9 100644 --- a/fuc_engine/src/ops/remove.rs +++ b/fuc_engine/src/ops/remove.rs @@ -320,7 +320,9 @@ mod compat { } let file_type = match file.file_type() { - FileType::Unknown => get_file_type(&dir, file.file_name(), &node.as_ref().path)?, + FileType::Unknown => { + get_file_type(&dir, file.file_name(), &node.as_ref().path, false)? + } t => t, }; if file_type == FileType::Directory { diff --git a/fuc_engine/tests/copy.rs b/fuc_engine/tests/copy.rs index 5de165e..c90fbc4 100644 --- a/fuc_engine/tests/copy.rs +++ b/fuc_engine/tests/copy.rs @@ -1,5 +1,6 @@ use std::{borrow::Cow, fs, fs::File}; +use rstest::rstest; use tempfile::tempdir; #[test] @@ -144,3 +145,90 @@ fn symbolic_link_copy_link() { assert!(to.exists()); } + +#[rstest] +#[cfg(unix)] +fn dereference_symbolic_link_to_regular_file(#[values(false, true)] follow_symlinks: bool) { + let root = tempdir().unwrap(); + let from = root.path().join("from"); + File::create(from).unwrap(); + let link = root.path().join("link"); + std::os::unix::fs::symlink("from", &link).unwrap(); + let to = root.path().join("to"); + + fuc_engine::CopyOp::builder() + .files([(Cow::Owned(link), Cow::Borrowed(to.as_path()))]) + .follow_symlinks(follow_symlinks) + .build() + .run() + .unwrap(); + + if follow_symlinks { + assert!(to.symlink_metadata().unwrap().is_file()); + } else { + assert!(to.symlink_metadata().unwrap().is_symlink()); + } +} + +#[rstest] +#[cfg(unix)] +fn dereference_symbolic_link_to_regular_file_in_dir(#[values(false, true)] follow_symlinks: bool) { + let root = tempdir().unwrap(); + let from = root.path().join("from"); + fs::create_dir(&from).unwrap(); + File::create(from.join("file")).unwrap(); + std::os::unix::fs::symlink("file", from.join("link")).unwrap(); + let to = root.path().join("to"); + + fuc_engine::CopyOp::builder() + .files([(Cow::Owned(from), Cow::Borrowed(to.as_path()))]) + .follow_symlinks(follow_symlinks) + .build() + .run() + .unwrap(); + + assert!(to.join("file").symlink_metadata().unwrap().is_file()); + if follow_symlinks { + assert!(to.join("link").symlink_metadata().unwrap().is_file()); + } else { + assert!(to.join("link").symlink_metadata().unwrap().is_symlink()); + } +} + +#[rstest] +#[cfg(unix)] +fn dereference_symbolic_link_to_dir_in_dir(#[values(false, true)] follow_symlinks: bool) { + let root = tempdir().unwrap(); + let from = root.path().join("from"); + fs::create_dir(&from).unwrap(); + fs::create_dir(from.join("subdir")).unwrap(); + File::create(from.join("subdir/file")).unwrap(); + std::os::unix::fs::symlink("subdir", from.join("subdirlink")).unwrap(); + let to = root.path().join("to"); + + fuc_engine::CopyOp::builder() + .files([(Cow::Owned(from), Cow::Borrowed(to.as_path()))]) + .follow_symlinks(follow_symlinks) + .build() + .run() + .unwrap(); + + assert!(to.join("subdir").symlink_metadata().unwrap().is_dir()); + assert!(to.join("subdir/file").symlink_metadata().unwrap().is_file()); + if follow_symlinks { + assert!(to.join("subdirlink").symlink_metadata().unwrap().is_dir()); + assert!( + to.join("subdirlink/file") + .symlink_metadata() + .unwrap() + .is_file() + ); + } else { + assert!( + to.join("subdirlink") + .symlink_metadata() + .unwrap() + .is_symlink() + ); + } +}