main_binary and cargo_binary fail with Environment::empty

[epage]

cargo can't be found because it is no longer in the path.

[epage]

I think this is a reasonable limitation, I just wanted to ensure it is noted somewhere.

So ways I see forward for this

• Rejecting it with label wontfix
• Noting the limitation somewhere (an example workaround?) and closing this out with that (maybe still with a label wontfix).

[killercup]

Good point! Let's add a test for these cases! I think we are only testing with shell internal commands (printenv) currently. That's obviously not good enough. (Okay if these tests are skipped with a comment as they are failing for now.) To solve this: Can we just run `which cargo` in the constructor and run the command with the full path to cargo? Ed Page <[email protected]> schrieb am Fr. 13. Okt. 2017 um 02:53:

…

I think this is a reasonable limitation, I just wanted to ensure it is noted somewhere. So ways I see forward for this - Rejecting it with label wontfix - Noting the limitation somewhere (an example workaround?) and closing this out with that (maybe still with a label wontfix). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABOXw8hRk0mv2sZA92K1Ju7NUhped47ks5srrR9gaJpZM4P30-W> .

[Freyskeyd]

Yep if we wipe out the env we don't have access to the PATH.

Do we have a simple workaround? Saving the CARGO_* variables?

[epage]

To solve this: Can we just run which cargo in the constructor and run the command with the full path to cargo?

We'd have to keep in mind support for both Windows and Linux

Do we have a simple workaround? Saving the CARGO_* variables?

I was thinking of contributing to Environment a inherit_var(key) so clients could work around this more easily.

The main question is what the API should look like. As Environment grows different var manipulation functions (inherit, append, etc), should they be var_append, inherit_var, etc or var(key).inherit(), var(key).append(text), etc?

[sevagh]

Just to confirm that I expected the with_env(&vec[("MY_ENV_VAR", "MY_VAL")] to Just Work(TM) and it didn't - I had to use Environment::inherit().insert("MY_ENV_VAR", "MY_VAL").

Previously, I used to use https://doc.rust-lang.org/1.1.0/std/process/struct.Command.html#method.env - where:

let mut cmd = std::process::Command();
cmd.env("MY_ENV_VAR", "MY_VAL");

The usage there is just to append to the inherited environment variables, not to totally clear them.

I find the fact that with_env erases all the inherited env vars to be jarring. Any opinions?

[epage]

To not detract from the discussion in this thread, I opened #58

[luser]

I don't know if this is new, but the cargo docs say that cargo sets the CARGO env var to the path to the cargo binary in use, so you ought to just be able to use env!("CARGO").

[epage]

Thanks for looking into this!

CARGO - Path to the cargo binary performing the build.

Sounds like it is literally the cargo binary and not the binary cargo is producing but some of the other variables might be helpful.

EDIT: Oh. right, that'd still allow us to delegate to cargo for running the binary

[luser]

FWIW, I used $CARGO (along with assert-cli) in a test I wrote today:
https://github.com/mozilla/sccache/blob/2d9bdc1e8fb1fa5a531da2930670bbfcb66f78a0/tests/sccache_cargo.rs#L48

[killercup]

Good idea using env!("CARGO")!

I think the CI problems blocking #54 (which should also fix this) are resolves and we can land this (and #52) after a rebase.

[epage]

I gave running CARGO with an empty env a try. It couldn't find rustc

[luser]

Yeah, cargo first looks for rustc in the RUSTC env var, and then looks for rustc in PATH`:
https://github.com/rust-lang/cargo/blob/859c2305b5d7ff820d7f7af158bbca999cbd8bf3/src/cargo/util/config.rs#L595

If you unset PATH it won't find it.

[epage]

Here is an alternative approach to finding the test binary
https://github.com/emk/subtitles-rs/blob/master/cli_test_dir/src/lib.rs#L173

I suspect it won't work with skeptic which runs examples in a tmp directory
See budziq/rust-skeptic#69

[killercup]

Apropos skeptic: Do we still have problems with that? I recently rediscovered cargo build && rustdoc --test ./README.md -L ./target/debug/ -L ./target/debug/deps/ but totally forgot to tell you about it, @epage.

[epage]

That could be a real nice way to avoid the skeptic problems.

• PWD is a temp dir rather than the same as the tests
  • Issue opened
  • Reason why this was done was Rust Cookbook was leaving dummy files on the file system
  • Proposed fix is to have a feature flag
• Skeptic fails without cargo clean when upgrading rust
• Skeptic fails when mixing cargo check and cargo test
• Skeptic gets pushed on to dependents

The downside to running the README like that is it won't be in people's regular workflow. On the otherhand, the README should probably be light on examples anyways and the CI should be sufficient.

[epage]

Addressed in assert_cmd
https://github.com/assert-rs/assert_cmd
assert_cli is going to morph into a wider-focused tool, including things like assert_fs. See #41