Uh oh!
There was an error while loading. Please reload this page.
gh-100220: Fix error handling in make rules - #100328
Conversation
bedevere-bot
commented
Jan 11, 2023
zware
commented
Feb 8, 2023
This looks good, but is going to need some conflict resolution now after GH-100329. I wonder if this could be achieved with less churn by setting |
Use `set -e` before compound shell commands in order to ensure that make targets fail correctly when at least one of the subcommands fail. This is necessary since make considers a target failed only if one of the shell invocations returns with unsuccessful exit status. If a shell script does not exit explicitly, the shell uses the exit status of the *last* executed command. This means that when multiple commands are executed (e.g. through a `for` loop), the exit statuses of prior command invocations are ignored. This can be either resolved by adding an explicit `|| exit 1` to every command that is expected to succeed, or by running the whole script with `set -e`. The latter was used here as it the rules seem to be written with the assumption that individual commands were supposed to cause the make rules to fail.
mgorny
commented
Feb 9, 2023
Done.
Hmm, I haven't though of that. I suppose it could work too. |
merwok
commented
Feb 9, 2023
Please use regular merges and pushes for Python repos, force-pushes give reviewers a poor experience on github. |
mgorny
commented
Mar 2, 2023
@zware, do you want to rework this to try updating this to use |
zware
commented
Mar 2, 2023
If the |
Instead of setting `set -e` per target, add it to the definition of `SHELL` to force it globally. This was suggested by Zachary Ware.
mgorny
commented
Mar 3, 2023
Switched to the |
bedevere-bot
commented
Mar 3, 2023
zware
commented
Mar 6, 2023
The two buildbot failures don't appear to have anything to do with this; they shouldn't block a merge. @kushaldas, you had assigned yourself to this one; WDYT? |
mgorny
commented
Apr 5, 2023
Ping. |
Uh oh!
There was an error while loading. Please reload this page.
mgorny
commented
Apr 8, 2023
Thank you! |
Set `SHELL = /bin/sh -e` to ensure that complex recipes fail on the first error rather than incorrectly reporting success. Co-authored-by: Zachary Ware <zach@python.org>
Use
set -ebefore compound shell commands in order to ensure that make targets fail correctly when at least one of the subcommands fail.This is necessary since make considers a target failed only if one of the shell invocations returns with unsuccessful exit status. If a shell script does not exit explicitly, the shell uses the exit status of the last executed command. This means that when multiple commands are executed (e.g. through a
forloop), the exit statuses of prior command invocations are ignored.This can be either resolved by adding an explicit
|| exit 1to every command that is expected to succeed, or by running the whole script withset -e. The latter was used here as it the rules seem to be written with the assumption that individual commands were supposed to cause the make rules to fail.make sharedinstalldoes not return failure if install commands fail #100220