Anton (Apr 26 2025 at 11:07): Anton (Apr 26 2025 at 11:07):I've found out that basic-cli's Cmd.status! doesn't actually have a convenient way to get the status if it is non-zero, so you get e.g. (CmdStatusErr (Other "Non-zero exit code: 2")). While the type is Cmd => Result I32 ..., the I32 can only be 0 in practice.
In accordance with Hyrum's Law, people are now relying on this behavior and are not actually checking the returned status. So I think we should correct the behavior of status and also (temporarily) rename the function to increase the likelihood that people become aware of this change in behavior.
We currently have:
status! : Cmd => Result I32 [CmdStatusErr InternalIOErr.IOErr]
I propose we turn this into:
## To replace the old use of the incorrect `status`
execute! : Cmd => Result {} [CmdErr InternalIOErr.IOErr]
## For those we actually want clean access to the exit code
exec_status! : Cmd => Result I32 [CmdErr InternalIOErr.IOErr]
What do you think?
Anton (Apr 28 2025 at 09:44):I'm going ahead with this proposal given the lack of objections.
Luke Boswell (Apr 28 2025 at 11:17):Sorry, I think I have an objection here
Anton (Apr 28 2025 at 11:18):Go ahead :)
Luke Boswell (Apr 28 2025 at 11:18):Isn't the behaviour your describing above just a bug in our implementation... like it should return Ok(2) instead of the error.
Anton (Apr 28 2025 at 11:18):Oh yes, I agree
Anton (Apr 28 2025 at 11:19):But I did not just want to fix the bug because "people are now relying on this behavior and are not actually checking the returned status"
Luke Boswell (Apr 28 2025 at 11:20):oof... hmm :upside_down:
I would still lean towards fixing it.
Luke Boswell (Apr 28 2025 at 11:20):It's a breaking change either way
Anton (Apr 28 2025 at 11:21):Yes, but it is important that it is not a silent change in behavior breaking change
Luke Boswell (Apr 28 2025 at 11:21):Would giving a nice big warning label be enough here?
Anton (Apr 28 2025 at 11:22):If someone upgrades to basic-cli 0.20 and they don't check the release notes and everything looks like it's working, they will never see the warning label
Luke Boswell (Apr 28 2025 at 11:24):On the other hand, now everone has to change thier scripts to rename that function
Luke Boswell (Apr 28 2025 at 11:24):So it's garunteed pain for everyone, or possibly a little pain for a few that dont check the upgrade notes
Luke Boswell (Apr 28 2025 at 11:25):And given where the platform is at in terms of maturity, being more experimental, I think it would be ok to make this kind of trade off
Anton (Apr 28 2025 at 11:26):A significant amount of people will not check the upgrade notes, especially if they're going from 0.19 to 0.23 or something like that.
Anton (Apr 28 2025 at 11:26):Avoiding silent failures has been important in Roc and I feel strongly about it
Luke Boswell (Apr 28 2025 at 11:28):I don't feel strongly about this...
How about just fixing it and renaming to execute. No need yo change the API or add another function.
Luke Boswell (Apr 28 2025 at 11:30):Or even just exec!
Anton (Apr 28 2025 at 11:30):We already have exec!
Anton (Apr 28 2025 at 11:31):My proposed execute! : Cmd => Result {} [CmdErr InternalIOErr.IOErr] was to fill the missing analog to exec!
Anton (Apr 28 2025 at 11:31):exec! is for reference:
exec! : Str, List Str => Result {} [CmdStatusErr InternalIOErr.IOErr]
Cmd => Result {} [CmdErr InternalIOErr.IOErr] seems like something a lot of people would want when they don't care about the specific exit code but just success or failure.
Anton (Apr 28 2025 at 11:56):seems like something a lot of people would want
This is already confirmed by how people are currently using status! without using the exit code:
https://github.com/ostcar/kingfisher/blob/6c4e83c28c0483fe126f8e10ac37d00d795344ef/build.roc#L24
https://github.com/isaacvando/basic-webserver/blob/ead72acb6e22c7cc74c2634d4bf83618ea1a924c/examples/todos.roc#L147
https://github.com/isaacvando/rtl/blob/9c3ea0a2b8e9eabce1800e2d6a4be2ed932615b2/rtl.roc#L114
https://github.com/megakilo/roc2048/blob/e92736a0db35e0e9ddf31e1e41349768d2ddf3c0/main.roc#L33
https://github.com/adomurad/r2e-platform/blob/6ff0c102e1322f71379edd46d4ec6d7e887634dc/build.roc#L108
Brendan Hansknecht (Apr 28 2025 at 16:29):In accordance with Hyrum's Law, people are now relying on this behavior and are not actually checking the returned status.
I think we should ignore this. Roc is too early to care about this and we would build up a ton of tech debt if we lock in things like this early. All version bumps are breaking changes today.
Brendan Hansknecht (Apr 28 2025 at 16:34):## To replace the old use of the incorrect `status`
execute! : Cmd => Result {} [CmdErr InternalIOErr.IOErr]
## For those we actually want clean access to the exit code
exec_status! : Cmd => Result I32 [CmdErr InternalIOErr.IOErr]
This API looks fine. Though having one be exec and the other be execute feels inconsistent.
One thing I do think we should change though. We probably should change the error type to explicitly understand the exit code. Instead of Other "Non-zero exit code: 2", it should be NonZeroExitCode 2
Brendan Hansknecht (Apr 28 2025 at 16:34):But yeah, general change seems like a good idea
Anton (Apr 28 2025 at 16:50):Though having one be
execand the other beexecutefeels inconsistent.
exec_cmd is another possibility but Cmd.exec_cmd is a bit weird too
Anton (Apr 28 2025 at 16:58):By the way, I thought status was a bad function name way before this. If a beginning programmer was asked "what happens if you call status on a command?", I think they would not guess the true behavior.
Brendan Hansknecht (Apr 28 2025 at 17:08):Yeah, and Cmd.execute_returning_status is way too long
Anton (Apr 28 2025 at 17:34):People may rarely need this function though, so it does not need to be short. I only want to use it because roc returns a specific exit code if there is a warning but no errors. I don't know how common it is to need to branch on a specific exit code in case of failure.
Brendan Hansknecht (Apr 28 2025 at 17:35):Not sure how common it is. Have seen it with git bisection as well. I'm sure there are other cases.
Last updated: Jun 16 2026 at 16:19 UTC