Display environment variables when showing the command being run

[mikeyhew]

Even with #5666, when commands are shown in verbose output, there are still some cases where they can’t be run in the terminal. This is because the code is expecting certain environment variables that are provided by cargo. This PR updates the Display impl for ProcessBuilder to show the environment variables in front of the command (on unix), so that the command can be run. Example output:

Note that for workspaces, you will still have to run the command from the workspace root.

I deliberately didn’t update any of the tests yet, because I want to see if this is desired. I also want to see if this can be done on windows, so hopefully someone with experience with that platform can comment.

[rust-highfive]

r? @alexcrichton

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

[alexcrichton]

Thanks for the PR!

It's true yeah that sometimes it's nice to have these baked in so they can be copy-pasted, but I think I'd prefer to solve a few issues first before merging if we merge:

• This should have both a Unix and a Windows implementation
• I think we may want to gate this on -vv instead of just -v

And I think that should be good enough?

[alexcrichton]

ping @mikeyhew, have you had a chance to take another look at this?

[mikeyhew]

Hey @alexcrichton that all sounds good, I'm just not familiar with windows, so not really sure what to do there. I was reading the answer to this question, and it looks like the cleanest way to pass environment variables to a subprocess is with a command (env) that you have to install.

One thing we could do is just output the environment variables on a separate line, so people can add them manually.

Any thoughts?

[alexcrichton]

Hm so that answer does have at least a solution:

cmd /V /C "set EDITOR=vim&& echo !EDITOR!"

albeit not the greatest script :(

Another possible idea is to split this output onto multiple lines, but that has the unfortunate side effect of maybe getting mixed up with compiler diagnostic output (which Cargo doesn't synchronize with currently). I'm unfortunately also not sure what the best solution here would be...

[mikeyhew]

@alexchrichton I take it stdin.lock() wouldn't help, because they are different processes.

@Marwes, you use windows — what would you do here?

[alexcrichton]

Yeah unfortunately stdin.lock() wouldn't help the cross-process issue here

[Marwes]

@mikeyhew I use git bash for this reason and many others instead of cmd, so I am not any wiser of what's the best idea here (That SO link is the one I have used in the rare cases that I have needed it)

[alexcrichton]

@mikeyhew hm do you have thoughts for how we might land this? Do you think it's important enough to land a unix-only implementation?

[mikeyhew]

@alexcrichton ya, I think we should definitely support it at least on unix. For windows, I'm leaning toward the cmd option, because it can be pasted into the terminal and run:

cmd /c "set FOO=foo && BAR=bar && rustc --flag1 --flag2 file"

[alexcrichton]

Ok makes sense to me, @rust-lang/cargo do y'all have thoughts on the feature here? It's pretty wonky (albeit not impossible) to implement this on Windows but wanted to check to see if anyone feels we should do something different here!

[joshtriplett]

I absolutely want to see this land, to ensure that people can always re-run commands.

Regarding an implementation for Windows using cmd, I'd be deeply concerned about getting quoting correct to allow for copy-pasting.

[matklad]

Seems very useful! Does it make sense to show env only with -v -v, double verbose?

I also don’t think that we should go out of our way to make this copy-pastable on Windows. So, using VAR=foo binary syntax on windows seems fine by me: the user can adjust the command line themselthes depending on the shell being used.

[wycats]

I like gating it on -vv. I also think that if we think that the copy-pasting facility is useful on Linux and OSX, we should try to figure out how to make it work on Windows. I don't love that these micro-decisions often result in a noticeably worse user-experience on Windows in aggregate, and I don't think it would be too hard to figure out how to make this work properly.

[alexcrichton]

Ok thanks for the input! @mikeyhew sounds like a "yes" to let's do the windows weird cmd syntax :)

(and then test it a bit for quoting and whatnot)

[alexcrichton]

@mikeyhew ping on this, will you be able to implement the sytnax for windows?

[mikeyhew]

@alexcrichton I've been busy lately, so that's why I haven't gotten to this yet. I don't have a windows machine, so I'd need to rely on CI to run tests for me, but I'll try that this coming week unless someone else wants to do the windows version.

[alexcrichton]

Ok I'm going to close this for now as it's gone somewhat quite, but it can always be resubmitted!

[joshtriplett]

I ran into another request for this from someone trying to debug builds, and I'd like to reopen it in the hopes that someone may wish to address it.

To avoid the perfect becoming the enemy of the good: I do think a UNIX implementation would be great to have. I'd also like to see this work on Windows, and I think it's important it work everywhere, but a solution that works on UNIX and makes things no worse on Windows would be an improvement.

[bors]

☔ The latest upstream changes (presumably #6416) made this pull request unmergeable. Please resolve the merge conflicts.

[dwijnand]

Along with a rebase, this is currently failing CI.

[mikeyhew]

@dwijnand this never passed CI, since I didn't update the existing tests

[dwijnand]

Superseded by #6492.