Skip to content

Commit 65d6cdb

Browse files
mgornymgorny
committed
gh-100220: Fix error handling in make rules
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.
1 parent 0fe61d0 commit 65d6cdb

File tree

3 files changed

+42
-19
lines changed

3 files changed

+42
-19
lines changed

‎Makefile.pre.in‎

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,7 @@ $(LIBRARY): $(LIBRARY_OBJS)
740740
$(AR) $(ARFLAGS) $@ $(LIBRARY_OBJS)
741741

742742
libpython$(LDVERSION).so: $(LIBRARY_OBJS) $(DTRACE_OBJS)
743+
set -e; \
743744
if test $(INSTSONAME) != $(LDLIBRARY); then \
744745
$(BLDSHARED) -Wl,-h$(INSTSONAME) -o $(INSTSONAME) $(LIBRARY_OBJS) $(MODLIBS) $(SHLIBS) $(LIBC) $(LIBM); \
745746
$(LN) -f $(INSTSONAME) $@; \
@@ -896,7 +897,8 @@ $(LIBEXPAT_A): $(LIBEXPAT_OBJS)
896897
# pybuilddir.txt is created too late. We cannot use it in Makefile
897898
# targets. ln --relative is not portable.
898899
sharedmods: $(SHAREDMODS) pybuilddir.txt
899-
@target=`cat pybuilddir.txt`; \
900+
@set -e; \
901+
target=`cat pybuilddir.txt`; \
900902
$(MKDIR_P) $$target; \
901903
for mod in X $(SHAREDMODS); do \
902904
if test $$mod != X; then \
@@ -909,7 +911,8 @@ checksharedmods: sharedmods $(PYTHON_FOR_BUILD_DEPS) $(BUILDPYTHON)
909911
@$(RUNSHARED) $(PYTHON_FOR_BUILD) $(srcdir)/Tools/build/check_extension_modules.py
910912

911913
rundsymutil: sharedmods $(PYTHON_FOR_BUILD_DEPS) $(BUILDPYTHON)
912-
@if [ ! -z $(DSYMUTIL) ] ; then \
914+
@set -e; \
915+
if [ ! -z $(DSYMUTIL) ] ; then \
913916
echo $(DSYMUTIL_PATH) $(BUILDPYTHON); \
914917
$(DSYMUTIL_PATH) $(BUILDPYTHON); \
915918
if test -f $(LDLIBRARY); then \
@@ -1804,7 +1807,8 @@ commoninstall: check-clean-src @FRAMEWORKALTINSTALLFIRST@ \
18041807
DESTDIRS= $(exec_prefix) $(LIBDIR) $(BINLIBDEST) $(DESTSHARED)
18051808

18061809
sharedinstall: $(DESTSHARED) all
1807-
@for i in X $(SHAREDMODS); do \
1810+
@set -e; \
1811+
for i in X $(SHAREDMODS); do \
18081812
if test $$i != X; then \
18091813
echo $(INSTALL_SHARED) $$i $(DESTSHARED)/`basename $$i`; \
18101814
$(INSTALL_SHARED) $$i $(DESTDIR)$(DESTSHARED)/`basename $$i`; \
@@ -1817,7 +1821,8 @@ sharedinstall: $(DESTSHARED) all
18171821

18181822

18191823
$(DESTSHARED):
1820-
@for i in $(DESTDIRS); \
1824+
@set -e; \
1825+
for i in $(DESTDIRS); \
18211826
do \
18221827
if test ! -d $(DESTDIR)$$i; then \
18231828
echo "Creating directory $$i"; \
@@ -1829,7 +1834,8 @@ $(DESTSHARED):
18291834
# Install the interpreter with $(VERSION) affixed
18301835
# This goes into $(exec_prefix)
18311836
altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
1832-
@for i in $(BINDIR) $(LIBDIR); \
1837+
@set -e; \
1838+
for i in $(BINDIR) $(LIBDIR); \
18331839
do \
18341840
if test ! -d $(DESTDIR)$$i; then \
18351841
echo "Creating directory $$i"; \
@@ -1848,7 +1854,8 @@ altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
18481854
fi; \
18491855
(cd $(DESTDIR)$(BINDIR); $(LN) python$(LDVERSION)$(EXE) python$(VERSION)$(EXE)); \
18501856
fi
1851-
@if test "$(PY_ENABLE_SHARED)" = 1 -o "$(STATIC_LIBPYTHON)" = 1; then \
1857+
@set -e; \
1858+
if test "$(PY_ENABLE_SHARED)" = 1 -o "$(STATIC_LIBPYTHON)" = 1; then \
18521859
if test -f $(LDLIBRARY) && test "$(PYTHONFRAMEWORKDIR)" = "no-framework" ; then \
18531860
if test -n "$(DLLLIBRARY)" ; then \
18541861
$(INSTALL_SHARED) $(DLLLIBRARY) $(DESTDIR)$(BINDIR); \
@@ -1935,7 +1942,8 @@ bininstall: altbininstall
19351942

19361943
# Install the versioned manual page
19371944
altmaninstall:
1938-
@for i in $(MANDIR) $(MANDIR)/man1; \
1945+
@set -e; \
1946+
for i in $(MANDIR) $(MANDIR)/man1; \
19391947
do \
19401948
if test ! -d $(DESTDIR)$$i; then \
19411949
echo "Creating directory $$i"; \
@@ -2070,15 +2078,17 @@ COMPILEALL_OPTS=-j0
20702078

20712079
TEST_MODULES=@TEST_MODULES@
20722080
libinstall: all $(srcdir)/Modules/xxmodule.c
2073-
@for i in $(SCRIPTDIR) $(LIBDEST); \
2081+
@set -e; \
2082+
for i in $(SCRIPTDIR) $(LIBDEST); \
20742083
do \
20752084
if test ! -d $(DESTDIR)$$i; then \
20762085
echo "Creating directory $$i"; \
20772086
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
20782087
else true; \
20792088
fi; \
20802089
done
2081-
@if test "$(TEST_MODULES)" = yes; then \
2090+
@set -e; \
2091+
if test "$(TEST_MODULES)" = yes; then \
20822092
subdirs="$(LIBSUBDIRS) $(TESTSUBDIRS)"; \
20832093
else \
20842094
subdirs="$(LIBSUBDIRS)"; \
@@ -2094,7 +2104,8 @@ libinstall: all $(srcdir)/Modules/xxmodule.c
20942104
else true; \
20952105
fi; \
20962106
done
2097-
@for i in $(srcdir)/Lib/*.py; \
2107+
@set -e; \
2108+
for i in $(srcdir)/Lib/*.py; \
20982109
do \
20992110
if test -x $$i; then \
21002111
$(INSTALL_SCRIPT) $$i $(DESTDIR)$(LIBDEST); \
@@ -2104,7 +2115,8 @@ libinstall: all $(srcdir)/Modules/xxmodule.c
21042115
echo $(INSTALL_DATA) $$i $(LIBDEST); \
21052116
fi; \
21062117
done
2107-
@if test "$(TEST_MODULES)" = yes; then \
2118+
@set -e; \
2119+
if test "$(TEST_MODULES)" = yes; then \
21082120
subdirs="$(LIBSUBDIRS) $(TESTSUBDIRS)"; \
21092121
else \
21102122
subdirs="$(LIBSUBDIRS)"; \
@@ -2194,7 +2206,8 @@ scripts: $(SCRIPT_2TO3) $(SCRIPT_IDLE) $(SCRIPT_PYDOC) python-config
21942206
# Install the include files
21952207
INCLDIRSTOMAKE=$(INCLUDEDIR) $(CONFINCLUDEDIR) $(INCLUDEPY) $(CONFINCLUDEPY)
21962208
inclinstall:
2197-
@for i in $(INCLDIRSTOMAKE); \
2209+
@set -e; \
2210+
for i in $(INCLDIRSTOMAKE); \
21982211
do \
21992212
if test ! -d $(DESTDIR)$$i; then \
22002213
echo "Creating directory $$i"; \
@@ -2212,17 +2225,20 @@ inclinstall:
22122225
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$(INCLUDEPY)/internal; \
22132226
else true; \
22142227
fi
2215-
@for i in $(srcdir)/Include/*.h; \
2228+
@set -e; \
2229+
for i in $(srcdir)/Include/*.h; \
22162230
do \
22172231
echo $(INSTALL_DATA) $$i $(INCLUDEPY); \
22182232
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY); \
22192233
done
2220-
@for i in $(srcdir)/Include/cpython/*.h; \
2234+
@set -e; \
2235+
for i in $(srcdir)/Include/cpython/*.h; \
22212236
do \
22222237
echo $(INSTALL_DATA) $$i $(INCLUDEPY)/cpython; \
22232238
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY)/cpython; \
22242239
done
2225-
@for i in $(srcdir)/Include/internal/*.h; \
2240+
@set -e; \
2241+
for i in $(srcdir)/Include/internal/*.h; \
22262242
do \
22272243
echo $(INSTALL_DATA) $$i $(INCLUDEPY)/internal; \
22282244
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY)/internal; \
@@ -2237,15 +2253,17 @@ LIBPL= @LIBPL@
22372253
LIBPC= $(LIBDIR)/pkgconfig
22382254

22392255
libainstall: all scripts
2240-
@for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
2256+
@set -e; \
2257+
for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
22412258
do \
22422259
if test ! -d $(DESTDIR)$$i; then \
22432260
echo "Creating directory $$i"; \
22442261
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
22452262
else true; \
22462263
fi; \
22472264
done
2248-
@if test "$(STATIC_LIBPYTHON)" = 1; then \
2265+
@set -e; \
2266+
if test "$(STATIC_LIBPYTHON)" = 1; then \
22492267
if test -d $(LIBRARY); then :; else \
22502268
if test "$(PYTHONFRAMEWORKDIR)" = no-framework; then \
22512269
if test "$(SHLIB_SUFFIX)" = .dll; then \
@@ -2275,7 +2293,8 @@ libainstall: all scripts
22752293
$(INSTALL_SCRIPT) $(SCRIPT_2TO3) $(DESTDIR)$(BINDIR)/2to3-$(VERSION)
22762294
$(INSTALL_SCRIPT) $(SCRIPT_IDLE) $(DESTDIR)$(BINDIR)/idle$(VERSION)
22772295
$(INSTALL_SCRIPT) $(SCRIPT_PYDOC) $(DESTDIR)$(BINDIR)/pydoc$(VERSION)
2278-
@if [ -s Modules/python.exp -a \
2296+
@set -e; \
2297+
if [ -s Modules/python.exp -a \
22792298
"`echo $(MACHDEP) | sed 's/^\(...\).*/\1/'`" = "aix" ]; then \
22802299
echo; echo "Installing support files for building shared extension modules on AIX:"; \
22812300
$(INSTALL_DATA) Modules/python.exp \
@@ -2315,7 +2334,8 @@ frameworkinstallstructure: $(LDLIBRARY)
23152334
exit 1; \
23162335
else true; \
23172336
fi
2318-
@for i in $(prefix)/Resources/English.lproj $(prefix)/lib; do\
2337+
@set -e; \
2338+
for i in $(prefix)/Resources/English.lproj $(prefix)/lib; do\
23192339
if test ! -d $(DESTDIR)$$i; then \
23202340
echo "Creating directory $(DESTDIR)$$i"; \
23212341
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \

‎Misc/ACKS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,7 @@ Tiago Gonçalves
640640
Chris Gonnerman
641641
Shelley Gooch
642642
David Goodger
643+
Michał Górny
643644
Elliot Gorokhovsky
644645
Hans de Graaff
645646
Tim Graham

‎Misc/NEWS.d/next/Build/2022-12-18-07-24-44.gh-issue-100220.BgSV7C.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix error handling in compound Make rules that lead to the install targets
2+
succeeding when the respective files were not installed.

0 commit comments

Comments
 (0)