Fix rm for symlinks pointing to directory on windows (issue #11461) (#11463)

<!--
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!
-->

- this PR closes #11461

# 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.
-->

Using `std::fs::remove_dir` instead of `std::fs::remove_file` when try
remove symlinks pointing to a directory on Windows.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

none

# 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 std testing; testing run-tests --path
crates/nu-std"` 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
> ```
-->

- [x] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used`
to check that you're using the standard code style
- [x] `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))
- I got 2 test fails on my Windows devenv; these fails in main branch
too
- `commands::complete::basic` : passed on Ubuntu, failed on Windows (a
bug?)
- `commands::cp::copy_file_with_read_permission`: failed on Windows with
Japanese environment (This test refers error message, so that fails on
environments using a language except for english.)
- [x] `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

# 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 fix has no changes to user-facing interface.
This commit is contained in:
tomoda 2024-01-02 22:27:03 +09:00 committed by GitHub
parent f597380112
commit 42bb42a2e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 5 deletions

View File

@ -381,11 +381,23 @@ fn rm(
{ {
unreachable!() unreachable!()
} }
} else if metadata.is_file() } else if metadata.is_symlink() {
|| is_socket // In Windows, symlink pointing to a directory can be removed using
|| is_fifo // std::fs::remove_dir instead of std::fs::remove_file.
|| metadata.file_type().is_symlink() #[cfg(windows)]
{ {
f.metadata().and_then(|metadata| {
if metadata.is_dir() {
std::fs::remove_dir(&f)
} else {
std::fs::remove_file(&f)
}
})
}
#[cfg(not(windows))]
std::fs::remove_file(&f)
} else if metadata.is_file() || is_socket || is_fifo {
std::fs::remove_file(&f) std::fs::remove_file(&f)
} else { } else {
std::fs::remove_dir_all(&f) std::fs::remove_dir_all(&f)

View File

@ -375,6 +375,19 @@ fn removes_symlink() {
}); });
} }
#[test]
fn removes_symlink_pointing_to_directory() {
Playground::setup("rm_symlink_to_directory", |dirs, sandbox| {
sandbox.mkdir("test").symlink("test", "test_link");
nu!(cwd: sandbox.cwd(), "rm test_link");
assert!(!dirs.test().join("test_link").exists());
// The pointed directory should not be deleted.
assert!(dirs.test().join("test").exists());
});
}
#[test] #[test]
fn removes_file_after_cd() { fn removes_file_after_cd() {
Playground::setup("rm_after_cd", |dirs, sandbox| { Playground::setup("rm_after_cd", |dirs, sandbox| {