mirror of
https://github.com/nushell/nushell.git
synced 2024-11-21 16:03:19 +01:00
no deref in touch (#14214)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> Adds --no-deref flag to `touch`. Nice and backwards compatible, and I get to touch symlinks. I still don't get to set their dates directly, but maybe that'll come with utouch. Some sadness in the implementation, since `set_symlink_file_times` doesn't take Option values and we call it twice with the old "read" values from reference (or now, if missing). This shouldn't be a big concern since `touch` already did two calls if you set both mtime and atime. Also, `--no-deref` applies both to the reference file, and to the target file. No splitting them up, because that's silly. Can always bikeshed. I nicked `--no-deref` from the uutils flag, and made the short flag `-d` because it obviously can't be `-h`. I thought of `-S` like in `glob`, for the "negative/filter out" uppercase short letters. Ultimately I don't think it matters much. Should fix #14212 since it's not really tied to uutils, besides the comment about setting a `datetime` value directly. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> New flag. # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> Maybe. # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
This commit is contained in:
Doru
GitHub
parent
1c3ff179bc
commit
a935e0720f
@ -48,6 +48,11 @@ impl Command for Touch {
|
||||
"do not create the file if it does not exist",
|
||||
Some('c'),
|
||||
)
|
||||
.switch(
|
||||
"no-deref",
|
||||
"do not follow symlinks",
|
||||
Some('s')
|
||||
)
|
||||
.category(Category::FileSystem)
|
||||
}
|
||||
|
||||
@ -64,6 +69,7 @@ impl Command for Touch {
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
let mut change_mtime: bool = call.has_flag(engine_state, stack, "modified")?;
|
||||
let mut change_atime: bool = call.has_flag(engine_state, stack, "access")?;
|
||||
let no_follow_symlinks: bool = call.has_flag(engine_state, stack, "no-deref")?;
|
||||
let reference: Option<Spanned<String>> = call.get_flag(engine_state, stack, "reference")?;
|
||||
let no_create: bool = call.has_flag(engine_state, stack, "no-create")?;
|
||||
let files: Vec<Spanned<NuGlob>> = get_rest_for_glob_pattern(engine_state, stack, call, 0)?;
|
||||
@ -88,19 +94,29 @@ impl Command for Touch {
|
||||
|
||||
if let Some(reference) = reference {
|
||||
let reference_path = nu_path::expand_path_with(reference.item, &cwd, true);
|
||||
if !reference_path.exists() {
|
||||
let exists = if no_follow_symlinks {
|
||||
// There's no symlink_exists function, so we settle for
|
||||
// getting direct metadata and if it's OK, it exists
|
||||
reference_path.symlink_metadata().is_ok()
|
||||
} else {
|
||||
reference_path.exists()
|
||||
};
|
||||
if !exists {
|
||||
return Err(ShellError::FileNotFoundCustom {
|
||||
msg: "Reference path not found".into(),
|
||||
span: reference.span,
|
||||
});
|
||||
}
|
||||
|
||||
let metadata = reference_path
|
||||
.metadata()
|
||||
.map_err(|err| ShellError::IOErrorSpanned {
|
||||
msg: format!("Failed to read metadata: {err}"),
|
||||
span: reference.span,
|
||||
})?;
|
||||
let metadata = if no_follow_symlinks {
|
||||
reference_path.symlink_metadata()
|
||||
} else {
|
||||
reference_path.metadata()
|
||||
};
|
||||
let metadata = metadata.map_err(|err| ShellError::IOErrorSpanned {
|
||||
msg: format!("Failed to read metadata: {err}"),
|
||||
span: reference.span,
|
||||
})?;
|
||||
mtime = metadata
|
||||
.modified()
|
||||
.map_err(|err| ShellError::IOErrorSpanned {
|
||||
@ -117,14 +133,27 @@ impl Command for Touch {
|
||||
|
||||
for glob in files {
|
||||
let path = expand_path_with(glob.item.as_ref(), &cwd, glob.item.is_expand());
|
||||
let exists = if no_follow_symlinks {
|
||||
path.symlink_metadata().is_ok()
|
||||
} else {
|
||||
path.exists()
|
||||
};
|
||||
|
||||
// If --no-create is passed and the file/dir does not exist there's nothing to do
|
||||
if no_create && !path.exists() {
|
||||
if no_create && !exists {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Create a file at the given path unless the path is a directory
|
||||
if !path.is_dir() {
|
||||
// If --no-deref was passed in, the behavior of touch is to error on missing
|
||||
if no_follow_symlinks && !exists {
|
||||
return Err(ShellError::FileNotFound {
|
||||
file: path.to_string_lossy().into_owned(),
|
||||
span: glob.span,
|
||||
});
|
||||
}
|
||||
|
||||
// Create a file at the given path unless the path is a directory (or a symlink with -d)
|
||||
if !path.is_dir() && (!no_follow_symlinks || !path.is_symlink()) {
|
||||
if let Err(err) = OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
@ -138,9 +167,31 @@ impl Command for Touch {
|
||||
};
|
||||
}
|
||||
|
||||
// We have to inefficiently access the target metadata to not reset it
|
||||
// in set_symlink_file_times, because the filetime doesn't expose individual methods for it
|
||||
let get_target_metadata = || {
|
||||
path.symlink_metadata()
|
||||
.map_err(|err| ShellError::IOErrorSpanned {
|
||||
msg: format!("Failed to read metadata: {err}"),
|
||||
span: glob.span,
|
||||
})
|
||||
};
|
||||
|
||||
if change_mtime {
|
||||
if let Err(err) = filetime::set_file_mtime(&path, FileTime::from_system_time(mtime))
|
||||
{
|
||||
let result = if no_follow_symlinks {
|
||||
filetime::set_symlink_file_times(
|
||||
&path,
|
||||
if change_atime {
|
||||
FileTime::from_system_time(atime)
|
||||
} else {
|
||||
FileTime::from_system_time(get_target_metadata()?.accessed()?)
|
||||
},
|
||||
FileTime::from_system_time(mtime),
|
||||
)
|
||||
} else {
|
||||
filetime::set_file_mtime(&path, FileTime::from_system_time(mtime))
|
||||
};
|
||||
if let Err(err) = result {
|
||||
return Err(ShellError::ChangeModifiedTimeNotPossible {
|
||||
msg: format!("Failed to change the modified time: {err}"),
|
||||
span: glob.span,
|
||||
@ -149,8 +200,20 @@ impl Command for Touch {
|
||||
}
|
||||
|
||||
if change_atime {
|
||||
if let Err(err) = filetime::set_file_atime(&path, FileTime::from_system_time(atime))
|
||||
{
|
||||
let result = if no_follow_symlinks {
|
||||
filetime::set_symlink_file_times(
|
||||
&path,
|
||||
FileTime::from_system_time(atime),
|
||||
if change_mtime {
|
||||
FileTime::from_system_time(mtime)
|
||||
} else {
|
||||
FileTime::from_system_time(get_target_metadata()?.modified()?)
|
||||
},
|
||||
)
|
||||
} else {
|
||||
filetime::set_file_atime(&path, FileTime::from_system_time(atime))
|
||||
};
|
||||
if let Err(err) = result {
|
||||
return Err(ShellError::ChangeAccessTimeNotPossible {
|
||||
msg: format!("Failed to change the access time: {err}"),
|
||||
span: glob.span,
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
use chrono::{DateTime, Local};
|
||||
use nu_test_support::fs::{files_exist_at, Stub};
|
||||
use nu_test_support::nu;
|
||||
use nu_test_support::playground::Playground;
|
||||
use nu_test_support::playground::{Dirs, Playground};
|
||||
|
||||
// Use 1 instead of 0 because 0 has a special meaning in Windows
|
||||
const TIME_ONE: filetime::FileTime = filetime::FileTime::from_unix_time(1, 0);
|
||||
@ -527,3 +527,128 @@ fn reference_respects_cwd() {
|
||||
assert!(path.exists());
|
||||
})
|
||||
}
|
||||
|
||||
fn setup_symlink_fs(dirs: &Dirs, sandbox: &mut Playground<'_>) {
|
||||
sandbox.mkdir("d");
|
||||
sandbox.with_files(&[Stub::EmptyFile("f"), Stub::EmptyFile("d/f")]);
|
||||
sandbox.symlink("f", "fs");
|
||||
sandbox.symlink("d", "ds");
|
||||
sandbox.symlink("d/f", "fds");
|
||||
|
||||
// sandbox.symlink does not handle symlinks to missing files well. It panics
|
||||
// But they are useful, and they should be tested.
|
||||
#[cfg(unix)]
|
||||
{
|
||||
std::os::unix::fs::symlink(dirs.test().join("m"), dirs.test().join("fms")).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
std::os::windows::fs::symlink_file(dirs.test().join("m"), dirs.test().join("fms")).unwrap();
|
||||
}
|
||||
|
||||
// Change the file times to a known "old" value for comparison
|
||||
filetime::set_symlink_file_times(dirs.test().join("f"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("d"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("d/f"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("ds"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("fs"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("fds"), TIME_ONE, TIME_ONE).unwrap();
|
||||
filetime::set_symlink_file_times(dirs.test().join("fms"), TIME_ONE, TIME_ONE).unwrap();
|
||||
}
|
||||
|
||||
fn get_times(path: &nu_path::AbsolutePath) -> (filetime::FileTime, filetime::FileTime) {
|
||||
let metadata = path.symlink_metadata().unwrap();
|
||||
|
||||
(
|
||||
filetime::FileTime::from_system_time(metadata.accessed().unwrap()),
|
||||
filetime::FileTime::from_system_time(metadata.modified().unwrap()),
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn follow_symlinks() {
|
||||
Playground::setup("touch_follows_symlinks", |dirs, sandbox| {
|
||||
setup_symlink_fs(&dirs, sandbox);
|
||||
|
||||
let missing = dirs.test().join("m");
|
||||
assert!(!missing.exists());
|
||||
|
||||
nu!(
|
||||
cwd: dirs.test(),
|
||||
"
|
||||
touch fds
|
||||
touch ds
|
||||
touch fs
|
||||
touch fms
|
||||
"
|
||||
);
|
||||
|
||||
// We created the missing symlink target
|
||||
assert!(missing.exists());
|
||||
|
||||
// The timestamps for files and directories were changed from TIME_ONE
|
||||
let file_times = get_times(&dirs.test().join("f"));
|
||||
let dir_times = get_times(&dirs.test().join("d"));
|
||||
let dir_file_times = get_times(&dirs.test().join("d/f"));
|
||||
|
||||
assert_ne!(file_times, (TIME_ONE, TIME_ONE));
|
||||
assert_ne!(dir_times, (TIME_ONE, TIME_ONE));
|
||||
assert_ne!(dir_file_times, (TIME_ONE, TIME_ONE));
|
||||
|
||||
// For symlinks, they remain (mostly) the same
|
||||
// We can't test accessed times, since to reach the target file, the symlink must be accessed!
|
||||
let file_symlink_times = get_times(&dirs.test().join("fs"));
|
||||
let dir_symlink_times = get_times(&dirs.test().join("ds"));
|
||||
let dir_file_symlink_times = get_times(&dirs.test().join("fds"));
|
||||
let file_missing_symlink_times = get_times(&dirs.test().join("fms"));
|
||||
|
||||
assert_eq!(file_symlink_times.1, TIME_ONE);
|
||||
assert_eq!(dir_symlink_times.1, TIME_ONE);
|
||||
assert_eq!(dir_file_symlink_times.1, TIME_ONE);
|
||||
assert_eq!(file_missing_symlink_times.1, TIME_ONE);
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_follow_symlinks() {
|
||||
Playground::setup("touch_touches_symlinks", |dirs, sandbox| {
|
||||
setup_symlink_fs(&dirs, sandbox);
|
||||
|
||||
let missing = dirs.test().join("m");
|
||||
assert!(!missing.exists());
|
||||
|
||||
nu!(
|
||||
cwd: dirs.test(),
|
||||
"
|
||||
touch fds -s
|
||||
touch ds -s
|
||||
touch fs -s
|
||||
touch fms -s
|
||||
"
|
||||
);
|
||||
|
||||
// We did not create the missing symlink target
|
||||
assert!(!missing.exists());
|
||||
|
||||
// The timestamps for files and directories remain the same
|
||||
let file_times = get_times(&dirs.test().join("f"));
|
||||
let dir_times = get_times(&dirs.test().join("d"));
|
||||
let dir_file_times = get_times(&dirs.test().join("d/f"));
|
||||
|
||||
assert_eq!(file_times, (TIME_ONE, TIME_ONE));
|
||||
assert_eq!(dir_times, (TIME_ONE, TIME_ONE));
|
||||
assert_eq!(dir_file_times, (TIME_ONE, TIME_ONE));
|
||||
|
||||
// For symlinks, everything changed. (except their targets, and paths, and personality)
|
||||
let file_symlink_times = get_times(&dirs.test().join("fs"));
|
||||
let dir_symlink_times = get_times(&dirs.test().join("ds"));
|
||||
let dir_file_symlink_times = get_times(&dirs.test().join("fds"));
|
||||
let file_missing_symlink_times = get_times(&dirs.test().join("fms"));
|
||||
|
||||
assert_ne!(file_symlink_times, (TIME_ONE, TIME_ONE));
|
||||
assert_ne!(dir_symlink_times, (TIME_ONE, TIME_ONE));
|
||||
assert_ne!(dir_file_symlink_times, (TIME_ONE, TIME_ONE));
|
||||
assert_ne!(file_missing_symlink_times, (TIME_ONE, TIME_ONE));
|
||||
})
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user