From 219c719e986db46b75b8623332e2a23a057b949b Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Thu, 26 May 2022 23:42:52 +0800 Subject: [PATCH] make cp can copy folders contains dangling symbolic link (#5645) * cp with no dangling link * add -p to not follow symbolic link * change comment * add one more test case to check symblink body after copied * better help message --- crates/nu-command/src/filesystem/cp.rs | 109 +++++++++++++++--- crates/nu-command/tests/commands/cp.rs | 69 +++++++++++ crates/nu-test-support/src/playground/play.rs | 4 +- 3 files changed, 162 insertions(+), 20 deletions(-) diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index 90b25a89bd..fbf7b7dd16 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/crates/nu-command/src/filesystem/cp.rs @@ -1,8 +1,9 @@ +use std::fs::read_link; use std::path::PathBuf; use nu_engine::env::current_dir; use nu_engine::CallExt; -use nu_path::canonicalize_with; +use nu_path::{canonicalize_with, expand_path_with}; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ @@ -54,6 +55,11 @@ impl Command for Cp { // TODO: add back in additional features // .switch("force", "suppress error when no file", Some('f')) .switch("interactive", "ask user to confirm action", Some('i')) + .switch( + "no-dereference", + "If the -r option is specified, no symbolic links are followed.", + Some('p'), + ) .category(Category::FileSystem) } @@ -70,9 +76,9 @@ impl Command for Cp { let verbose = call.has_flag("verbose"); let interactive = call.has_flag("interactive"); - let path = current_dir(engine_state, stack)?; - let source = path.join(src.item.as_str()); - let destination = path.join(dst.item.as_str()); + let current_dir_path = current_dir(engine_state, stack)?; + let source = current_dir_path.join(src.item.as_str()); + let destination = current_dir_path.join(dst.item.as_str()); // check if destination is a dir and it exists let path_last_char = destination.as_os_str().to_string_lossy().chars().last(); @@ -140,7 +146,7 @@ impl Command for Cp { if entry.is_file() { let sources = sources.paths_applying_with(|(source_file, _depth_level)| { if destination.is_dir() { - let mut dest = canonicalize_with(&dst.item, &path)?; + let mut dest = canonicalize_with(&dst.item, ¤t_dir_path)?; if let Some(name) = entry.file_name() { dest.push(name); } @@ -153,7 +159,7 @@ impl Command for Cp { for (src, dst) in sources { if src.is_file() { let res = if interactive && dst.exists() { - interactive_copy_file(interactive, src, dst, span) + interactive_copy(interactive, src, dst, span, copy_file) } else { copy_file(src, dst, span) }; @@ -188,9 +194,23 @@ impl Command for Cp { ) })?; + let not_follow_symlink = call.has_flag("no-dereference"); let sources = sources.paths_applying_with(|(source_file, depth_level)| { let mut dest = destination.clone(); - let path = canonicalize_with(&source_file, &path)?; + + let path = if not_follow_symlink { + expand_path_with(&source_file, ¤t_dir_path) + } else { + canonicalize_with(&source_file, ¤t_dir_path).or_else(|err| { + // check if dangling symbolic link. + let path = expand_path_with(&source_file, ¤t_dir_path); + if path.is_symlink() && !path.exists() { + Ok(path) + } else { + Err(err) + } + })? + }; #[allow(clippy::needless_collect)] let comps: Vec<_> = path @@ -220,14 +240,21 @@ impl Command for Cp { })?; } - if s.is_file() { + if s.is_symlink() && not_follow_symlink { let res = if interactive && d.exists() { - interactive_copy_file(interactive, s, d, span) + interactive_copy(interactive, s, d, span, copy_symlink) + } else { + copy_symlink(s, d, span) + }; + result.push(res); + } else if s.is_file() { + let res = if interactive && d.exists() { + interactive_copy(interactive, s, d, span, copy_file) } else { copy_file(s, d, span) }; result.push(res); - } + }; } } } @@ -265,7 +292,13 @@ impl Command for Cp { } } -fn interactive_copy_file(interactive: bool, src: PathBuf, dst: PathBuf, span: Span) -> Value { +fn interactive_copy( + interactive: bool, + src: PathBuf, + dst: PathBuf, + span: Span, + copy_impl: impl Fn(PathBuf, PathBuf, Span) -> Value, +) -> Value { let (interaction, confirmed) = try_interaction(interactive, "cp: overwrite", &dst.to_string_lossy()); if let Err(e) = interaction { @@ -282,7 +315,7 @@ fn interactive_copy_file(interactive: bool, src: PathBuf, dst: PathBuf, span: Sp let msg = format!("{:} not copied to {:}", src.display(), dst.display()); Value::String { val: msg, span } } else { - copy_file(src, dst, span) + copy_impl(src, dst, span) } } @@ -293,13 +326,51 @@ fn copy_file(src: PathBuf, dst: PathBuf, span: Span) -> Value { Value::String { val: msg, span } } Err(e) => Value::Error { - error: ShellError::GenericError( - e.to_string(), - e.to_string(), - Some(span), - None, - Vec::new(), - ), + error: ShellError::FileNotFoundCustom(format!("copy file {src:?} failed: {e}"), span), + }, + } +} + +fn copy_symlink(src: PathBuf, dst: PathBuf, span: Span) -> Value { + let target_path = read_link(src.as_path()); + let target_path = match target_path { + Ok(p) => p, + Err(err) => { + return Value::Error { + error: ShellError::GenericError( + err.to_string(), + err.to_string(), + Some(span), + None, + vec![], + ), + } + } + }; + + let create_symlink = { + #[cfg(unix)] + { + std::os::unix::fs::symlink + } + + #[cfg(windows)] + { + if !target_path.exists() || target_path.is_file() { + std::os::windows::fs::symlink_file + } else { + std::os::windows::fs::symlink_dir + } + } + }; + + match create_symlink(target_path.as_path(), dst.as_path()) { + Ok(_) => { + let msg = format!("copied {:} to {:}", src.display(), dst.display()); + Value::String { val: msg, span } + } + Err(e) => Value::Error { + error: ShellError::GenericError(e.to_string(), e.to_string(), Some(span), None, vec![]), }, } } diff --git a/crates/nu-command/tests/commands/cp.rs b/crates/nu-command/tests/commands/cp.rs index 3fb0b97490..fc6c82b424 100644 --- a/crates/nu-command/tests/commands/cp.rs +++ b/crates/nu-command/tests/commands/cp.rs @@ -1,3 +1,4 @@ +use nu_test_support::fs::file_contents; use nu_test_support::fs::{files_exist_at, AbsoluteFile, Stub::EmptyFile}; use nu_test_support::nu; use nu_test_support::playground::Playground; @@ -249,3 +250,71 @@ fn copy_to_non_existing_dir() { assert!(actual.err.contains("destination directory does not exist")); }); } + +#[test] +fn copy_dir_contains_symlink_ignored() { + Playground::setup("cp_test_12", |_dirs, sandbox| { + sandbox + .within("tmp_dir") + .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")]) + .within("tmp_dir") + .symlink("good_bye", "dangle_symlink"); + + // make symbolic link and copy. + nu!( + cwd: sandbox.cwd(), + "rm tmp_dir/good_bye; cp -r tmp_dir tmp_dir_2", + ); + + // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` don't exists inside `tmp_dir_2`. + let expected = sandbox.cwd().join("tmp_dir_2"); + assert!(files_exist_at(vec!["hello_there"], expected.clone())); + let path = expected.join("dangle_symlink"); + assert!(!path.exists() && !path.is_symlink()); + }); +} + +#[test] +fn copy_dir_contains_symlink() { + Playground::setup("cp_test_13", |_dirs, sandbox| { + sandbox + .within("tmp_dir") + .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")]) + .within("tmp_dir") + .symlink("good_bye", "dangle_symlink"); + + // make symbolic link and copy. + nu!( + cwd: sandbox.cwd(), + "rm tmp_dir/good_bye; cp -r -p tmp_dir tmp_dir_2", + ); + + // check hello_there exists inside `tmp_dir_2`, and `dangle_symlink` also exists inside `tmp_dir_2`. + let expected = sandbox.cwd().join("tmp_dir_2"); + assert!(files_exist_at(vec!["hello_there"], expected.clone())); + let path = expected.join("dangle_symlink"); + assert!(path.is_symlink()); + }); +} + +#[test] +fn copy_dir_symlink_file_body_not_changed() { + Playground::setup("cp_test_14", |_dirs, sandbox| { + sandbox + .within("tmp_dir") + .with_files(vec![EmptyFile("hello_there"), EmptyFile("good_bye")]) + .within("tmp_dir") + .symlink("good_bye", "dangle_symlink"); + + // make symbolic link and copy. + nu!( + cwd: sandbox.cwd(), + "rm tmp_dir/good_bye; cp -r -p tmp_dir tmp_dir_2; rm -r tmp_dir; cp -r -p tmp_dir_2 tmp_dir; echo hello_data | save tmp_dir/good_bye", + ); + + // check dangle_symlink in tmp_dir is no longer dangling. + let expected_file = sandbox.cwd().join("tmp_dir").join("dangle_symlink"); + let actual = file_contents(expected_file); + assert!(actual.contains("hello_data")); + }); +} diff --git a/crates/nu-test-support/src/playground/play.rs b/crates/nu-test-support/src/playground/play.rs index 29765ccfb1..b84f6d58bb 100644 --- a/crates/nu-test-support/src/playground/play.rs +++ b/crates/nu-test-support/src/playground/play.rs @@ -228,7 +228,9 @@ impl<'a> Playground<'a> { pub fn within(&mut self, directory: &str) -> &mut Self { self.cwd.push(directory); - std::fs::create_dir(&self.cwd).expect("can not create directory"); + if !(self.cwd.exists() && self.cwd.is_dir()) { + std::fs::create_dir(&self.cwd).expect("can not create directory"); + } self }