Fix mv data loss when changing folder case (step 1) (#6599)

* Fix mv data loss when changing folder case (step 1)

* Use same-file to detect when changing case on Windows
This commit is contained in:
Reilly Wood 2022-09-23 11:09:31 -07:00 committed by GitHub
parent d323ac3edc
commit 848550771a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 65 additions and 1 deletions

1
Cargo.lock generated
View file

@ -2738,6 +2738,7 @@ dependencies = [
"rstest",
"rusqlite",
"rust-embed",
"same-file",
"serde",
"serde_ini",
"serde_urlencoded",

View file

@ -70,6 +70,7 @@ rayon = "1.5.1"
reqwest = {version = "0.11", features = ["blocking", "json"] }
roxmltree = "0.14.0"
rust-embed = "6.3.0"
same-file = "1.0.6"
serde = { version="1.0.123", features=["derive"] }
serde_ini = "0.2.0"
serde_urlencoded = "0.7.0"

View file

@ -253,8 +253,12 @@ fn move_file(
return Err(ShellError::DirectoryNotFound(to_span, None));
}
// This can happen when changing case on a case-insensitive filesystem (ex: changing foo to Foo on Windows)
// When it does, we want to do a plain rename instead of moving `from` into `to`
let from_to_are_same_file = same_file::is_same_file(&from, &to).unwrap_or(false);
let mut to = to;
if to.is_dir() {
if !from_to_are_same_file && to.is_dir() {
let from_file_name = match from.file_name() {
Some(name) => name,
None => return Err(ShellError::DirectoryNotFound(to_span, None)),

View file

@ -393,3 +393,61 @@ fn mv_directory_with_same_name() {
assert!(actual.err.contains("Directory not empty"));
})
}
#[test]
// Test that changing the case of a file/directory name works;
// this is an important edge case on Windows (and any other case-insensitive file systems).
// We were bitten badly by this once: https://github.com/nushell/nushell/issues/6583
fn mv_change_case_of_directory() {
Playground::setup("mv_change_case_of_directory", |dirs, sandbox| {
sandbox
.mkdir("somedir")
.with_files(vec![EmptyFile("somedir/somefile.txt")]);
let original_dir = String::from("somedir");
let new_dir = String::from("SomeDir");
nu!(
cwd: dirs.test(),
format!("mv {original_dir} {new_dir}")
);
// Doing this instead of `Path::exists()` because we need to check file existence in
// a case-sensitive way. `Path::exists()` is understandably case-insensitive on NTFS
let files_in_test_directory: Vec<String> = std::fs::read_dir(dirs.test())
.unwrap()
.map(|de| de.unwrap().file_name().to_string_lossy().into_owned())
.collect();
assert!(!files_in_test_directory.contains(&original_dir));
assert!(files_in_test_directory.contains(&new_dir));
assert!(files_exist_at(
vec!["somefile.txt",],
dirs.test().join(new_dir)
));
})
}
#[test]
fn mv_change_case_of_file() {
Playground::setup("mv_change_case_of_file", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("somefile.txt")]);
let original_file_name = String::from("somefile.txt");
let new_file_name = String::from("SomeFile.txt");
nu!(
cwd: dirs.test(),
format!("mv {original_file_name} {new_file_name}")
);
// Doing this instead of `Path::exists()` because we need to check file existence in
// a case-sensitive way. `Path::exists()` is understandably case-insensitive on NTFS
let files_in_test_directory: Vec<String> = std::fs::read_dir(dirs.test())
.unwrap()
.map(|de| de.unwrap().file_name().to_string_lossy().into_owned())
.collect();
assert!(!files_in_test_directory.contains(&original_file_name));
assert!(files_in_test_directory.contains(&new_file_name));
})
}