/bin/rmdir: Exit with status 2 for invalid arguments

[hhartzer]

PR: 277677

Signed-off-by: henrichhartzer@tuta.io

[rilysh]

Do you've any relevant information about it? Exit status of rmdir (on error) seems to always was 1, and still 1 on other BSDs (OpenBSD and NetBSD).

[hhartzer]

It's a common convention to have invalid arguments exit 2 instead of 1. This helps you know if you're using the program incorrectly, or if there was an error while running with correct arguments.

Many other programs follow this approach (I can provide examples if you like). I can't see what this might break -- most code checks for if the program did not exit with status 0, and not explicitly that it errored with status 1 to indicate a failure.

This just seems like a logical direction to me, even though it's very minor.

[llfw]

fwiw, <sysexits.h> has:

#define EX_USAGE        64      /* command line usage error */

but i have the impression these codes are considered a bit deprecated nowadays.

[bsdimp]

Generally I like it, but we should update the man page to document this.
Please do NOT bump .Dd when you do that, but otherwise make sure that igor is happy. I'll bump the date landing.

[bsdimp]

fwiw, <sysexits.h> has:

#define EX_USAGE        64      /* command line usage error */

but i have the impression these codes are considered a bit deprecated nowadays.

Generally, yes. Some programs make religious use of them, others exit(1) or err(1) w/o much thought.

[hhartzer]

I did not know about EX_USAGE! Very cool!

@bsdimp do you want me to update to EX_USAGE, or just do the man page with status 2?

[concussious]

Fwiw I think it's very handsome to use EX_USAGE. This is consistent with the information presented in the intro to general commands. Consistency is beautiful and discoverable.

[hhartzer]

Fwiw I think it's very handsome to use EX_USAGE. This is consistent with the information presented in the intro to general commands. Consistency is beautiful and discoverable.

I agree! I wish I knew about it sooner. Python even has os.EX_USAGE.

[hhartzer]

Here's an updated commit with man page updates and sysexit.h usage.

I was not confident about which sysexit.h status applied here, for the typical non-zero case that isn't usage. I read through and none of them seemed terribly applicable. Or maybe EX_OSERR?

I'd like to update it once more to either use something like EX_OSERR for the other condition, or maybe be more explicit that it will exit with code 1.

I modeled the man page off of lockf(1), which makes very nice use of sysexit.h.

After looking at sysexit, personally, I feel like I'd prefer one that had EX_OK, EX_USAGE, and EX_ERROR. It's quite opinionated and takes a while to find something matching. That, or adding something like EX_GENERIC_ERROR, so you can use sysexit.h without having to think too much about it.

I'm sure for some programs precise errors are nice, but it seems fairly rare to be tracking exactly which exit status (beyond usage) and acting on that.

I'm happy to squash these commits if desired. Please let me know where to go from here.

[bsdimp]

Sorry for this late feedback... But I have a question

So what's the motivating use case here? After polling other developers, I've learned that the EX_xxx values have fallen out of favor since 4.4BSD...

[hhartzer]

No problem! This is really a minor thing. I like when there's a different exit status for usage than a failure during runtime. Whether EX_USAGE vs 2, I don't care too much.

There's a benefit that during tests, you're very explicit if you want to catch a usage-based failure, or a runtime-based failure. It can also be beneficial writing scripts if you see that something exits 2, indicating that you simply used the program the wrong way.

In the case of rmdir, with its two arguments, it is a rare issue indeed. I just like the convention of exit 0, 1, or 2 (or EX_*).

However, this may well be a case of if it's not broken don't fix it, and it's such a minor detail that it's best to just leave it alone.

I do feel that if sysexits has fallen out of favor, it should be deprecated in some manner. Maybe just a note in the man page, or maybe a more concerted effort.

My suspicion is that sysexits is too opinionated, and most would be better served by a simplified version of just EX_OK, EX_FAILURE, EX_USAGE. If there was interest in that, there's several directions one could take. EX_FAILURE could be added to sysexits and the man page could be amended to suggest just using one of the three exit codes, that the others may be overkill for most applications. Alternatively, there could be a different header file for more modern usage.

Or we just leave it as-is and see how many decades rmdir and sysexits stay exactly the same. There's probably bigger fish to fry than rmdir's exit codes.

[concussious]

I've learned that the EX_xxx values have fallen out of favor since 4.4BSD.

Sir, if you can (have time to) share any more information on this, I would be honored to update the third paragraph of intro(1) accordingly.

[llfw]

i think i might have unnecessarily confused this discussion by mentioning sysexits.h, sorry! but, if this is no longer used that should be mentioned somewhere (including in the header itself).

[concussious]

i think i might have unnecessarily confused this discussion by mentioning sysexits.h, sorry! but, if this is no longer used that should be mentioned somewhere (including in the header itself).

Fwiw I really appreciate this discussion.

[bsdimp]

Let me find a good reference to and I'll provide info you can use to do that.

[bsdimp]

let's lose the EX_* patches, and go ahead and make the stuff exit(2). However, there's lots of other programs that don't have the right exit status to do automated command line testing and identification of invalid options and combinations.

We got rid of it in 2008, but it had been the prevailing wisdom for years before that. See a1432b4 for the commit that did this.

There were sweeps in the tree in the 90s to introduce this, and then backout sweeps in the early 2000s, that culminated in the above style(9) change... though they were incomplete, and EX_USAGE still remains in rm. A similar change wasn't made to rmdir, though, which is why it is still 1. In theory, other programs should do this too, at the very least rm and rmdir should be consistent I'd think. I believe POSIX is silent on this issue.

[hhartzer]

Sounds good! I'll get started on that soon.

Speaking of style(9), should it then suggest exit(2) for usage and exit(1) for other failures? It seems to currently encourage exit(1) for usage.

https://github.com/freebsd/freebsd-src/blob/main/share/man/man9/style.9#L872

[hhartzer]

Should be ready to review again. Please let me know what you think. Thank you!

[bsdimp]

I think that the commits need to be squashed as well down to one commit that should at this point be almost a one liner, apart for the tests and man changes.

[hhartzer]

This is squashed now. Thank you for the feedback, @bsdimp and @concussious!

Please let me know if this needs any other adjustments.

[bsdimp]

OK. This is ready to land.
It's clear to me, though, that we should maybe pause changes like this and have the conversation that was started in the phab that was referenced in the sysexits.h review.
So i'll land this one (since there's at least a weak argument that rm and rmdir should behave similarly), but reject future ones until the bigger issue is discussed and consensus reached.

[hhartzer]

Thank you! That sounds completely reasonable. I appreciate all of the thought and feedback over such a simple change. Hopefully some more consensus and standards can be found moving forward.