Remove whitespaces on rust-env directives from some test codes #133148
Conversation
Signed-off-by: Shunpoco <[email protected]>
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @fmease (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
Shunpoco
changed the title
|
The Miri subtree was changed cc @rust-lang/miri |
|
r? @jieyouxu |
|
Miri uses a different test runner that does not have this problem (I've linked its parsing code for this directive below). Can you revert the changes in "rustc-env" => (this, args, _span){
for env in args.split_whitespace() { |
|
If this is important for a test's correctness, the runner should complain about the whitespace. Otherwise, we'll soon get new tests with such whitespace. Fixing this just on the tests we currently have is not sustainable. Or, alternatively, we could just adjust the runner to work fine despite whitespace? |
I'm inclined to adjust the @Shunpoco thanks for the PR, but I think I'm going to address the underlying directive parsing problem instead. |
|
Sure, I will continue to track the issue for my curiosity 😄 |
|
(Also to make it more complicated, |
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
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
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
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
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 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
This PR is a part of #132990.
{unset-,}{rustc,exec}-env directive shouldn't have whitespace.
NOTE: It looks that those whitespaces don't affect tests because I think those are parsed then added as temporary env vars on command line. Those are parsed by Rust properly.
For example,
SOME_ENV=1 ⌴RUST_TRACEBACK=0 rustdoc src/lib.rsis equivalent withSOME_ENV=1 RUST_TRACEBACK=0 rustdoc src/lib.rs. (Of course for code's consistency, we should use the directives without any whitespaces.)