Rollup merge of #139507 - Zalathar:trim-env-name, r=jieyouxu

compiletest: Trim whitespace from environment variable names

When a test contains a directive like `//@ exec-env: FOO=bar`, compiletest currently includes that leading space in the name of the environment variable, so it is defined as ` FOO` instead of `FOO`.

This is an annoying footgun that is pretty much never intended, especially since most other directives *do* trim whitespace. So let's get rid of it by trimming the environment variable name.

Values remain untrimmed, since there could conceivably be a use-case for values with leading space, but perhaps we'll end up trimming values too in the future.

Recently observed in https://github.com/rust-lang/rust/pull/138603#issuecomment-2783709359.

Fixes #132990.
Supersedes #133148.

---

try-job: test-various

2 files changed, 13 insertions, 16 deletions

diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs
index 36a9e5df583..01a6ae00825 100644
--- a/src/tools/compiletest/src/header.rs
+++ b/src/tools/compiletest/src/header.rs
@@ -452,7 +452,7 @@ impl TestProps {
                         ln,
                         UNSET_EXEC_ENV,
                         &mut self.unset_exec_env,
-                        |r| r,
+                        |r| r.trim().to_owned(),
                     );
                     config.push_name_value_directive(
                         ln,
@@ -464,7 +464,7 @@ impl TestProps {
                         ln,
                         UNSET_RUSTC_ENV,
                         &mut self.unset_rustc_env,
-                        |r| r,
+                        |r| r.trim().to_owned(),
                     );
                     config.push_name_value_directive(
                         ln,
@@ -997,16 +997,13 @@ impl Config {
 
     fn parse_env(nv: String) -> (String, String) {
         // nv is either FOO or FOO=BAR
-        let mut strs: Vec<String> = nv.splitn(2, '=').map(str::to_owned).collect();
-
-        match strs.len() {
-            1 => (strs.pop().unwrap(), String::new()),
-            2 => {
-                let end = strs.pop().unwrap();
-                (strs.pop().unwrap(), end)
-            }
-            n => panic!("Expected 1 or 2 strings, not {}", n),
-        }
+        // FIXME(Zalathar): The form without `=` seems to be unused; should
+        // we drop support for it?
+        let (name, value) = nv.split_once('=').unwrap_or((&nv, ""));
+        // Trim whitespace from the name, so that `//@ exec-env: FOO=BAR`
+        // sees the name as `FOO` and not ` FOO`.
+        let name = name.trim();
+        (name.to_owned(), value.to_owned())
     }
 
     fn parse_pp_exact(&self, line: &str, testfile: &Path) -> Option<PathBuf> {
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index 2805e6b3d62..a3aae39be81 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -968,16 +968,16 @@ impl<'test> TestCx<'test> {
         delete_after_success: bool,
     ) -> ProcRes {
         let prepare_env = |cmd: &mut Command| {
-            for key in &self.props.unset_exec_env {
-                cmd.env_remove(key);
-            }
-
             for (key, val) in &self.props.exec_env {
                 cmd.env(key, val);
             }
             for (key, val) in env_extra {
                 cmd.env(key, val);
             }
+
+            for key in &self.props.unset_exec_env {
+                cmd.env_remove(key);
+            }
         };
 
         let proc_res = match &*self.config.target {