about summary refs log tree commit diff
path: root/src/tools/compiletest
diff options
context:
space:
mode:
authorTrevor Gross <t.gross35@gmail.com>2024-12-23 02:07:31 -0500
committerGitHub <noreply@github.com>2024-12-23 02:07:31 -0500
commitfde85a8e5f029ae8b064faec6af5ca8a9501006d (patch)
tree492be018a414e511b3825d234cbf04eff75dcf85 /src/tools/compiletest
parent3acf9c93a46f7d2e602ec44a7c296f3539ac55be (diff)
parent4d3bf1f5eed1247d773096e35e0cd471a0692e70 (diff)
downloadrust-fde85a8e5f029ae8b064faec6af5ca8a9501006d.tar.gz
rust-fde85a8e5f029ae8b064faec6af5ca8a9501006d.zip
Rollup merge of #134659 - jieyouxu:recursive-remove, r=ChrisDenton
test-infra: improve compiletest and run-make-support symlink handling

I was trying to implement #134656 to port `tests/run-make/incr-add-rust-src-component.rs`, but found some blockers related to symlink handling, so in this PR I tried to resolve them by improving symlink handling in compiletest and run-make-support (particularly for native windows msvc environment).

Key changes:

- I needed to copy symlinks (duplicate a symlink pointing to the same file), so I pulled out the copy symlink logic and re-exposed it as `run_make_support::rfs::copy_symlink`. This helper correctly accounts for the Windows symlink-to-file vs symlink-to-dir distinction (hereafter "Windows symlinks") when copying symlinks.
- `recursive_remove`:
    - I needed a way to remove symlinks themselves (no symlink traversal). `std::fs::remove_dir_all` handles them, but only if the root path is a directory. So I wrapped `std::fs::remove_dir_all` to also handle when the root path is a non-directory entity (e.g. file or symlink). Again, this properly accounts for Windows symlinks.
    - I wanted to use this for both compiletest and run-make-support, so I put the implementation and accompanying tests in `build_helper`.
    - In this sense, it's a reland of #129302 with proper test coverage.
    - It's a thin wrapper around `std::fs::remove_dir_all` (`remove_dir_all` correctly handles read-only entries on Windows). The helper has additional permission-setting logic for when the root path is a non-dir entry on Windows to handle read-only non-dir entry.

Fixes #126334.
Diffstat (limited to 'src/tools/compiletest')
-rw-r--r--src/tools/compiletest/Cargo.toml2
-rw-r--r--src/tools/compiletest/src/runtest.rs23
-rw-r--r--src/tools/compiletest/src/runtest/run_make.rs12
3 files changed, 7 insertions, 30 deletions
diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml
index b784bdb7139..16cc1d2a565 100644
--- a/src/tools/compiletest/Cargo.toml
+++ b/src/tools/compiletest/Cargo.toml
@@ -16,7 +16,7 @@ indexmap = "2.0.0"
 miropt-test-tools = { path = "../miropt-test-tools" }
 build_helper = { path = "../../build_helper" }
 tracing = "0.1"
-tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] }
+tracing-subscriber = { version = "0.3.3", default-features = false, features = ["ansi", "env-filter", "fmt", "parking_lot", "smallvec"] }
 regex = "1.0"
 semver = { version = "1.0.23", features = ["serde"] }
 serde = { version = "1.0", features = ["derive"] }
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index 6a4f0b96bb4..108fde1c899 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -2809,29 +2809,6 @@ impl<'test> TestCx<'test> {
             println!("init_incremental_test: incremental_dir={}", incremental_dir.display());
         }
     }
-
-    fn aggressive_rm_rf(&self, path: &Path) -> io::Result<()> {
-        for e in path.read_dir()? {
-            let entry = e?;
-            let path = entry.path();
-            if entry.file_type()?.is_dir() {
-                self.aggressive_rm_rf(&path)?;
-            } else {
-                // Remove readonly files as well on windows (by default we can't)
-                fs::remove_file(&path).or_else(|e| {
-                    if cfg!(windows) && e.kind() == io::ErrorKind::PermissionDenied {
-                        let mut meta = entry.metadata()?.permissions();
-                        meta.set_readonly(false);
-                        fs::set_permissions(&path, meta)?;
-                        fs::remove_file(&path)
-                    } else {
-                        Err(e)
-                    }
-                })?;
-            }
-        }
-        fs::remove_dir(path)
-    }
 }
 
 struct ProcArgs {
diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs
index 85ade5b727a..ee7aed2a39c 100644
--- a/src/tools/compiletest/src/runtest/run_make.rs
+++ b/src/tools/compiletest/src/runtest/run_make.rs
@@ -2,6 +2,8 @@ use std::path::Path;
 use std::process::{Command, Output, Stdio};
 use std::{env, fs};
 
+use build_helper::fs::{ignore_not_found, recursive_remove};
+
 use super::{ProcRes, TestCx, disable_error_reporting};
 use crate::util::{copy_dir_all, dylib_env_var};
 
@@ -27,9 +29,8 @@ impl TestCx<'_> {
         // are hopefully going away, it seems safer to leave this perilous code
         // as-is until it can all be deleted.
         let tmpdir = cwd.join(self.output_base_name());
-        if tmpdir.exists() {
-            self.aggressive_rm_rf(&tmpdir).unwrap();
-        }
+        ignore_not_found(|| recursive_remove(&tmpdir)).unwrap();
+
         fs::create_dir_all(&tmpdir).unwrap();
 
         let host = &self.config.host;
@@ -218,9 +219,8 @@ impl TestCx<'_> {
         //
         // This setup intentionally diverges from legacy Makefile run-make tests.
         let base_dir = self.output_base_dir();
-        if base_dir.exists() {
-            self.aggressive_rm_rf(&base_dir).unwrap();
-        }
+        ignore_not_found(|| recursive_remove(&base_dir)).unwrap();
+
         let rmake_out_dir = base_dir.join("rmake_out");
         fs::create_dir_all(&rmake_out_dir).unwrap();