Auto merge of rust-lang#139507 - Zalathar:trim-env-name, r=<try>

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: bors

src/doc/rustc-dev-guide/src/tests/directives.md

@@ -158,6 +158,7 @@ settings:
 
 - `needs-asm-support` — ignores if it is running on a target that doesn't have
   stable support for `asm!`
+- `needs-env-vars` - ignores if the target doesn't support environment variables
 - `needs-profiler-runtime` — ignores the test if the profiler runtime was not
   enabled for the target
   (`build.profiler = true` in rustc's `bootstrap.toml`)

src/tools/compiletest/src/common.rs

@@ -520,6 +520,16 @@ impl Config {
             || self.target_cfg().os == "emscripten";
         !unsupported_target
     }
+
+    pub fn has_env_vars(&self) -> bool {
+        // As with some of the other target properties, we don't have a principled
+        // way to know whether the target supports environment variables, so we
+        // just hard-code a reasonable approximation.
+
+        let target = self.target_cfg();
+        let no_env = matches!(target.arch.as_str(), "wasm32" | "wasm64") && target.os == "unknown";
+        !no_env
+    }
 }
 
 /// Known widths of `target_has_atomic`.

src/tools/compiletest/src/directive-list.rs

@@ -135,6 +135,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "needs-deterministic-layouts",
     "needs-dlltool",
     "needs-dynamic-linking",
+    "needs-env-vars",
     "needs-enzyme",
     "needs-force-clang-based-tests",
     "needs-git-hash",

src/tools/compiletest/src/header.rs

@@ -979,16 +979,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/header/needs.rs

@@ -174,6 +174,11 @@ pub(super) fn handle_needs(
             condition: config.with_std_debug_assertions,
             ignore_reason: "ignored if std wasn't built with debug assertions",
         },
+        Need {
+            name: "needs-env-vars",
+            condition: config.has_env_vars(),
+            ignore_reason: "ignored on targets without environment variables",
+        },
     ];
 
     let (name, rest) = match ln.split_once([':', ' ']) {

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

@@ -0,0 +1,16 @@
+//@ edition: 2024
+//@ run-pass
+//@ needs-env-vars
+//@ rustc-env: MY_RUSTC_ENV = my-rustc-value
+//@ exec-env: MY_EXEC_ENV = my-exec-value
+
+// 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.
+
+fn main() {
+    assert_eq!(option_env!("MY_RUSTC_ENV"), Some(" my-rustc-value"));
+    assert_eq!(std::env::var("MY_EXEC_ENV").as_deref(), Ok(" my-exec-value"));
+}