compiletest: Trim whitespace from environment variable names #139507

Zalathar · 2025-04-08T02:29:31Z

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 #138603 (comment).

Fixes #132990.
Supersedes #133148.

try-job: test-various

rustbot · 2025-04-08T02:29:37Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

jieyouxu

Yeah, thanks! I think leaving value untrimmed is good, unless we have some kind of escape mechanism.

jieyouxu · 2025-04-08T02:34:19Z

@bors r+ rollup

bors · 2025-04-08T02:34:22Z

📌 Commit 9bfb203 has been approved by jieyouxu

It is now in the queue for this repository.

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. r? jieyouxu

jieyouxu · 2025-04-08T02:40:19Z

This is something I wanted to do but never got around to following up in #133148 (comment).

…errors Rollup of 19 pull requests Successful merges: - rust-lang#138676 (Implement overflow for infinite implied lifetime bounds) - rust-lang#139024 (Make error message for missing fields with `..` and without `..` more consistent) - rust-lang#139098 (Tell LLVM about impossible niche tags) - rust-lang#139124 (compiler: report error when trait object type param reference self) - rust-lang#139321 (Update to new rinja version (askama)) - rust-lang#139346 (Don't construct preds w escaping bound vars in `diagnostic_hir_wf_check`) - rust-lang#139386 (make it possible to use stage0 libtest on compiletest) - rust-lang#139421 (Fix trait upcasting to dyn type with no principal when there are projections) - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`) - rust-lang#139476 (rm `RegionInferenceContext::var_infos`) - rust-lang#139481 (Add job summary links to post-merge report) - rust-lang#139485 (compiletest: Stricter parsing for diagnostic kinds) - rust-lang#139490 (Update some comment/docs related to "extern intrinsic" removal) - rust-lang#139491 (Update books) - rust-lang#139496 (Revert r-a changes of rust-lang#139455) - rust-lang#139500 (document panic behavior of Vec::resize and Vec::resize_with) - rust-lang#139501 (Fix stack overflow in exhaustiveness due to recursive HIR opaque hidden types) - rust-lang#139504 (add missing word in doc comment) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2025-04-08T08:59:42Z

Failed in rollup: #139508 (comment)

@bors r-

rustbot · 2025-04-08T10:23:19Z

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

Zalathar · 2025-04-08T10:24:13Z

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

bors · 2025-04-08T10:25:25Z

⌛ Trying commit 21851a0 with merge 6b40b1d...

bors · 2025-04-08T11:21:19Z

💔 Test failed - checks-actions

Zalathar · 2025-04-08T11:35:05Z

Because #132990 was linked, I updated this to also trim unset-rustc-env and unset-exec-env, which the original version of this PR did not touch.

And I realised that there's no need for a needs-env-vars directive in this case, because it's a self-test, so we can just as easily just do ignore-cross-compile and not worry about weird targets without environment variables.

@rustbot ready

jieyouxu

Thanks!

jieyouxu · 2025-04-08T11:44:57Z

@bors r+

bors · 2025-04-08T11:44:59Z

📌 Commit 34e9759 has been approved by jieyouxu

It is now in the queue for this repository.

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

Rollup of 18 pull requests Successful merges: - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped) - rust-lang#138167 (Small code improvement in rustdoc hidden stripper) - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated) - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern) - rust-lang#138904 (Test linking and running `no_std` binaries) - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer) - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal) - rust-lang#139560 (fix title of offset_of_enum feature) - rust-lang#139563 (emit a better error message for using the macro incorrectly) - rust-lang#139568 (Don't use empty trait names) - rust-lang#139580 (Temporarily leave the review rotation) - rust-lang#139589 (saethlin is back from vacation) - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests) - rust-lang#139599 (Tracking issue template: fine-grained information on style update status) - rust-lang#139600 (Update `compiler-builtins` to 0.1.153) - rust-lang#139606 (Update compiletest to Edition 2024) r? `@ghost` `@rustbot` modify labels: rollup

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

Rollup of 17 pull requests Successful merges: - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped) - rust-lang#138167 (Small code improvement in rustdoc hidden stripper) - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated) - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern) - rust-lang#138904 (Test linking and running `no_std` binaries) - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer) - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal) - rust-lang#139560 (fix title of offset_of_enum feature) - rust-lang#139563 (emit a better error message for using the macro incorrectly) - rust-lang#139568 (Don't use empty trait names) - rust-lang#139580 (Temporarily leave the review rotation) - rust-lang#139589 (saethlin is back from vacation) - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests) - rust-lang#139599 (Tracking issue template: fine-grained information on style update status) - rust-lang#139600 (Update `compiler-builtins` to 0.1.153) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 13 pull requests Successful merges: - rust-lang#138167 (Small code improvement in rustdoc hidden stripper) - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated) - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer) - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal) - rust-lang#139560 (fix title of offset_of_enum feature) - rust-lang#139563 (emit a better error message for using the macro incorrectly) - rust-lang#139568 (Don't use empty trait names) - rust-lang#139580 (Temporarily leave the review rotation) - rust-lang#139589 (saethlin is back from vacation) - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests) - rust-lang#139599 (Tracking issue template: fine-grained information on style update status) r? `@ghost` `@rustbot` modify labels: rollup

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

rustbot assigned jieyouxu Apr 8, 2025

jieyouxu approved these changes Apr 8, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2025

Zalathar mentioned this pull request Apr 8, 2025

Rollup of 19 pull requests #139508

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 8, 2025

Zalathar force-pushed the trim-env-name branch from 9bfb203 to 21851a0 Compare April 8, 2025 10:23

rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Apr 8, 2025

This comment has been minimized.

Sign in to view

compiletest: Trim whitespace from environment variable names

34e9759

Zalathar force-pushed the trim-env-name branch from 21851a0 to 34e9759 Compare April 8, 2025 11:32

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2025

jieyouxu approved these changes Apr 8, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2025

Zalathar mentioned this pull request Apr 10, 2025

Rollup of 18 pull requests #139612

Closed

Zalathar mentioned this pull request Apr 10, 2025

Rollup of 17 pull requests #139613

Closed

matthiaskrgr mentioned this pull request Apr 10, 2025

Rollup of 13 pull requests #139622

Merged

bors merged commit 2b0e47e into rust-lang:master Apr 10, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 10, 2025

Zalathar deleted the trim-env-name branch April 10, 2025 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiletest: Trim whitespace from environment variable names #139507

compiletest: Trim whitespace from environment variable names #139507

Zalathar commented Apr 8, 2025 •

edited

Loading

rustbot commented Apr 8, 2025

jieyouxu left a comment •

edited

Loading

jieyouxu commented Apr 8, 2025

bors commented Apr 8, 2025

jieyouxu commented Apr 8, 2025

Zalathar commented Apr 8, 2025

rustbot commented Apr 8, 2025

Zalathar commented Apr 8, 2025

bors commented Apr 8, 2025

This comment has been minimized.

bors commented Apr 8, 2025

Zalathar commented Apr 8, 2025

jieyouxu left a comment

jieyouxu commented Apr 8, 2025

bors commented Apr 8, 2025

compiletest: Trim whitespace from environment variable names #139507

compiletest: Trim whitespace from environment variable names #139507

Conversation

Zalathar commented Apr 8, 2025 • edited Loading

rustbot commented Apr 8, 2025

jieyouxu left a comment • edited Loading

Choose a reason for hiding this comment

jieyouxu commented Apr 8, 2025

bors commented Apr 8, 2025

jieyouxu commented Apr 8, 2025

Zalathar commented Apr 8, 2025

rustbot commented Apr 8, 2025

Zalathar commented Apr 8, 2025

bors commented Apr 8, 2025

This comment has been minimized.

bors commented Apr 8, 2025

Zalathar commented Apr 8, 2025

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Apr 8, 2025

bors commented Apr 8, 2025

Zalathar commented Apr 8, 2025 •

edited

Loading

jieyouxu left a comment •

edited

Loading