make sharedinstall does not return failure if install commands fail
#100220
Comments
mgorny
changed the title
make sharedinstall no longer returns failure if install commands failmake sharedinstall does not retrn failure if install commands fail
mgorny
changed the title
make sharedinstall does not retrn failure if install commands failmake sharedinstall does not return failure if install commands fail
mgorny
added a commit
to mgorny/cpython
that referenced
this issue
Dec 18, 2022
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
added a commit
to mgorny/cpython
that referenced
this issue
Feb 9, 2023
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.
zware
added a commit
that referenced
this issue
Apr 7, 2023
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>
|
GH-100328 merged, thanks for the report and patch! I think we'll pass on backporting this; there haven't been reported problems in this vein before (though the recipes haven't changed much, so the underlying issue does exist in older branches), and there could be some lesser-used recipes that shouldn't use |
|
Yeah, I suppose the risk is relatively small, as it affects only a few less frequently used rules. |
warsaw
pushed a commit
to warsaw/cpython
that referenced
this issue
Apr 11, 2023
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug report
If
make sharedinstallfails to install some Python extensions, the make target wrongly succeeds. For example, I'm seeing:Nevertheless,
make installreturns successfully in this case. This causes major problems for automated builds since they end up with broken Python installs when the make target should have failed.I need to investigate why it's failing but that's a separate issue.
Complete build log (1.2M): dev-lang:python-3.12.0_alpha3:20221207-142002.log
The problem seems to be that the
sharedinstalltargets runs a single shell command and make doesn't check the exit status until its very end.I suspect the same problem may apply to other install rules.
Your environment
Linked PRs
The text was updated successfully, but these errors were encountered: