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

Tracking issue for std::process::ExitCode (feature process_exitcode_placeholder) #48711

Closed
1 task
scottmcm opened this issue Mar 4, 2018 · 20 comments
Closed
1 task
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Mar 4, 2018

https://doc.rust-lang.org/nightly/std/process/struct.ExitCode.html

Code added in #48497 (comment); changed to this issue in #48618
Creating this because ?-in-main (#43301) will probably stabilize first

This is very minimal so far, and hasn't had an RFC. IRLO thread for what it should be.

Unresolved Questions

@frewsxcv frewsxcv added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Mar 4, 2018
@yoshuawuyts
Copy link
Member

yoshuawuyts commented May 6, 2018

From discussions we've had in the WG-CLI gitter, something that would be amazing to have would be a way to enumerate all possible exit codes for a program.

We're currently looking into automatically creating man pages for CLIs, and being able to automatically document which exit codes are possible would let Rust do something that no other language does.

I don't have any proposals on how to best approach this, but figured it would probably be worth bringing up.

Thanks!

edit: PS. I hope this is the right thread to mention this in! Apologies if I got it wrong!

@scottmcm
Copy link
Member Author

scottmcm commented May 7, 2018

@yoshuawuyts Sounds like a cool feature! I don't know if this struct is exactly the thing you'd need, though, since it'll probably just remain a newtype. Maybe you could have a custom derive to generate Termination or Into<ExitCode> on an enum where the variants are the possible codes, as a place to put the numbers and documentation about them, and still use in the code?

@raphaelcohn
Copy link

Would you consider also adding the definitions from sysexits.h, which are commonly used by mail programs (eg postfix) amongst others?

For example:-

#define EX_USAGE 64
#define EX_DATAERR 65
#define EX_NOINPUT 66
#define EX_NOUSER 67
#define EX_NOHOST 68
#define EX_UNAVAILABLE 69
#define EX_SOFTWARE 70

See https://github.com/bminor/musl/blob/master/include/sysexits.h for a full list. All part of any POSIX libc.

In particular, it'd be nice if an uncaught panic resulted in an exit code of EX_SOFTWARE on POSIX systems rather than EXIT_FAILURE...

@scottmcm
Copy link
Member Author

scottmcm commented May 17, 2018

@raphaelcohn The design space here is still wide open, so it's certainly possible. There are some thoughts in the IRLO thread, but really it needs someone to dig in and make a proposal.

As for uncaught panics, I believe they currently give a 101 exit code, not EXIT_FAILURE.

@JoshMcguigan
Copy link

...something that would be amazing to have would be a way to enumerate all possible exit codes for a program... currently looking into automatically creating man pages for CLIs

@yoshuawuyts Has any progress been made on this? I'm wondering if https://github.com/JoshMcguigan/exit could be expanded or somehow tied into some broader cli library to solve this problem?

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Feb 18, 2019

@JoshMcguigan hey, thanks for asking! I read your blog post, and I really like the work you've been doing!

To answer your question: man now supports defining custom exit codes. However we haven't created any sort of bridge (e.g. the closest we've come is to create a bridge to clap).

That said, I do have some thoughts on this. I was chatting with @davidbarsky last week about the failure crate, and the exitfailure crate came up. This fills a similar program slot as exit, but does a slightly different job.

It's worth mentioning that at the Rust all-hands there was talk about moving error chains (.context()) into stdlib. Unless anything has changed in the past week, it seems likely there'll be an RFC for this.

I'm wondering if we could somehow merge all of these features into a single crate, and find a solution that works for most people. The features I'm currently thinking of are:

  • be able to define custom exit codes, with a reason
  • be able to use use pre-defined exit codes (e.g. from the exitcode crate, + exit code 101 for Rust panics).
  • log a list of causes using the log crate on error. (similar to exitfailure, but without writing directly to stdout.)
  • be able to define these exit statuses in a separate struct, so it can be exported as a man page in a build.rs script without needing to do a double compile of all of the code.

Haha, I hope all this makes sense. Sorry it's a lot of text; it's something I've been thinking about for a few months, but didn't quite get a chance to connect all pieces until now. I hope this is useful!

@bazaah
Copy link

bazaah commented Aug 27, 2019

I'm not sure if this or the ?-in-main thread is the correct venue for my question, so feel free to slap me if this is the wrong place.

I'm curious what needs doing before I can return an arbitrary exit code (between 1-127) from main using ? in main on stable. I currently do this on nightly by implementing Try + Termination on an enum that is basically a specialized Option where T: From<i32> + Error, with None representing a successful program exit.

I've read most of the links in #43301 and the preRFC but am still uncertain about status of my need, or how go about implementing it.

@jonas-schievink jonas-schievink added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Nov 26, 2019
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 31, 2020
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 6, 2022
Add From<u8> for ExitCode

This should cover a mostly cross-platform subset of supported exit codes.

We decided to stick with `u8` initially since its the common subset between all platforms that we support (excluding wasm which I think only works with `true` or `false`). Posix is supposed to take i32s, but in practice many unix platforms mask out all but the low 8 bits or in some cases the 8-15th bits. Windows takes a u32 instead of an i32. Bourne-compatible shells also report signals as exitcode 128 + `signal_no`, so there's some ambiguity there when returning exit codes > 127, but it is possible to disambiguate them on the other side so we decided against restricting the possible codes further than to `u8`.

## Related

- Detailed analysis of exit code support on various platforms: https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426
- rust-lang#48711
- rust-lang#43301
- https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 7, 2022
Add From<u8> for ExitCode

This should cover a mostly cross-platform subset of supported exit codes.

We decided to stick with `u8` initially since its the common subset between all platforms that we support (excluding wasm which I think only works with `true` or `false`). Posix is supposed to take i32s, but in practice many unix platforms mask out all but the low 8 bits or in some cases the 8-15th bits. Windows takes a u32 instead of an i32. Bourne-compatible shells also report signals as exitcode 128 + `signal_no`, so there's some ambiguity there when returning exit codes > 127, but it is possible to disambiguate them on the other side so we decided against restricting the possible codes further than to `u8`.

## Related

- Detailed analysis of exit code support on various platforms: https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426
- rust-lang#48711
- rust-lang#43301
- https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 7, 2022
Add From<u8> for ExitCode

This should cover a mostly cross-platform subset of supported exit codes.

We decided to stick with `u8` initially since its the common subset between all platforms that we support (excluding wasm which I think only works with `true` or `false`). Posix is supposed to take i32s, but in practice many unix platforms mask out all but the low 8 bits or in some cases the 8-15th bits. Windows takes a u32 instead of an i32. Bourne-compatible shells also report signals as exitcode 128 + `signal_no`, so there's some ambiguity there when returning exit codes > 127, but it is possible to disambiguate them on the other side so we decided against restricting the possible codes further than to `u8`.

## Related

- Detailed analysis of exit code support on various platforms: https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426
- rust-lang#48711
- rust-lang#43301
- https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization
JohnTitor added a commit to JohnTitor/rust that referenced this issue Feb 9, 2022
Add From<u8> for ExitCode

This should cover a mostly cross-platform subset of supported exit codes.

We decided to stick with `u8` initially since its the common subset between all platforms that we support (excluding wasm which I think only works with `true` or `false`). Posix is supposed to take i32s, but in practice many unix platforms mask out all but the low 8 bits or in some cases the 8-15th bits. Windows takes a u32 instead of an i32. Bourne-compatible shells also report signals as exitcode 128 + `signal_no`, so there's some ambiguity there when returning exit codes > 127, but it is possible to disambiguate them on the other side so we decided against restricting the possible codes further than to `u8`.

## Related

- Detailed analysis of exit code support on various platforms: https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426
- rust-lang#48711
- rust-lang#43301
- https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization
@jplatte
Copy link
Contributor

jplatte commented Mar 30, 2022

Stabilized in #93840.

@gluax
Copy link

gluax commented Jun 20, 2022

Hi all, been looking through a few issues but didn't see what I was looking for. Is there any intent for the following behavior:

use std::process::{ExitCode, Termination};

#[derive(Debug)]
pub enum MyError {
    Internal,
    Other,
}

impl Termination for MyError {
    fn report(self) -> ExitCode {
        match self {
            MyError::Internal => ExitCode::from(1),
            MyError::Other => ExitCode::from(255),
        }
    }
}

fn main() -> Result<(), MyError> {
    Err(MyError::Other)?;
    Ok(())
}

Where the program would return the error code of the Error from the Result type?

@yaahc
Copy link
Member

yaahc commented Jun 20, 2022

@gluax the current plan is to expose this functionality through the generic member access RFC / provider API once those have been added to Error. https://github.com/yaahc/rfcs/blob/master/text/0000-dyn-error-generic-member-access.md

fn provide_context<'a>(&'a self, mut request: &mut Request<'a>) {
    request
        .provide_ref::<Backtrace>(&self.backtrace)
        .provide_value::<ExitCode>(self.exit_code);
}

Note: the RFC is a bit out of date and needs to be updated still

@sanmai-NL
Copy link

@yaahc To me that reads a lot more complicated than @gluax' sample.

@gluax
Copy link

gluax commented Jun 21, 2022

Agreed, I think it's better for Result<T, E> in main to auto return the underlying ExitCode from the Termination trait if it exists on E. Otherwise, no matter what accessor we have, it's clunky to grab it and return it ourselves while still maintaining the traditional behavior of printing, etc.

Could we not do something like the following:

impl<T, E> Termination for Result<T, E> where
E: Termination {
// Do match pattern here and grab report but also display the error as is standard.
}

impl<T, E> Termination for Result<T, E> where
E: !Termination {
// This would be the old impl making it valid for all E types who do not have Termination implemented.
}

I'm not entirely sure the above pattern would work, but I feel something akin to it should be possible?

@yaahc
Copy link
Member

yaahc commented Jun 21, 2022

I'm not entirely sure the above pattern would work, but I feel something akin to it should be possible?

To my knowledge, this kind of feature cannot be implemented. Unfortunately, we must also deal with the fact that stable Termination impls for Result already exist.

existing Result termination impls

Your example makes use of negative trait bounds, which is a feature we are highly unlikely to accept because, as far as I know, it makes trait solving infeasible1. I know @nikomatsakis suggested a variant of negative bounds in which a negative bound implies that you have an explicit negative impl, but then you'd still need to cover the case where you have neither a positive impl nor a negative impl, and I'm not sure how that would work.

I think it's more likely that we'll want to solve this problem through specialization, but specialization like this—Termination + Debug over Debug—is currently unsound, and we don't know if we'll ever get it. The same problem arises when specializing Error over Debug, and it never ceases to make me sad. I wish I knew how to reach a solution as straightforward as the one @gluax provided, but sad to say, I do not.

Footnotes

  1. https://rust-lang.zulipchat.com/#narrow/stream/315151-t-types.2Fnegative-impls/topic/Negative.20trait.20bounds.20and.20coherence

@gluax
Copy link

gluax commented Jun 21, 2022

@yaahc thanks for that clarification and explanation!

@jplatte
Copy link
Contributor

jplatte commented Jun 22, 2022

Actually, your initial snippet is just going to work in a few releases, those two Result impls mentioned above were replaced by one generalized Result, impl recently: https://doc.rust-lang.org/nightly/std/process/trait.Termination.html#impl-Termination-4

Edit: oh, actually it's not going to use the ExitCode impl and just return the default error code.

@gluax
Copy link

gluax commented Jun 22, 2022

@jplatte is there? I see the trait implemented as Result<T: Termination, E: Debug>. Which unfortunately doesn't do me or any applications I usually work on any good, as my exit codes are attached to the error generic. This makes more sense since errors are usually what return status codes and not successes who just return 0.

@jplatte
Copy link
Contributor

jplatte commented Jun 22, 2022

Yeah, I shouldn't comment in the middle of the night with half my brain still asleep 😅

yaahc was entirely right that using a Termination impl on E, if availabe, is blocked on (full) specialization.

@SUPERCILEX
Copy link
Contributor

SUPERCILEX commented Oct 4, 2022

I came here looking for the Termination impl on the E side of Result after migrating to error_stack and I'm so confused. Why was #97803 (comment) accepted as a common use case for implementing termination on the success side? The code didn't improve IMO.

I think it's more likely that we'll want to solve this problem through specialization, but specialization like this—Termination + Debug over Debug—is currently unsound, and we don't know if we'll ever get it.

If what @yaahc said is true then this is so sad. Everyone is now doomed to write:

match result {
    Ok(o) => o.report(),
    Err(err) => {
        drop(writeln!(io::stderr(), "Error: {err:?}"));
        err.report()
    }
}

When you could have just written result. I'm almost certain that's a much more common use case. It's also more intuitive... why on earth would success return an exit code? It's always 0. So sure, you're redefining Result to be Either and then Left doesn't mean Ok so why not pick that one for the exit code, but we live in the world where Result has Left=Ok and Right=Err. The code example provided in #97803 is also so weird: it's matching on an Err and then returning an Ok. Anyway, at this point I'm just lamenting what could have been.

Is it possible to reverse this decision in the next edition?

And unless I completely misunderstood the use case, could the people involved evaluate the process that led to this state? Reading the PR comments it seems like this was a quick and dirty decision that didn't have time to receive input from stakeholders. At the very least a separate issue should have been created in the https://github.com/rust-lang/libs-team repo.

@SUPERCILEX
Copy link
Contributor

Is it possible to reverse this decision in the next edition?

If this is not possible, I just thought of something that would make the situation a little less bad. Since we'd be stuck with the convention of having Termination on the success side, we could do the same for Option and impl<T: Debug + Termination> Termination for Option<T> where None -> ExitCode::SUCCESS. I kind of hate that because we're just digging ourselves deeper into nonsense since I would think that None should be FAILURE, but it would allow you to do result.err() which is a lot simpler than the match statement I provided above. Thoughts on that?

@joshtriplett
Copy link
Member

This tracking issue no longer seems to be tracking anything. The relevant feature has been shipped some time ago.

There's additional work we may want to do in the future, but that's not being tracked by this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests