homepage

The attached patch fixes issues with Python's Makefile, which manifest when doing parallel builds. The Makefile invoked "make" recursively for some targets. This caused some files (which were depended upon by multiple targets) to be built by both the original "make" and the sub-"make".
Besides duplicate work, this caused failed builds with non-threadsafe compilers.
The proposed patch removes recursive calls to "make", and instead builds all targets in the same "make" process.

Martin, Matthias, do you think this would break any legitimate use?

New changeset c2a53aa27cad by Antoine Pitrou in branch 'default':
Issue #22359: Remove incorrect uses of recursive make. Patch by Jonas Wagner.
https://hg.python.org/cpython/rev/c2a53aa27cad 

Ok, I've pushed the patch. Let's see if anyone complains.

Sorry, I'm complaining. Cross builds broke. Please see issue22809.

reopening, breaks cross builds. I'll have a look

We've (FreeBSD) seen and received several random and intermittent reports of failures during this stage of the build for all versions of Python, resulting in errors such as, among others:
Parser/pgen.o: file not recognized: File truncated
*** [Parser/pgen] Error code 1
pgenmain.c:(.text+0x258): undefined reference to `_Py_pgen'
*** [Parser/pgen] Error code 1
1 error
*** [Include/graminit.h] Error code 2
See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=200622
Given the existing patch breaks cross-builds, is there an alternative or safer way to make this section of the build more robust in order to make progress?

FWIW two times recently I saw strange intermittent build errors. I use "make -j2" on Linux. The one time that I investigated was a Python module (math or cmath perhaps?) failing to build because some dependency was zero bytes, and looking through the build log, I saw duplicate compiler commands lines printed out one after the other to build that dependency.

koobs: It sounds like applying the fix for this bug to the Python 2 branch might satisfy you, right? It only seems to have been applied to 3.5+ so far.
Matt: It sounds like you are trying to pre-compile $(PGEN) for the host, and then rely on it not being rebuilt. Maybe there is a better way, like changing the $(PGEN) command line to use a $(HOST_CC) or something. (I’m not familiar with the build system; certainly not when cross compiling.) But you could also try and identify the dependency that is causing $(PGEN) to be rebuilt, for example with Gnu Make:
$ make --debug=b Parser/pgen
Updating goal targets....
 Prerequisite `Include/Python.h' is newer than target `Parser/acceler.o'.
 Must remake target `Parser/acceler.o'.
[. . .]
 Prerequisite `Parser/acceler.o' is newer than target `Parser/pgen'.
[. . .]
Must remake target `Parser/pgen'.

martin: yes, if the incorrect usage of recursive make is in the 2.7 and 3.4 branches and the fix is relevant to the issues I reported. Of course all subsequent changes (fixing cross-builds) should be applied there too.

My math and cmath issue seems to be completely separate. I opened Issue 24421 for that, so sorry for the noise here :)
I think the patch here should be applied (or adapted if necessary) to the 3.4 and 2.7 branches. If it breaks cross compilation, then the cross compilation technique needs improving instead; perhaps see Issue 22625 for that.

Initial commit to default is done. Backport to 2.7 & 3.4 branches remains to do

Add 3.5 to list of versions merging required for

One of the FreeBSD buildbots (koobs-freebsd9) just failed on a 2.7 build due to this issue.
Full log attached, as the bug is intermittent and non-deterministic (though appears more likely during high build host loads).
This issue no longer 'needs patch' as it has was committed to default (3.5)
This really needs to be backported to the rest of supported bugfix branches as soon as possible (along with the cross-build regression outlined in issue 22809 (a duplicate of 22625)

Since finishing committing this to other branches seems to be controversial, it might help resolving the cross compilation problem first.

This patch improves changeset c2a53aa27cad by allowing cross-compilation:
The following checks have been made:
 * successfull cross-compilation of python3.4 after retrofiting changeset
 c2a53aa27cad to python3.4 since I don't have a working cross-compilation
 setup for python3.6 at the moment
 * successfull native compilation of python3.6 (default branch)
 * Python/graminit.c is re-generated after touching Parser/pgenmain.c on a
 native build

Thanks for you work Xavier. Making it conditional on cross compilation is another approach I hadn’t thought of. But I am a bit worried at the new makefile syntax [ifeq directive and $(findstring) function]. I suspect it is Gnu specific, and that Python tries to support other versions of Make as well. Koobs: does Free BSD accept this makefile syntax?
Much as I hate the configure.ac stuff, I guess it could be used to enable and disable conditional stuff in the makefile, so that might be another option.
Whatever the end solution is, I think we need a big comment in the makefile pointing out the quirks and different people’s interests:
* Reliable way to update generated files when necessary
* Shared prerequisites unsafe with recursive concurrent $(MAKE)
* Reasons to avoid regenerating files (not out of date, cross compilation, less build steps and prerequisites; see python-dev)
* Building with read-only sources (see revision 67ed8a6905c3, then r87558)

Here is a patch implementing my idea to make file reneration a separate step, not automatically run during the normal build process. But it is sounding like other people don’t like this idea, so Xavier’s approach might be less controversial even if it is more complicated.

> But I am a bit worried at the new makefile syntax [ifeq directive and $(findstring) function]. I suspect it is Gnu specific, and that Python tries to support other versions of Make as well.
Yes, they are both GNU extensions :(
To avoid modifying configure.ac since the information is already available, the recipes could be modified with the following conditional:
 cross_compiling=$$(echo $(PYTHON_FOR_BUILD) | sed "s/.*_PYTHON_HOST_PLATFORM.*/yes/"); \
 if [ "$$cross_compiling" = "yes" ]; then \
 touch $@;
 else \
 # The original recipes.
 fi
So the built binaries $(PGEN) and Programs/_freeze_importlib would be dummy empty files, but that would not prevent Programs/_freeze_importlib.o, $(LIBRARY_OBJS_OMIT_FROZEN) and $(PGENOBJS) to be needlessly cross-compiled.

Here is a patch that does not use GNU make conditionals.
Patch tested by running the test suite both natively and after a
cross-compilation.

I think this may be a viable solution Xavier. See review suggestion about building from read-only source though.

Martin, thanks for the review and the suggestions.
The attached patch makes the following changes:
* do not modify the source files so as not to break builds made from
 read-only build sources (as per issue 15819)
* the cross-build copies the graminit.[ch] files to the build directory
* fix tab indentation
Patch tested by checking that the graminit.[ch] time stamps do not
change in the source directory afer a build and after a cross-build,
and by running the test suite both natively and on android emulator.

Looks pretty good. One more question: Why do you make the linking of _freeze_importlib conditional, but always build $(PGEN)?

> Why do you make the linking of _freeze_importlib conditional, but always build $(PGEN)?
Yes, this is not consistent. The cross-build is correct when both are linked or when both are not linked. And $(PGENOBJS) are cross-compiled in both cases.
Should both programs not be created ?

I see. I guess it would keep the makefile simpler if we cross-compiled both programs, and never used them.

For the record, to fix this in 2.7 will involve backporting revision c2a53aa27cad, plus Xavier’s patch (except the freeze_importlib parts which is not relevant to 2.7).

_freeze_importlib and pgen are cross-built in this patch. Patch tested with a run of the testsuite after a cross-build.

New changeset 66e40df31fac by Martin Panter in branch '3.5':
Issue #22359: Disable running cross-compiled _freeze_importlib and pgen
https://hg.python.org/cpython/rev/66e40df31fac
New changeset c36b9ef8ff39 by Martin Panter in branch 'default':
Issue #22359: Merge cross-compilation fix from 3.5
https://hg.python.org/cpython/rev/c36b9ef8ff39
New changeset 0f7a299c6d50 by Martin Panter in branch '2.7':
Issue #22359: Avoid recursive $(MAKE); disable running cross-compiled pgen
https://hg.python.org/cpython/rev/0f7a299c6d50 

Thanks for your help with this Xavier.