Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
mgorny committed Feb 9, 2023
commit 6c05684f323cc01fccaaa62f1d492f60ae2df8e5
58 changes: 39 additions & 19 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ $(LIBRARY): $(LIBRARY_OBJS)
$(AR) $(ARFLAGS) $@ $(LIBRARY_OBJS)

libpython$(LDVERSION).so: $(LIBRARY_OBJS) $(DTRACE_OBJS)
set -e; \
if test $(INSTSONAME) != $(LDLIBRARY); then \
$(BLDSHARED) -Wl,-h$(INSTSONAME) -o $(INSTSONAME) $(LIBRARY_OBJS) $(MODLIBS) $(SHLIBS) $(LIBC) $(LIBM); \
$(LN) -f $(INSTSONAME) $@; \
Expand Down Expand Up @@ -894,7 +895,8 @@ $(LIBEXPAT_A): $(LIBEXPAT_OBJS)
# pybuilddir.txt is created too late. We cannot use it in Makefile
# targets. ln --relative is not portable.
sharedmods: $(SHAREDMODS) pybuilddir.txt
@target=`cat pybuilddir.txt`; \
@set -e; \
target=`cat pybuilddir.txt`; \
$(MKDIR_P) $$target; \
for mod in X $(SHAREDMODS); do \
if test $$mod != X; then \
Expand All @@ -907,7 +909,8 @@ checksharedmods: sharedmods $(PYTHON_FOR_BUILD_DEPS) $(BUILDPYTHON)
@$(RUNSHARED) $(PYTHON_FOR_BUILD) $(srcdir)/Tools/build/check_extension_modules.py

rundsymutil: sharedmods $(PYTHON_FOR_BUILD_DEPS) $(BUILDPYTHON)
@if [ ! -z $(DSYMUTIL) ] ; then \
@set -e; \
if [ ! -z $(DSYMUTIL) ] ; then \
echo $(DSYMUTIL_PATH) $(BUILDPYTHON); \
$(DSYMUTIL_PATH) $(BUILDPYTHON); \
if test -f $(LDLIBRARY); then \
Expand Down Expand Up @@ -1814,15 +1817,17 @@ commoninstall: check-clean-src @FRAMEWORKALTINSTALLFIRST@ \
DESTDIRS= $(exec_prefix) $(LIBDIR) $(BINLIBDEST) $(DESTSHARED)

sharedinstall: all
@for i in $(DESTDIRS); \
@set -e; \
for i in $(DESTDIRS); \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
else true; \
fi; \
done
@for i in X $(SHAREDMODS); do \
@set -e; \
for i in X $(SHAREDMODS); do \
if test $$i != X; then \
echo $(INSTALL_SHARED) $$i $(DESTSHARED)/`basename $$i`; \
$(INSTALL_SHARED) $$i $(DESTDIR)$(DESTSHARED)/`basename $$i`; \
Expand All @@ -1836,7 +1841,8 @@ sharedinstall: all
# Install the interpreter with $(VERSION) affixed
# This goes into $(exec_prefix)
altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
@for i in $(BINDIR) $(LIBDIR); \
@set -e; \
for i in $(BINDIR) $(LIBDIR); \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
Expand All @@ -1855,7 +1861,8 @@ altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
fi; \
(cd $(DESTDIR)$(BINDIR); $(LN) python$(LDVERSION)$(EXE) python$(VERSION)$(EXE)); \
fi
@if test "$(PY_ENABLE_SHARED)" = 1 -o "$(STATIC_LIBPYTHON)" = 1; then \
@set -e; \
if test "$(PY_ENABLE_SHARED)" = 1 -o "$(STATIC_LIBPYTHON)" = 1; then \
if test -f $(LDLIBRARY) && test "$(PYTHONFRAMEWORKDIR)" = "no-framework" ; then \
if test -n "$(DLLLIBRARY)" ; then \
$(INSTALL_SHARED) $(DLLLIBRARY) $(DESTDIR)$(BINDIR); \
Expand Down Expand Up @@ -1942,7 +1949,8 @@ bininstall: altbininstall

# Install the versioned manual page
altmaninstall:
@for i in $(MANDIR) $(MANDIR)/man1; \
@set -e; \
for i in $(MANDIR) $(MANDIR)/man1; \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
Expand Down Expand Up @@ -2077,15 +2085,17 @@ COMPILEALL_OPTS=-j0

TEST_MODULES=@TEST_MODULES@
libinstall: all $(srcdir)/Modules/xxmodule.c
@for i in $(SCRIPTDIR) $(LIBDEST); \
@set -e; \
for i in $(SCRIPTDIR) $(LIBDEST); \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
else true; \
fi; \
done
@if test "$(TEST_MODULES)" = yes; then \
@set -e; \
if test "$(TEST_MODULES)" = yes; then \
subdirs="$(LIBSUBDIRS) $(TESTSUBDIRS)"; \
else \
subdirs="$(LIBSUBDIRS)"; \
Expand All @@ -2101,7 +2111,8 @@ libinstall: all $(srcdir)/Modules/xxmodule.c
else true; \
fi; \
done
@for i in $(srcdir)/Lib/*.py; \
@set -e; \
for i in $(srcdir)/Lib/*.py; \
do \
if test -x $$i; then \
$(INSTALL_SCRIPT) $$i $(DESTDIR)$(LIBDEST); \
Expand All @@ -2111,7 +2122,8 @@ libinstall: all $(srcdir)/Modules/xxmodule.c
echo $(INSTALL_DATA) $$i $(LIBDEST); \
fi; \
done
@if test "$(TEST_MODULES)" = yes; then \
@set -e; \
if test "$(TEST_MODULES)" = yes; then \
subdirs="$(LIBSUBDIRS) $(TESTSUBDIRS)"; \
else \
subdirs="$(LIBSUBDIRS)"; \
Expand Down Expand Up @@ -2201,7 +2213,8 @@ scripts: $(SCRIPT_2TO3) $(SCRIPT_IDLE) $(SCRIPT_PYDOC) python-config
# Install the include files
INCLDIRSTOMAKE=$(INCLUDEDIR) $(CONFINCLUDEDIR) $(INCLUDEPY) $(CONFINCLUDEPY)
inclinstall:
@for i in $(INCLDIRSTOMAKE); \
@set -e; \
for i in $(INCLDIRSTOMAKE); \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
Expand All @@ -2219,17 +2232,20 @@ inclinstall:
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$(INCLUDEPY)/internal; \
else true; \
fi
@for i in $(srcdir)/Include/*.h; \
@set -e; \
for i in $(srcdir)/Include/*.h; \
do \
echo $(INSTALL_DATA) $$i $(INCLUDEPY); \
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY); \
done
@for i in $(srcdir)/Include/cpython/*.h; \
@set -e; \
for i in $(srcdir)/Include/cpython/*.h; \
do \
echo $(INSTALL_DATA) $$i $(INCLUDEPY)/cpython; \
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY)/cpython; \
done
@for i in $(srcdir)/Include/internal/*.h; \
@set -e; \
for i in $(srcdir)/Include/internal/*.h; \
do \
echo $(INSTALL_DATA) $$i $(INCLUDEPY)/internal; \
$(INSTALL_DATA) $$i $(DESTDIR)$(INCLUDEPY)/internal; \
Expand All @@ -2244,15 +2260,17 @@ LIBPL= @LIBPL@
LIBPC= $(LIBDIR)/pkgconfig

libainstall: all scripts
@for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
@set -e; \
for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
do \
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $$i"; \
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
else true; \
fi; \
done
@if test "$(STATIC_LIBPYTHON)" = 1; then \
@set -e; \
if test "$(STATIC_LIBPYTHON)" = 1; then \
if test -d $(LIBRARY); then :; else \
if test "$(PYTHONFRAMEWORKDIR)" = no-framework; then \
if test "$(SHLIB_SUFFIX)" = .dll; then \
Expand Down Expand Up @@ -2282,7 +2300,8 @@ libainstall: all scripts
$(INSTALL_SCRIPT) $(SCRIPT_2TO3) $(DESTDIR)$(BINDIR)/2to3-$(VERSION)
$(INSTALL_SCRIPT) $(SCRIPT_IDLE) $(DESTDIR)$(BINDIR)/idle$(VERSION)
$(INSTALL_SCRIPT) $(SCRIPT_PYDOC) $(DESTDIR)$(BINDIR)/pydoc$(VERSION)
@if [ -s Modules/python.exp -a \
@set -e; \
if [ -s Modules/python.exp -a \
"`echo $(MACHDEP) | sed 's/^\(...\).*/\1/'`" = "aix" ]; then \
echo; echo "Installing support files for building shared extension modules on AIX:"; \
$(INSTALL_DATA) Modules/python.exp \
Expand Down Expand Up @@ -2322,7 +2341,8 @@ frameworkinstallstructure: $(LDLIBRARY)
exit 1; \
else true; \
fi
@for i in $(prefix)/Resources/English.lproj $(prefix)/lib; do\
@set -e; \
for i in $(prefix)/Resources/English.lproj $(prefix)/lib; do\
if test ! -d $(DESTDIR)$$i; then \
echo "Creating directory $(DESTDIR)$$i"; \
$(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix error handling in compound Make rules that lead to the install targets
succeeding when the respective files were not installed.