Rollup merge of rust-lang#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 rust-lang#138603 (comment).

Fixes rust-lang#132990.
Supersedes rust-lang#133148.

---

try-job: test-various

Author: Zalathar

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

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 {

tests/ui/compiletest-self-test/trim-env-name.rs

@@ -0,0 +1,23 @@
+//@ edition: 2024
+//@ revisions: set unset
+//@ run-pass
+//@ ignore-cross-compile (assume that non-cross targets have working env vars)
+//@ rustc-env: MY_RUSTC_ENV =  my-rustc-value
+//@ exec-env:  MY_EXEC_ENV  =  my-exec-value
+//@[unset] unset-rustc-env:    MY_RUSTC_ENV
+//@[unset] unset-exec-env:     MY_EXEC_ENV
+
+// Check that compiletest trims whitespace from environment variable names
+// specified in `rustc-env` and `exec-env` directives, so that
+// `//@ exec-env: FOO=bar` sees the name as `FOO` and not ` FOO`.
+//
+// Values are currently not trimmed.
+//
+// Since this is a compiletest self-test, only run it on non-cross targets,
+// to avoid having to worry about weird targets that don't support env vars.
+
+fn main() {
+    let is_set = cfg!(set);
+    assert_eq!(option_env!("MY_RUSTC_ENV"), is_set.then_some("  my-rustc-value"));
+    assert_eq!(std::env::var("MY_EXEC_ENV").ok().as_deref(), is_set.then_some("  my-exec-value"));
+}