Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix #83462

Merged
merged 1 commit into from
Mar 27, 2021

Conversation

ijackson
Copy link
Contributor

Proper Unix terminology is "exit status" (vs "wait status"). "exit
code" is imprecise on Unix and therefore unclear. (As far as I can
tell, "exit code" is correct terminology on Windows.)

This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.

It is the identifier names that are wrong, as discussed at length in eg
https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html

Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
std::process::ExitStatus to std::process::ChildStatus or
something), but we should fix the message output. Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.

So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.

This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses. Hopefully no-one is matching against the
exit status string, except perhaps in tests.

Proper Unix terminology is "exit status" (vs "wait status").  "exit
code" is imprecise on Unix and therefore unclear.  (As far as I can
tell, "exit code" is correct terminology on Windows.)

This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.

It is the identifier names that are wrong, as discussed at length in eg
  https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
  https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html

Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output.  Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.

So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.

This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses.  Hopefully no-one is matching against the
exit status string, except perhaps in tests.

Signed-off-by: Ian Jackson <[email protected]>
@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2021
@ijackson ijackson changed the title ExitStatus: print "exit status: {}" rather than "exit code: {}" ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix Mar 25, 2021
@joshtriplett
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 25, 2021

📌 Commit 11e40ce has been approved by joshtriplett

@bors 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 Mar 25, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 25, 2021
…, r=joshtriplett

ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix

Proper Unix terminology is "exit status" (vs "wait status").  "exit
code" is imprecise on Unix and therefore unclear.  (As far as I can
tell, "exit code" is correct terminology on Windows.)

This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.

It is the identifier names that are wrong, as discussed at length in eg
  https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
  https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html

Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output.  Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.

So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.

This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses.  Hopefully no-one is matching against the
exit status string, except perhaps in tests.
@Dylan-DPC-zz
Copy link

failed in rollup

@Dylan-DPC-zz
Copy link

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 26, 2021
ijackson added a commit to ijackson/cargo that referenced this pull request Mar 26, 2021
"exit code" is wrong terminology on Unix.  I am trying to fix this in
Rust stdlib in
    rust-lang/rust#83462
but this currently breaks the cargo test suite.

See that MR for full explanation of the change.

Signed-off-by: Ian Jackson <[email protected]>
@ijackson
Copy link
Contributor Author

I'm fixing the test in cargo. ISTM that we ought to mention this in the release notes, so let's see if I can do this:

@rustbot modify labels +relnotes

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2021

Error: Label relnotes can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@Dylan-DPC-zz Dylan-DPC-zz added the relnotes Marks issues that should be documented in the release notes of the next release. label Mar 26, 2021
bors added a commit to rust-lang/cargo that referenced this pull request Mar 26, 2021
tests: Tolerate "exit status" in error messages

"exit code" is wrong terminology on Unix.  I am trying to fix this in Rust stdlib in rust-lang/rust#83462 but this currently breaks the cargo test suite.

See that MR for full explanation of the change.
@ijackson
Copy link
Contributor Author

@joshtriplett Thanks for getting rust-lang/cargo#9307 merged so quickly! Do you know whether the rust-lang CI tests use cargo main branch, or if I need to wait for the cargo change to be released or something?

@joshtriplett
Copy link
Member

I believe this will require updating the cargo submodule.

I don't know the policy for doing mid-cycle updates for something like this.

cc @ehuss

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2021

There's an update in-flight (#83418) that was blocked. I'll look at moving it forward today without RLS.

@ehuss ehuss mentioned this pull request Mar 26, 2021
@ehuss
Copy link
Contributor

ehuss commented Mar 27, 2021

#83418 has merged.
@bors retry

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2021
@ehuss
Copy link
Contributor

ehuss commented Mar 27, 2021

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Mar 27, 2021

📌 Commit 11e40ce has been approved by joshtriplett

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
…, r=joshtriplett

ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix

Proper Unix terminology is "exit status" (vs "wait status").  "exit
code" is imprecise on Unix and therefore unclear.  (As far as I can
tell, "exit code" is correct terminology on Windows.)

This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.

It is the identifier names that are wrong, as discussed at length in eg
  https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
  https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html

Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output.  Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.

So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.

This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses.  Hopefully no-one is matching against the
exit status string, except perhaps in tests.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
…, r=joshtriplett

ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix

Proper Unix terminology is "exit status" (vs "wait status").  "exit
code" is imprecise on Unix and therefore unclear.  (As far as I can
tell, "exit code" is correct terminology on Windows.)

This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.

It is the identifier names that are wrong, as discussed at length in eg
  https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
  https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html

Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output.  Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.

So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.

This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses.  Hopefully no-one is matching against the
exit status string, except perhaps in tests.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#79399 (Use detailed and shorter fs error explaination)
 - rust-lang#83348 (format macro argument parsing fix)
 - rust-lang#83462 (ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix)
 - rust-lang#83526 (lazily calls some fns)
 - rust-lang#83558 (Use DebugStruct::finish_non_exhaustive() in std.)
 - rust-lang#83559 (Fix Debug implementation for RwLock{Read,Write}Guard.)
 - rust-lang#83560 (Derive Debug for io::Chain instead of manually implementing it.)
 - rust-lang#83561 (Improve Debug implementations of Mutex and RwLock.)
 - rust-lang#83567 (Update rustup cross-compilation docs link)
 - rust-lang#83569 (Add regression tests for rust-lang#56445)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f41fdd into rust-lang:master Mar 27, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 27, 2021
ehuss pushed a commit to ehuss/cargo that referenced this pull request Apr 6, 2021
tests: Tolerate "exit status" in error messages

"exit code" is wrong terminology on Unix.  I am trying to fix this in Rust stdlib in rust-lang/rust#83462 but this currently breaks the cargo test suite.

See that MR for full explanation of the change.
@XAMPPRocky XAMPPRocky removed the relnotes Marks issues that should be documented in the release notes of the next release. label May 21, 2021
@XAMPPRocky
Copy link
Member

XAMPPRocky commented May 21, 2021

Going to untag this as relnotes, as the stability of Display impls is currently not guaranteed, and this isn't particularly notable besides that as far as I can tell. #72676

@ijackson ijackson deleted the exitstatus-message-wording branch August 24, 2021 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants