Set an environment variable for tests to find executables. #7697

ehuss · 2019-12-11T17:06:38Z

This adds the environment variable CARGO_BIN_EXE_<name> so that integration tests can find binaries to execute, instead of doing things like inspecting env::current_exe().

The use of uppercase is primarily motivated by Windows whose Rust implementation behaves in a strange way. It always ascii-upper-cases keys to implement case-insensitive matching (which loses the original case). Seems less likely to result in confusion?

Closes #5758.

rust-highfive · 2019-12-11T17:06:42Z

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

Eh2406 · 2019-12-11T17:21:02Z

I like it. But I think the team should discuss before we merge.

alexcrichton · 2019-12-11T19:26:01Z

@rfcbot fcp merge

Looks like a great feature to me!

rfcbot · 2019-12-11T20:03:24Z

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

~~should-use-same-case-as-Cargo.toml~~ resolved by Set an environment variable for tests to find executables. #7697 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

ehuss · 2019-12-11T20:18:18Z

TIL that HFS normalizes unicode differently from APFS.

joshtriplett

This commit seems to be doing many different things. It's adding documentation for other methods, changing some existing documentation files (including things like changing dashes to Unicode dashes), and adding the new variable/test/documentation. Could you please split the commit, and make one logical change per commit? (I would also request that the other commits not be in one omnibus pull request with several unrelated changes.)

Apart from that: I'm quite uncomfortable with the "convert to uppercase" approach. I appreciate that you've taken the care to do so in what seems like a correct manner for Unicode, but it still doesn't seem like a good idea to me. I would prefer to always match the exact name used in the Cargo.toml file (or the exact name that Cargo infers the binary name from if not explicitly specified), without performing any kind of conversion.

That aside, I do like the concept, and I think it will make tests simpler and more robust.

joshtriplett · 2020-01-14T00:15:39Z

@rfcbot concern should-use-same-case-as-Cargo.toml

ehuss · 2020-01-15T01:09:59Z

I'll split the unrelated things out.

As for the uppercase thing, I think it will be extremely difficult to support Windows without it. For example, the Rust standard library loses case (here). In general, supporting case-sensitive environment variables can be difficult (and also unusual in my experience). Are there specific concerns about what could happen? I'm well aware that that there can be difficult edge cases, but I think for the vast majority of cases it should be fine.

joshtriplett · 2020-01-15T07:46:48Z

I appreciate that Windows is not case-sensitive. Unix and Linux, however, are. If there's no better solution available for Windows than uppercasing the variable name, then that should only occur on Windows.

Files differing only by case do indeed appear in real projects and real use cases. Furthermore, on a case-sensitive platform, changing the case of a binary name refers to a different binary, and users of such platforms (myself included) will find that behavior surprising, unexpected, and unwelcome.

joshtriplett · 2020-01-15T07:51:44Z

Also, different environments uppercase Unicode differently, and some Microsoft tools and libraries will handle the example you have (ß) differently.

alexcrichton · 2020-01-15T16:34:38Z

I disagree that we should be doing something different only on Windows here, that's generally not what Cargo does and can cause compatibility issues when your project works on Linux and doesn't work on Windows. In general I believe we try to have uniform behavior across all platforms, which in cases like this involves taking the intersection of possible behaivors and making that the API that we expose.

I don't think that we really need to worry all that much about unicode here. While it's of course a minefield if you're constantly knee-deep in unicode we're talking about Cargo projects and binary names, which 99.9% of the time don't even contain anything beyond ASCII. I don't think that an extremely small slice of projects which may have odd upper-casings in unicode should cause us to force everyone to deal with Windows-specific behavior.

joshtriplett · 2020-01-15T21:45:28Z

@alexcrichton I don't want everyone to deal with Windows-specific behavior either. Uppercasing on all platforms makes everyone have to deal with case-insensitivity, which is Windows-specific behavior. On a Linux platform, I expect names to be case sensitive, and squelching case is a bug.

Programs distinguished only by case isn't a theoretical issue, it's quite common. As one example, many Linux systems have both /usr/bin/HEAD and /usr/bin/head, which are two completely different programs. (The former sends an HTTP HEAD request, the latter outputs the first lines of a file.)

What I'm suggesting is this: use the actual case on all platforms, have test programs reference environment variables using the actual case on all platforms, and we can have an internal workaround (that we don't have to expose to the user) if it turns out that that doesn't work on Windows. It sounds like Rust on Windows already uppercases environment variable names, so wouldn't that Just Work? Cargo would set the environment variable by name (which gets internally uppercased on Windows), and rustc would read the environment variable by name (which gets internally uppercased on Windows), so they'd match and write/read the same environment variable.

ehuss · 2020-01-15T21:54:23Z

I'll need to do some tests on Windows to see how it treats unicode. I know the Rust std lib only handles ascii, but I'm not sure what Windows itself does (I can't find any info online). I'll investigate and see what issues arise. In theory the rustc side should also be case-insensitive on windows so it shouldn't matter what case it is, but I suspect it is broken for unicode.

Are you aware of examples of tools with environment variables that use case-sensitive mixed-case variable names? It seems very unusual to me.

Programs distinguished only by case isn't a theoretical issue,

I think that's a bit of a stretch, this only matters when the programs are in the same Cargo workspace.

joshtriplett · 2020-01-15T22:07:27Z

@ehuss

I appreciate you looking into this further. Thank you.

Are you aware of examples of tools with environment variables that use case-sensitive mixed-case variable names? It seems very unusual to me.

Yes, typically for this exact reason: a fixed prefix followed by arbitrary user-defined data. Off the top of my head, I know autotools does this quite often.

I think that's a bit of a stretch, this only matters when the programs are in the same Cargo workspace.

Consider a busybox-like project that implements many programs as part of the same project.
Even if it doesn't cause breakage in a given project, it still leads to confusion and not a small amount of dismay to run into a program on Linux that changes the case of filenames, as people who have run into such issues before know the potential for such breakage.

alexcrichton · 2020-01-16T17:14:37Z

If env::var("CARGO_THING_the_binary") works on all platforms, that's fine by me. I don't mind whether it's uppercase or lowercase, but "the naive thing" should work on all platforms. I'm not sure whether that works on Windows or not, myself, though.

joshtriplett · 2020-01-17T09:44:47Z

@alexcrichton

If env::var("CARGO_THING_the_binary") works on all platforms, that's fine by me.

It should, as far as I can tell.

ehuss · 2020-02-06T17:56:41Z

I have updated so that it doesn't modify the binary name in the environment variable. I think it should be fine, though it feels a bit unnatural to me. @joshtriplett does that resolve your concern?

joshtriplett · 2020-02-07T22:53:27Z

Reviewed. This looks great to me, thank you!

joshtriplett · 2020-02-07T22:54:37Z

@rfcbot resolved should-use-same-case-as-Cargo.toml

@rfcbot reviewed

rfcbot · 2020-02-07T23:52:40Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2020-02-17T23:57:20Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

alexcrichton · 2020-02-18T15:24:38Z

@bors: r+

bors · 2020-02-18T15:24:40Z

📌 Commit 4cf531f has been approved by alexcrichton

bors · 2020-02-18T15:24:47Z

⌛ Testing commit 4cf531f with merge e029740...

Set an environment variable for tests to find executables. This adds the environment variable `CARGO_BIN_EXE_<name>` so that integration tests can find binaries to execute, instead of doing things like inspecting `env::current_exe()`. The use of uppercase is primarily motivated by Windows whose Rust implementation behaves in a strange way. It always ascii-upper-cases keys to implement case-insensitive matching (which loses the original case). Seems less likely to result in confusion? Closes #5758.

bors · 2020-02-18T15:40:57Z

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing e029740 to master...

Update cargo 9 commits in 3c53211c3d7fee4f430f170115af5baad17a3da9..e02974078a692d7484f510eaec0e88d1b6cc0203 2020-02-07 15:35:03 +0000 to 2020-02-18 15:24:43 +0000 - Set an environment variable for tests to find executables. (rust-lang/cargo#7697) - Rework internal errors. (rust-lang/cargo#7896) - Improvements to StringList config handling. (rust-lang/cargo#7891) - Add new/old rustflags to fingerprint log. (rust-lang/cargo#7890) - Fix inaccurate doc comment on `env_args`. (rust-lang/cargo#7889) - Add some extra fingerprint debug information. (rust-lang/cargo#7888) - Link the licenses into crates/cargo-platform (rust-lang/cargo#7886) - Modify test to make `rustc` PR mergeable (rust-lang/cargo#7883) - Keep environment variables in a BTreeMap to preserve sort order (rust-lang/cargo#7877)

Pkgsrc changes: * Bump rust bootstrap version to 1.42.0, except for Darwin/i686 where the bootstrap is not (yet?) available. Upstream changes: Version 1.43.0 (2020-04-23) ========================== Language -------- - [Fixed using binary operations with `&{number}` (e.g. `&1.0`) not having the type inferred correctly.][68129] - [Attributes such as `#[cfg()]` can now be used on `if` expressions.][69201] **Syntax only changes** - [Allow `type Foo: Ord` syntactically.][69361] - [Fuse associated and extern items up to defaultness.][69194] - [Syntactically allow `self` in all `fn` contexts.][68764] - [Merge `fn` syntax + cleanup item parsing.][68728] - [`item` macro fragments can be interpolated into `trait`s, `impl`s, and `extern` blocks.][69366] For example, you may now write: ```rust macro_rules! mac_trait { ($i:item) => { trait T { $i } } } mac_trait! { fn foo() {} } ``` These are still rejected *semantically*, so you will likely receive an error but these changes can be seen and parsed by macros and conditional compilation. Compiler -------- - [You can now pass multiple lint flags to rustc to override the previous flags.][67885] For example; `rustc -D unused -A unused-variables` denies everything in the `unused` lint group except `unused-variables` which is explicitly allowed. However, passing `rustc -A unused-variables -D unused` denies everything in the `unused` lint group **including** `unused-variables` since the allow flag is specified before the deny flag (and therefore overridden). - [rustc will now prefer your system MinGW libraries over its bundled libraries if they are available on `windows-gnu`.][67429] - [rustc now buffers errors/warnings printed in JSON.][69227] Libraries --------- - [`Arc<[T; N]>`, `Box<[T; N]>`, and `Rc<[T; N]>`, now implement `TryFrom<Arc<[T]>>`,`TryFrom<Box<[T]>>`, and `TryFrom<Rc<[T]>>` respectively.][69538] **Note** These conversions are only available when `N` is `0..=32`. - [You can now use associated constants on floats and integers directly, rather than having to import the module.][68952] e.g. You can now write `u32::MAX` or `f32::NAN` with no imports. - [`u8::is_ascii` is now `const`.][68984] - [`String` now implements `AsMut<str>`.][68742] - [Added the `primitive` module to `std` and `core`.][67637] This module reexports Rust's primitive types. This is mainly useful in macros where you want avoid these types being shadowed. - [Relaxed some of the trait bounds on `HashMap` and `HashSet`.][67642] - [`string::FromUtf8Error` now implements `Clone + Eq`.][68738] Stabilized APIs --------------- - [`Once::is_completed`] - [`f32::LOG10_2`] - [`f32::LOG2_10`] - [`f64::LOG10_2`] - [`f64::LOG2_10`] - [`iter::once_with`] Cargo ----- - [You can now set config `[profile]`s in your `.cargo/config`, or through your environment.][cargo/7823] - [Cargo will now set `CARGO_BIN_EXE_<name>` pointing to a binary's executable path when running integration tests or benchmarks.][cargo/7697] `<name>` is the name of your binary as-is e.g. If you wanted the executable path for a binary named `my-program`you would use `env!("CARGO_BIN_EXE_my-program")`. Misc ---- - [Certain checks in the `const_err` lint were deemed unrelated to const evaluation][69185], and have been moved to the `unconditional_panic` and `arithmetic_overflow` lints. Compatibility Notes ------------------- - [Having trailing syntax in the `assert!` macro is now a hard error.][69548] This has been a warning since 1.36.0. - [Fixed `Self` not having the correctly inferred type.][69340] This incorrectly led to some instances being accepted, and now correctly emits a hard error. [69340]: rust-lang/rust#69340 Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of `rustc` and related tools. - [All components are now built with `opt-level=3` instead of `2`.][67878] - [Improved how rustc generates drop code.][67332] - [Improved performance from `#[inline]`-ing certain hot functions.][69256] - [traits: preallocate 2 Vecs of known initial size][69022] - [Avoid exponential behaviour when relating types][68772] - [Skip `Drop` terminators for enum variants without drop glue][68943] - [Improve performance of coherence checks][68966] - [Deduplicate types in the generator witness][68672] - [Invert control in struct_lint_level.][68725] [67332]: rust-lang/rust#67332 [67429]: rust-lang/rust#67429 [67637]: rust-lang/rust#67637 [67642]: rust-lang/rust#67642 [67878]: rust-lang/rust#67878 [67885]: rust-lang/rust#67885 [68129]: rust-lang/rust#68129 [68672]: rust-lang/rust#68672 [68725]: rust-lang/rust#68725 [68728]: rust-lang/rust#68728 [68738]: rust-lang/rust#68738 [68742]: rust-lang/rust#68742 [68764]: rust-lang/rust#68764 [68772]: rust-lang/rust#68772 [68943]: rust-lang/rust#68943 [68952]: rust-lang/rust#68952 [68966]: rust-lang/rust#68966 [68984]: rust-lang/rust#68984 [69022]: rust-lang/rust#69022 [69185]: rust-lang/rust#69185 [69194]: rust-lang/rust#69194 [69201]: rust-lang/rust#69201 [69227]: rust-lang/rust#69227 [69548]: rust-lang/rust#69548 [69256]: rust-lang/rust#69256 [69361]: rust-lang/rust#69361 [69366]: rust-lang/rust#69366 [69538]: rust-lang/rust#69538 [cargo/7823]: rust-lang/cargo#7823 [cargo/7697]: rust-lang/cargo#7697 [`Once::is_completed`]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed [`f32::LOG10_2`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG10_2.html [`f32::LOG2_10`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG2_10.html [`f64::LOG10_2`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG10_2.html [`f64::LOG2_10`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG2_10.html [`iter::once_with`]: https://doc.rust-lang.org/std/iter/fn.once_with.html

rust-highfive assigned Eh2406 Dec 11, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2019

alexcrichton added the T-cargo Team: Cargo label Dec 11, 2019

rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Dec 11, 2019

ehuss force-pushed the bin-test-env branch from 9d0a103 to 9c56a4b Compare December 11, 2019 20:15

joshtriplett requested changes Jan 14, 2020

View reviewed changes

ehuss force-pushed the bin-test-env branch from 9c56a4b to 07dce15 Compare January 15, 2020 18:20

ehuss added 2 commits February 6, 2020 08:15

Set an environment variable for tests to find executables.

3489428

Switch to case-sensitive environment variable.

499f917

ehuss force-pushed the bin-test-env branch from 07dce15 to 499f917 Compare February 6, 2020 16:33

Add more docs on CARGO_BIN_EXE_.

4cf531f

rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Feb 7, 2020

rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Feb 7, 2020

rfcbot added finished-final-comment-period FCP complete and removed final-comment-period FCP — a period for last comments before action is taken labels Feb 17, 2020

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 Feb 18, 2020

bors merged commit 4cf531f into rust-lang:master Feb 18, 2020

ehuss mentioned this pull request Feb 18, 2020

Update cargo rust-lang/rust#69265

Merged

ehuss mentioned this pull request Mar 24, 2020

Update RELEASES.md for 1.43.0 rust-lang/rust#70354

Merged

matklad mentioned this pull request Apr 15, 2020

Add proc-macro related config and tests rust-lang/rust-analyzer#3958

Merged

epage mentioned this pull request Apr 21, 2020

Take advantage of new binary environment variable assert-rs/assert_cmd#101

Closed

kolloch mentioned this pull request Apr 24, 2020

New ENV var pointing to CLI nix-community/crate2nix#131

Open

edwin0cheng mentioned this pull request May 8, 2020

ra_project_model::json_project::Crate.key_value_cfg doesn't match ra_config::CfgOptions.key_values rust-lang/rust-analyzer#4310

Closed

Minoru mentioned this pull request May 9, 2020

Port FsLock to Rust newsboat/newsboat#914

Closed

This was referenced Sep 20, 2020

Moduletest: Test only the executable for the right build anordal/shellharden#35

Closed

Make it possible to run binaries produced by cargo directly. #3670

Open

orhun mentioned this pull request Oct 12, 2020

Use CARGO_BIN_EXE instead of hardcoded path for tests Skallwar/suckit#98

Merged

taiki-e mentioned this pull request Jan 14, 2021

chore: use CARGO_BIN_EXE instead of inspecting env::current_exe() tokio-rs/tokio#3427

Merged

ehuss added this to the 1.43.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set an environment variable for tests to find executables. #7697

Set an environment variable for tests to find executables. #7697

ehuss commented Dec 11, 2019

rust-highfive commented Dec 11, 2019

Eh2406 commented Dec 11, 2019

alexcrichton commented Dec 11, 2019

rfcbot commented Dec 11, 2019 •

edited

Loading

ehuss commented Dec 11, 2019

joshtriplett left a comment •

edited

Loading

joshtriplett commented Jan 14, 2020

ehuss commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

alexcrichton commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

ehuss commented Jan 15, 2020

joshtriplett commented Jan 15, 2020 •

edited

Loading

alexcrichton commented Jan 16, 2020

joshtriplett commented Jan 17, 2020

ehuss commented Feb 6, 2020

joshtriplett commented Feb 7, 2020

joshtriplett commented Feb 7, 2020

rfcbot commented Feb 7, 2020

rfcbot commented Feb 17, 2020

alexcrichton commented Feb 18, 2020

bors commented Feb 18, 2020

bors commented Feb 18, 2020

bors commented Feb 18, 2020

Set an environment variable for tests to find executables. #7697

Set an environment variable for tests to find executables. #7697

Conversation

ehuss commented Dec 11, 2019

rust-highfive commented Dec 11, 2019

Eh2406 commented Dec 11, 2019

alexcrichton commented Dec 11, 2019

rfcbot commented Dec 11, 2019 • edited Loading

ehuss commented Dec 11, 2019

joshtriplett left a comment • edited Loading

Choose a reason for hiding this comment

joshtriplett commented Jan 14, 2020

ehuss commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

alexcrichton commented Jan 15, 2020

joshtriplett commented Jan 15, 2020

ehuss commented Jan 15, 2020

joshtriplett commented Jan 15, 2020 • edited Loading

alexcrichton commented Jan 16, 2020

joshtriplett commented Jan 17, 2020

ehuss commented Feb 6, 2020

joshtriplett commented Feb 7, 2020

joshtriplett commented Feb 7, 2020

rfcbot commented Feb 7, 2020

rfcbot commented Feb 17, 2020

alexcrichton commented Feb 18, 2020

bors commented Feb 18, 2020

bors commented Feb 18, 2020

bors commented Feb 18, 2020

rfcbot commented Dec 11, 2019 •

edited

Loading

joshtriplett left a comment •

edited

Loading

joshtriplett commented Jan 15, 2020 •

edited

Loading