Skip to content

Commit

Permalink
gh-93584: Make all install+tests targets depends on all (GH-93589)
Browse files Browse the repository at this point in the history
All install targets use the "all" target as synchronization point to
prevent race conditions with PGO builds. PGO builds use recursive make,
which can lead to two parallel `./python setup.py build` processes that
step on each others toes.

"test" targets now correctly compile PGO build in a clean repo.
  • Loading branch information
tiran authored Jun 8, 2022
1 parent 875de61 commit 243ed54
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
28 changes: 16 additions & 12 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ LIBEXPAT_HEADERS= \

# Default target
all: @DEF_MAKE_ALL_RULE@
build_all: check-clean-src $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks \
Programs/_testembed python-config
build_all: check-clean-src $(BUILDPYTHON) platform oldsharedmods sharedmods \
gdbhooks Programs/_testembed python-config
build_wasm: check-clean-src $(BUILDPYTHON) platform oldsharedmods python-config

# Check that the source is clean when building out of source.
Expand Down Expand Up @@ -1663,7 +1663,7 @@ cleantest: all

# Run a basic set of regression tests.
# This excludes some tests that are particularly resource-intensive.
test: @DEF_MAKE_RULE@ platform
test: all
$(TESTRUNNER) $(TESTOPTS)

# Run the full test suite twice - once without .pyc files, and once with.
Expand All @@ -1673,7 +1673,7 @@ test: @DEF_MAKE_RULE@ platform
# the bytecode read from a .pyc file had the bug, sometimes the directly
# generated bytecode. This is sometimes a very shy bug needing a lot of
# sample data.
testall: @DEF_MAKE_RULE@ platform
testall: all
-find $(srcdir)/Lib -name '*.py[co]' -print | xargs rm -f
$(TESTPYTHON) -E $(srcdir)/Lib/compileall.py
-find $(srcdir)/Lib -name '*.py[co]' -print | xargs rm -f
Expand All @@ -1682,7 +1682,7 @@ testall: @DEF_MAKE_RULE@ platform

# Run the test suite for both architectures in a Universal build on OSX.
# Must be run on an Intel box.
testuniversal: @DEF_MAKE_RULE@ platform
testuniversal: all
@if [ `arch` != 'i386' ]; then \
echo "This can only be used on OSX/i386" ;\
exit 1 ;\
Expand All @@ -1693,7 +1693,7 @@ testuniversal: @DEF_MAKE_RULE@ platform

# Like testall, but with only one pass and without multiple processes.
# Run an optional script to include information about the build environment.
buildbottest: all platform
buildbottest: all
-@if which pybuildbot.identify >/dev/null 2>&1; then \
pybuildbot.identify "CC='$(CC)'" "CXX='$(CXX)'"; \
fi
Expand All @@ -1708,7 +1708,7 @@ QUICKTESTOPTS= $(TESTOPTS) -x test_subprocess test_io test_lib2to3 \
test_multiprocessing_forkserver \
test_mailbox test_nntplib test_socket test_poll \
test_select test_zipfile test_concurrent_futures
quicktest: @DEF_MAKE_RULE@ platform
quicktest: all
$(TESTRUNNER) $(QUICKTESTOPTS)

# SSL tests
Expand All @@ -1719,6 +1719,10 @@ multisslcompile: all
multissltest: all
$(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/ssl/multissltests.py

# All install targets use the "all" target as synchronization point to
# prevent race conditions with PGO builds. PGO builds use recursive make,
# which can lead to two parallel `./python setup.py build` processes that
# step on each others toes.
install: @FRAMEWORKINSTALLFIRST@ commoninstall bininstall maninstall @FRAMEWORKINSTALLLAST@
if test "x$(ENSUREPIP)" != "xno" ; then \
case $(ENSUREPIP) in \
Expand Down Expand Up @@ -1747,7 +1751,7 @@ commoninstall: check-clean-src @FRAMEWORKALTINSTALLFIRST@ \
# Install shared libraries enabled by Setup
DESTDIRS= $(exec_prefix) $(LIBDIR) $(BINLIBDEST) $(DESTSHARED)

oldsharedinstall: $(DESTSHARED) $(SHAREDMODS)
oldsharedinstall: $(DESTSHARED) all
@for i in X $(SHAREDMODS); do \
if test $$i != X; then \
echo $(INSTALL_SHARED) $$i $(DESTSHARED)/`basename $$i`; \
Expand Down Expand Up @@ -2153,7 +2157,7 @@ LIBPL= @LIBPL@
# pkgconfig directory
LIBPC= $(LIBDIR)/pkgconfig

libainstall: @DEF_MAKE_RULE@ python-config
libainstall: all python-config
@for i in $(LIBDIR) $(LIBPL) $(LIBPC) $(BINDIR); \
do \
if test ! -d $(DESTDIR)$$i; then \
Expand Down Expand Up @@ -2207,7 +2211,7 @@ libainstall: @DEF_MAKE_RULE@ python-config

# Install the dynamically loadable modules
# This goes into $(exec_prefix)
sharedinstall: sharedmods
sharedinstall: all
$(RUNSHARED) $(PYTHON_FOR_BUILD) $(srcdir)/setup.py install \
--prefix=$(prefix) \
--install-scripts=$(BINDIR) \
Expand Down Expand Up @@ -2437,7 +2441,7 @@ distclean: clobber
-exec rm -f {} ';'

# Check that all symbols exported by libpython start with "Py" or "_Py"
smelly: @DEF_MAKE_RULE@
smelly: all
$(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/scripts/smelly.py

# Find files with funny names
Expand Down Expand Up @@ -2472,7 +2476,7 @@ funny:
-o -print

# Perform some verification checks on any modified files.
patchcheck: @DEF_MAKE_RULE@
patchcheck: all
$(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/scripts/patchcheck.py

check-limited-abi: all
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Address race condition in ``Makefile`` when installing a PGO build. All
``test`` and ``install`` targets now depend on ``all`` target.

0 comments on commit 243ed54

Please sign in to comment.