Skip to content

Commit

Permalink
fix removing symlinks on windows (#9704)
Browse files Browse the repository at this point in the history
this PR should close #9624

# Description

Fixes the `rm` command assuming that a symlink is a directory and trying
to delete the directory as opposed to unlinking the symlink.

Should probably be tested on linux before merge.

Added tests for deleting symlinks
  • Loading branch information
cramt committed Jul 20, 2023
1 parent c62cbcd commit bd9d865
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
12 changes: 10 additions & 2 deletions crates/nu-command/src/filesystem/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,11 @@ fn rm(
trash::delete(&f).map_err(|e: trash::Error| {
Error::new(ErrorKind::Other, format!("{e:?}\nTry '--trash' flag"))
})
} else if metadata.is_file() || is_socket || is_fifo {
} else if metadata.is_file()
|| is_socket
|| is_fifo
|| metadata.file_type().is_symlink()
{
std::fs::remove_file(&f)
} else {
std::fs::remove_dir_all(&f)
Expand All @@ -411,7 +415,11 @@ fn rm(
Err(e)
} else if interactive && !confirmed {
Ok(())
} else if metadata.is_file() || is_socket || is_fifo {
} else if metadata.is_file()
|| is_socket
|| is_fifo
|| metadata.file_type().is_symlink()
{
std::fs::remove_file(&f)
} else {
std::fs::remove_dir_all(&f)
Expand Down
23 changes: 23 additions & 0 deletions crates/nu-command/tests/commands/rm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,29 @@ fn remove_ignores_ansi() {
});
}

#[test]
fn removes_symlink() {
let symlink_target = "symlink_target";
let symlink = "symlink";
Playground::setup("rm_test_symlink", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile(symlink_target)]);

#[cfg(not(windows))]
std::os::unix::fs::symlink(dirs.test().join(symlink_target), dirs.test().join(symlink))
.unwrap();
#[cfg(windows)]
std::os::windows::fs::symlink_file(
dirs.test().join(symlink_target),
dirs.test().join(symlink),
)
.unwrap();

let _ = nu!(cwd: sandbox.cwd(), "rm symlink");

assert!(!dirs.test().join(symlink).exists());
});
}

struct Cleanup<'a> {
dir_to_clean: &'a PathBuf,
}
Expand Down

0 comments on commit bd9d865

Please sign in to comment.