Skip to content

Commit

Permalink
Auto merge of #11333 - ehuss:remove-remove_dir_all, r=epage
Browse files Browse the repository at this point in the history
Remove remove_dir_all

This removes the `remove_dir_all` dependency. It is no longer needed, as there has been significant improvements to std's implementation over the years (particularly recently). This improves the performance of running cargo's testsuite on my machine from 8m to 2m 30s (!!).

This was added in #7137 to deal with some issues with symbolic links. I believe those seem to be resolved now.

Note that std's implementation is still not bullet proof. In particular, I think there are still some cases where a file can be locked by another process which prevents it from being removed. There are some more notes about this at #7042 (comment) (and various other places linked there).

Closes #9585
  • Loading branch information
bors committed Nov 4, 2022
2 parents 68e9d6c + fa8375b commit 180ad91
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 2 deletions.
1 change: 0 additions & 1 deletion crates/cargo-test-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ git2 = "0.15.0"
glob = "0.3"
itertools = "0.10.0"
lazy_static = "1.0"
remove_dir_all = "0.5"
serde_json = "1.0"
tar = { version = "0.4.38", default-features = false }
termcolor = "1.1.2"
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-test-support/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl CargoPathExt for Path {
// actually performing the removal, but we don't care all that much
// for our tests.
if meta.is_dir() {
if let Err(e) = remove_dir_all::remove_dir_all(self) {
if let Err(e) = fs::remove_dir_all(self) {
panic!("failed to remove {:?}: {:?}", self, e)
}
} else if let Err(e) = fs::remove_file(self) {
Expand Down

0 comments on commit 180ad91

Please sign in to comment.