Fix up Command Debug output when arg0 is specified.

[jsgf]

PR #66512 added the ability to set argv[0] on
Command. As a side effect, it changed the Debug output to print both the program and
argv[0], which in practice results in stuttery output ("echo" "echo" "foo").

This PR reverts the behaviour to the the old one, so that the command is only printed
once - unless arg0 has been set. In that case it emits "[command]" "arg0" "arg1" ....

[rust-highfive]

r? @sfackler

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

[joshtriplett]

@bors r+

[bors]

📌 Commit ce56e75 has been approved by joshtriplett

[bors]

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

[Mark-Simulacrum]

I am nominating this for beta backport as it seems it's regressing some crates (i.e., this PR landed too late to fix beta as well). I tagged with T-compiler and T-libs; but I feel fairly confident that we just want to backport this. If T-compiler decides not to approve we'll want to ping T-libs as they don't generally notice otherwise I think.

• root: Srynetix/pssh-rs: start v. end
• root: submodules-0.1.0: start v. end

[jsgf]

@Mark-Simulacrum Do any of them seem to be using the Debug output for real functionality, or is it just unit tests failing?

[Mark-Simulacrum]

AFAICT, just unit tests - though some are calling public functions, so a little unclear. The duplication of the executable though is somewhat annoying (and will affect other places, e.g. rustc output on failure to run linkers, I imagine).

[Mark-Simulacrum]

@rust-lang/libs -- this is nominated for beta backport, and T-compiler (likely correctly :) decided that they feel that libs team should approve this. @pnkfelix will also write up a comment hopefully with some thoughts in an hour or so.

[pnkfelix]

Oh yeah, I promised a note here.

My instinct is that I wouldn't backport this PR, since it is inserting this new square-bracket delimted [command]. To me, a change like that seems like it would be better off riding the trains.

At the same time, there is a very real regression that should be addressed in some manner.

So what I would advise (and I'm speaking with basically zero authority here, since I'm not really involved with any libs design, apart from at best the Allocator API and the defunct placement-in APIs) is to backport a PR reverting all of #66512 from beta. But keep #66512 on nightly, and let this PR land there.

---

Before I started looking at this bug, I would have said that I believe our intent is that we are free to change std::fmt::Debug implementations for stdlib types at will. However, I am not sure if that policy is set in stone anywhere, so I am starting to doubt my own understanding of the guarantees here. The docs for the Debug trait and for std::fmt do not seem to make any strong statements giving stdlib the freedom that I thought it had to revise Debug implementations at will.

---

Update: Also, to be clear: as I understand it, this change really shouldn't be breaking anyone, at least no one who isn't already making use of the (brand-new) functionality to modify argv[0]. My arguments here are not really about stability guarantees. Rather, it is about giving people the chance, by riding the nightly trains, to discover problems with the choice to use square-brackets as the way to delimit the two distinct program name vs argv[0] values.)

[sfackler]

Debug representations are subject to change at any time.

[Mark-Simulacrum]

To be clear, I consider the behavior on beta at least suboptimal (double printing the program), and plausibly a bug. Given instability of Debug, though, we may decide to not actually backport anything and merely note it in the release notes.

[jsgf]

The ["executable"] output will only appear if you're using the new API. With this change, the Debug output is unchanged for existing uses and hence avoid the double-printing. Without this change the Debug output changes for no reason and in a generally unhelpful way.

I see this as being a bugfix to the original PR and I think it would be best to backport it so that they both land together. Otherwise we'll end up with a release with a Debug output regression.

[alexcrichton]

I personally think it's fine to backport this, so I'm going to accept it.

[jsgf]

@alexcrichton What's the actual process for backporting? Should I submit a PR?

[Mark-Simulacrum]

I will be doing a beta backport rollup today or tomorrow probably and this'll get picked up since it's been approved, so no need to do anything further.