classification
Title: Remove incorrect uses of recursive make
Type: behavior Stage: resolved
Components: Build Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Sjlver, WanderingLogic, brett.cannon, doko, koobs, loewis, martin.panter, pitrou, python-dev, xdegaye
Priority: normal Keywords: patch

Created on 2014-09-08 08:44 by Sjlver, last changed 2016-04-24 03:41 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
makefile_parallel.patch Sjlver, 2014-09-08 08:44 Patch: Remove incorrect uses of recursive make review
koobs-freebsd9.python-2.7-build.912.txt koobs, 2015-07-30 05:51
cross-chgeset-c2a53aa27cad.patch xdegaye, 2016-03-14 19:24 review
separate-regen.patch martin.panter, 2016-03-15 05:18 Separate "make graminit" etc targets review
cross-chgeset-c2a53aa27cad_2.patch xdegaye, 2016-04-19 08:28 review
crossbuild-sources-readonly.patch xdegaye, 2016-04-20 09:59 Do not modify build source files review
crossbuild-sources-readonly_2.patch xdegaye, 2016-04-21 12:11 _freeze_importlib and pgen are cross-built review
Messages (29)
msg226563 - (view) Author: Jonas Wagner (Sjlver) * Date: 2014-09-08 08:44
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.
msg226797 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-09-11 19:19
Martin, Matthias, do you think this would break any legitimate use?
msg227186 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-09-20 22:24
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
msg227187 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-09-20 22:24
Ok, I've pushed the patch. Let's see if anyone complains.
msg230766 - (view) Author: Matt Frank (WanderingLogic) * Date: 2014-11-06 21:13
Sorry, I'm complaining.  Cross builds broke.  Please see issue22809.
msg238199 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2015-03-16 13:39
reopening, breaks cross builds. I'll have a look
msg245110 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2015-06-10 03:57
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?
msg245112 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-10 04:16
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.
msg245117 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-10 06:26
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'.
msg245119 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2015-06-10 07:38
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.
msg245122 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-10 08:30
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.
msg245365 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2015-06-15 05:57
Initial commit to default is done. Backport to 2.7 & 3.4 branches remains to do
msg245366 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2015-06-15 05:59
Add 3.5 to list of versions merging required for
msg247650 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2015-07-30 05:51
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)
msg247651 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-07-30 06:16
Since finishing committing this to other branches seems to be controversial, it might help resolving the cross compilation problem first.
msg261775 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-03-14 19:24
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
msg261786 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-14 22:27
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)
msg261799 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-15 05:18
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.
msg261824 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-03-15 19:49
> 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.
msg263721 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-04-19 08:28
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.
msg263793 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-20 05:06
I think this may be a viable solution Xavier. See review suggestion about building from read-only source though.
msg263815 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-04-20 09:59
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.
msg263875 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-21 04:26
Looks pretty good. One more question: Why do you make the linking of _freeze_importlib conditional, but always build $(PGEN)?
msg263891 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-04-21 07:40
> 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 ?
msg263898 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-21 08:56
I see. I guess it would keep the makefile simpler if we cross-compiled both programs, and never used them.
msg263902 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-21 09:06
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).
msg263915 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2016-04-21 12:11
_freeze_importlib and pgen are cross-built in this patch. Patch tested with a run of the testsuite after a cross-build.
msg264035 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-04-23 01:51
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
msg264089 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-24 03:41
Thanks for your help with this Xavier.
History
Date User Action Args
2016-04-24 03:41:09martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg264089

stage: patch review -> resolved
2016-04-23 01:51:34python-devsetmessages: + msg264035
2016-04-21 12:11:22xdegayesetfiles: + crossbuild-sources-readonly_2.patch

messages: + msg263915
2016-04-21 09:06:36martin.pantersetmessages: + msg263902
2016-04-21 08:56:50martin.pantersetmessages: + msg263898
2016-04-21 07:40:46xdegayesetmessages: + msg263891
2016-04-21 04:26:26martin.pantersetmessages: + msg263875
2016-04-20 09:59:54xdegayesetfiles: + crossbuild-sources-readonly.patch

messages: + msg263815
2016-04-20 05:10:22martin.panterlinkissue22625 superseder
2016-04-20 05:07:32martin.pantersetstatus: languishing -> open
dependencies: - When cross-compiling, don’t try to execute binaries
2016-04-20 05:06:37martin.pantersetmessages: + msg263793
2016-04-19 08:28:29xdegayesetfiles: + cross-chgeset-c2a53aa27cad_2.patch

messages: + msg263721
2016-03-15 19:49:21xdegayesetmessages: + msg261824
2016-03-15 05:18:42martin.pantersetfiles: + separate-regen.patch

messages: + msg261799
2016-03-14 22:27:50martin.pantersetkeywords: - easy

stage: patch review
messages: + msg261786
versions: - Python 3.4
2016-03-14 19:24:42xdegayesetfiles: + cross-chgeset-c2a53aa27cad.patch
nosy: + xdegaye
messages: + msg261775

2015-07-30 06:21:53martin.pantersetstage: commit review -> (no value)
2015-07-30 06:16:27martin.pantersetdependencies: + When cross-compiling, don’t try to execute binaries
messages: + msg247651
stage: commit review
2015-07-30 05:51:30koobssetstatus: open -> languishing
files: + koobs-freebsd9.python-2.7-build.912.txt
messages: + msg247650

stage: needs patch -> (no value)
2015-06-15 05:59:22koobssetmessages: + msg245366
versions: + Python 3.6
2015-06-15 05:57:25koobssetkeywords: + easy

messages: + msg245365
components: - Cross-Build
2015-06-10 08:30:19martin.pantersetmessages: + msg245122
2015-06-10 07:38:28koobssetmessages: + msg245119
2015-06-10 06:26:02martin.pantersetmessages: + msg245117
2015-06-10 04:16:05martin.pantersetnosy: + martin.panter
messages: + msg245112
2015-06-10 03:57:19koobssetmessages: + msg245110
versions: + Python 2.7, Python 3.4
2015-06-10 03:51:49koobssetnosy: + koobs
2015-06-04 05:06:26koobssetcomponents: + Cross-Build
2015-06-04 05:06:05koobssetstage: resolved -> needs patch
2015-03-16 13:39:13dokosetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg238199
2014-11-06 21:13:42WanderingLogicsetnosy: + WanderingLogic
messages: + msg230766
2014-09-20 22:24:41pitrousetstatus: open -> closed
resolution: fixed
messages: + msg227187

stage: patch review -> resolved
2014-09-20 22:24:16python-devsetnosy: + python-dev
messages: + msg227186
2014-09-11 19:19:38pitrousetnosy: + pitrou, loewis
messages: + msg226797

type: compile error -> behavior
stage: patch review
2014-09-09 11:42:20Arfreversetnosy: + Arfrever
2014-09-08 14:23:05brett.cannonsetnosy: + brett.cannon
2014-09-08 12:03:45berker.peksagsetnosy: + doko
2014-09-08 08:44:29Sjlvercreate