Release | Crates.io in CD pipeline doesn't handle cargo publish errors

[hannobraun]

See this run, which has lots of errors: https://github.com/hannobraun/Fornjot/runs/6311042754?check_suite_focus=true#step:9:76

If a cargo publish fails, the whole step should fail. Maybe it would even be better to abort the step, if a single cargo publish fails.

Crate::submit in release-operator seems to be the relevant piece of code:
https://github.com/hannobraun/Fornjot/blob/6df520ae30c2202c381b7cde8489621d0e67d5f4/tools/release-operator/src/registry.rs#L152-L180

I'm not seeing any checking of the command's return value there, so adding that and failing the whole release-operator, if cargo-publish fails, would probably address this issue.

[hannobraun]

Still is still the case, despite my attempted fix. Possibly my fix was still wrong, or maybe cargo publish doesn't return proper exit codes.

edit:

Here's a run that has lots of errors in the Release | crates.io step, but neither aborts nor reports the build as having failed: https://github.com/hannobraun/Fornjot/runs/7235411115?check_suite_focus=true

[hendrikmaus]

Just a guess: the shell steps in GitHub actions do not exit properly. I haven't looked at the pipeline yet, but we can try to set -e to abort the entire step after the first non-zero exit code. Better yet set -euo pipefail which will also catch errors within pipelines of multiple shell commands.

I will come up with a pull request after experimenting a bit.

[hendrikmaus]

According to this document set -e is active by default. Will have to do some tests to verify this behavior.

[hannobraun]

I will come up with a pull request after experimenting a bit.

Thank you, @hendrikmaus!

I don't have any more insight, unfortunately. The exit code must get swallowed somewhere, but no idea where specifically.

[hendrikmaus]

I confirmed that the piece of code referenced in the description is the culprit: it does not fail if the command executed returns non-zero. I was under the impression that it would, because the cmd_lib macro in use returns a Result type which is handled by ?.

I will take a closer look at it. Already built myself a simulation to reproduce the error.

[hannobraun]

I confirmed that the piece of code referenced in the description is the culprit: it does not fail if the command executed returns non-zero. I was under the impression that it would, because the cmd_lib macro in use returns a Result type which is handled by ?.

According to the documentation, it only returns an io::Result. So judging from that alone, it must either ignore exit codes, or if it doesn't, at least doesn't allow the caller to handle them. There's another result type that works the same.

I'm very surprised by this. Seems like a huge oversight for a library whose sole purpose is to do this kind of thing. And one at a version >1.0.0 no less. Honestly, this makes me wonder if we shouldn't just stop using it. The standard library might be less convenient, but at least it returns ExitStatus.

I will take a closer look at it. Already built myself a simulation to reproduce the error.

Thank you!

[hannobraun]

I'm working on publishing the next release right now. I'll take a stab at fixing this, in case that turns out relatively straight-forward. If not, I'll leave it for now.

Sorry, if that conflicts with some of the work you're doing, @hendrikmaus.

[hannobraun]

Fixed in #834! This build failed correctly: https://github.com/hannobraun/Fornjot/runs/7388995951?check_suite_focus=true

[hendrikmaus]

Nice, I was going to submit something quite similar. You beat me to it 😊
Maybe I'll add back the continuous output while the command is running.

[hannobraun]

Maybe I'll add back the continuous output while the command is running.

I think the output already is continuous? At least it should be. Command inherits stdin/stdout/stderr from the parent process.

[hendrikmaus]

You are correct; I was overthinking it and did not see that Command::status() actually runs the command 🤦🏻 I assumed it would only yield a status after a command was executed.

Good job!

[hannobraun]

Good job!

Thank you!