Re: [Python-Dev] Bug in build system for cross-platform builds
On 13 March 2016 at 01:13, Russell Keith-Magee <russell@keith-magee.com> wrote:
Thanks very much for pointing that out. This has helped me understand a lot more things. Only now do I realize that the four files generated by pgen and _freeze_importlib are actually already committed into the Mercurial repository: Include/graminit.h Python/graminit.c Python/importlib.h Python/importlib_external.h A question for other Python developers: Why are these generated files stored in the repository? The graminit ones seem to have been there since forever (1990). It seems the importlib ones were there due to a bootstrapping problem, but now that is solved. Antoine <https://bugs.python.org/issue14928#msg163048> said he kept them in the repository on purpose, but I want to know why. If we ignore the cross compiling use case, would there be any other consequences of removing these generated files from the repository? E.g. would it affect the Windows build process? I have two possible solutions in mind: either remove the generated files from the repository and always build them, or keep them but do not automatically regenerate them every build. Since they are generated files, not source files, I would prefer to remove them, but I want to know the consequences first.
After my realization about the generated files, I think I can solve this one. Before the changes you identified, the build process probably thought the generated files were up to date, so it didn't need to cross-compile pgen or _freeze_importlib. If the generated file timestamps were out of date (e.g. depending on the order they are checked out or extracted), the first native build stage would have fixed them up.
On Sun, Mar 13, 2016 at 7:41 PM Martin Panter <vadmium+py@gmail.com> wrote:
They should not be regenerated every build, if they are, that seems like a bug in the makefile to me (or else the timestamp checks that make does vs how your code checkout happened). Having them checked in is convenient for cross builds as it is one less thing that needs a build-host-arch build. my 2 cents, -gps
On 14 March 2016 at 13:04, Gregory P. Smith <greg@krypto.org> wrote:
It's also two less things to go wrong for folks just wanting to work on the 99.9% of CPython that is neither the Grammar file nor importlib._bootstrap. I'm trying to remember the problem I was solving in making freezeimportlib.o explicitly depend on the Makefile (while cursing past me for not writing it down in the commit message), and I think the issue was that it wasn't correctly picking up changes to the builtin module list. If that's right, then fixing the dependency to be on "$(srcdir)/Makefile.pre.in" instead of the generated Makefile should keep it from getting confused in the cross-compilation scenario, and also restore the behaviour of using the checked in copy rather than rebuilding it just because you ran "./configure". Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Mon, 14 Mar 2016 03:04:08 -0000, "Gregory P. Smith" <greg@krypto.org> wrote:
The repo-timestamp problem is addressed by the 'make touch' target. And yes, checking in these platform-independent artifacts is very intentional: less to build, fewer external dependencies in the build process...you don't need to *have* python to *build* python, which you would have to if they were not checked in. --David
On 03/14/2016 02:26 PM, R. David Murray wrote:
Changeset c2a53aa27cad [1] was commited in issue 22359 [2] to remove incorrect uses of recursive make. The changeset added executable binaries as prerequisites to the existing rules (Python/importlib.h and $(GRAMMAR_H)). This broke cross-compilation: * the executables do not exist and must be cross-compiled * then the Python/importlib.h or $(GRAMMAR_H) target recipes must be run since the prerequisites are newer * but the executables cannot run on the build system Actually the files need not be re-generated as their timestamps have been setup for that purpose with 'make touch'. So a solution to the problem introduced by this changeset when cross-compiling could be to remove the binaries as prerequisites of these rules and include the recipe of their corresponding rules, the one used to build the executable, into the recipes of the original rule. Also IMHO the Programs/_freeze_importlib.c can be used instead of Programs/_freeze_importlib.o as a prerequisite in the Python/importlib.h rule. [1] https://hg.python.org/cpython/rev/c2a53aa27cad/ [2] http://bugs.python.org/issue22359 Xavier
On 03/14/2016 05:34 PM, Xavier de Gaye wrote:
The pgen dependencies are lost when following my previous suggestion, which is wrong. I have uploaded a patch at issue 22359 that uses a conditional to change the rules, based on whether a cross-compilation is being run. Xavier
On 14 March 2016 at 13:26, R. David Murray <rdmurray@bitdance.com> wrote:
The reason the current Python 3 build regenerates some files, is because of the makefile prerequisites. For example, Include/graminit.h currently depends on Parser/pgen, which needs to be compiled for the native build host.
Okay so it sounds like the generated files (more listed in .hgtouch) have to stay. Reasons given: * Some need Python to generate them (bootstrap problem) * Relied on by Windows build system * General convenience (less build steps, less prerequisites, less things to go wrong) One more idea I am considering is to decouple the makefile rules from the main build. So to update the generated files you would have to run a separate command like “make graminit” or “make frozen”. The normal build would never regenerate them; although perhaps it could still result in an error or warning if they appear out of date.
On 15 March 2016 at 10:49, Martin Panter <vadmium+py@gmail.com> wrote:
Some of them used to work that way and it's an incredible PITA when you actually *are* working on one of the affected bits of the interpreter - working on those parts is rare, so if there are special cases to remember, you can pretty much guarantee we'll have forgotten them by the time we work on that piece again. However, it would be worth reviewing the explicit dependencies on "Makefile" and see whether they could be replaced by dependencies on Makefile.pre.in instead. I'm confident that will work just fine for the importlib bootstrapping, and I suspect it will work for the other pregenerated-and-checked-in files as well. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 15 March 2016 at 04:32, Nick Coghlan <ncoghlan@gmail.com> wrote:
Perhaps if we wrapped them all up in a common “make regenerate” target, so it is only one special case to remember? Maybe you could include other stuff like “make clinic” in that as well. Or you could include the special commands in the warning messages.
The problem is not the reference to Makefile. The graminit files do not depend on Makefile. The bigger problem is that the checked-in files depend on compiled programs. This is a summary of the current rules for importlib: _freeze_importlib.o: _freeze_importlib.c Makefile _freeze_importlib: _freeze_importlib.o [. . .] $(LINKCC) [. . .] importlib_external.h: _bootstrap_external.py _freeze_importlib _freeze_importlib _bootstrap_external.py importlib_external.h importlib.h: _bootstrap.py _freeze_importlib _freeze_importlib _bootstrap.py importlib.h So importlib.h depends on the _freeze_importlib compiled program (and only indirectly on Makefile). The makefile says we have to compile _freeze_importlib before checking if importlib.h is up to date. Gnu Make has order-only prerequisites <https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html>, which it seems we could abuse to do most of what we want. But (1) I’m not sure if we can restrict ourselves to Gnu Make, and (2) it is a horrible hack and would always compile _freeze_importlib even if it is never run.
On 15 March 2016 at 15:15, Martin Panter <vadmium+py@gmail.com> wrote:
Ah, I understand now - the fundamental problem is with a checked in file depending on a non-checked-in file, so if you clean out all the native build artifacts when cross-compiling, the makefile will attempt to create target versions of all the helper utilities (pgen, _freeze_importlib, argument clinic, etc). Would it help to have a "make bootstrap" target that touched all the checked in generated files with build dependencies on non-checked-in files, but only after first touching the expected locations of the built binaries they depend on? Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 15 March 2016 at 08:04, Nick Coghlan <ncoghlan@gmail.com> wrote:
That sounds similar to “make touch”, with a couple differences. One trouble I forsee is the conflict with shared prerequisites. E.g. “make bootstrap” would have to create some dummy object files as prerequisites of the pgen program, but we would first have build others e.g. Parser/acceler.o properly for the main Python library. It all feels way too complicated to me. The Python build system is complicated enough as it is. Maybe it is simplest to just add something in the spirit of Xavier’s suggested patch. This would mean that we keep the generated files checked in (to help with Windows and cross compiled builds), we keep the current rules that force normal makefile builds to blindly regenerate the files, but we add some flag or configure.ac check to disable this regeneration if desired.
Simply removing them would break the Windows build, and it may not be easily fixable as the dependency system we use doesn't allow building a project twice. Currently we fail the build if a generated file changes and then the user has to trigger a second build with the new file, but typically they're fine and the first build succeeds. Cheers, Steve Top-posted from my Windows Phone -----Original Message----- From: "Martin Panter" <vadmium+py@gmail.com> Sent: 3/13/2016 19:43 To: "Russell Keith-Magee" <russell@keith-magee.com>; "python-dev" <python-dev@python.org> Cc: "antoine@python.org" <antoine@python.org> Subject: Re: [Python-Dev] Bug in build system for cross-platform builds On 13 March 2016 at 01:13, Russell Keith-Magee <russell@keith-magee.com> wrote:
Thanks very much for pointing that out. This has helped me understand a lot more things. Only now do I realize that the four files generated by pgen and _freeze_importlib are actually already committed into the Mercurial repository: Include/graminit.h Python/graminit.c Python/importlib.h Python/importlib_external.h A question for other Python developers: Why are these generated files stored in the repository? The graminit ones seem to have been there since forever (1990). It seems the importlib ones were there due to a bootstrapping problem, but now that is solved. Antoine <https://bugs.python.org/issue14928#msg163048> said he kept them in the repository on purpose, but I want to know why. If we ignore the cross compiling use case, would there be any other consequences of removing these generated files from the repository? E.g. would it affect the Windows build process? I have two possible solutions in mind: either remove the generated files from the repository and always build them, or keep them but do not automatically regenerate them every build. Since they are generated files, not source files, I would prefer to remove them, but I want to know the consequences first.
After my realization about the generated files, I think I can solve this one. Before the changes you identified, the build process probably thought the generated files were up to date, so it didn't need to cross-compile pgen or _freeze_importlib. If the generated file timestamps were out of date (e.g. depending on the order they are checked out or extracted), the first native build stage would have fixed them up. _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/steve.dower%40python.org
On Sun, Mar 13, 2016 at 7:41 PM Martin Panter <vadmium+py@gmail.com> wrote:
They should not be regenerated every build, if they are, that seems like a bug in the makefile to me (or else the timestamp checks that make does vs how your code checkout happened). Having them checked in is convenient for cross builds as it is one less thing that needs a build-host-arch build. my 2 cents, -gps
On 14 March 2016 at 13:04, Gregory P. Smith <greg@krypto.org> wrote:
It's also two less things to go wrong for folks just wanting to work on the 99.9% of CPython that is neither the Grammar file nor importlib._bootstrap. I'm trying to remember the problem I was solving in making freezeimportlib.o explicitly depend on the Makefile (while cursing past me for not writing it down in the commit message), and I think the issue was that it wasn't correctly picking up changes to the builtin module list. If that's right, then fixing the dependency to be on "$(srcdir)/Makefile.pre.in" instead of the generated Makefile should keep it from getting confused in the cross-compilation scenario, and also restore the behaviour of using the checked in copy rather than rebuilding it just because you ran "./configure". Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Mon, 14 Mar 2016 03:04:08 -0000, "Gregory P. Smith" <greg@krypto.org> wrote:
The repo-timestamp problem is addressed by the 'make touch' target. And yes, checking in these platform-independent artifacts is very intentional: less to build, fewer external dependencies in the build process...you don't need to *have* python to *build* python, which you would have to if they were not checked in. --David
On 03/14/2016 02:26 PM, R. David Murray wrote:
Changeset c2a53aa27cad [1] was commited in issue 22359 [2] to remove incorrect uses of recursive make. The changeset added executable binaries as prerequisites to the existing rules (Python/importlib.h and $(GRAMMAR_H)). This broke cross-compilation: * the executables do not exist and must be cross-compiled * then the Python/importlib.h or $(GRAMMAR_H) target recipes must be run since the prerequisites are newer * but the executables cannot run on the build system Actually the files need not be re-generated as their timestamps have been setup for that purpose with 'make touch'. So a solution to the problem introduced by this changeset when cross-compiling could be to remove the binaries as prerequisites of these rules and include the recipe of their corresponding rules, the one used to build the executable, into the recipes of the original rule. Also IMHO the Programs/_freeze_importlib.c can be used instead of Programs/_freeze_importlib.o as a prerequisite in the Python/importlib.h rule. [1] https://hg.python.org/cpython/rev/c2a53aa27cad/ [2] http://bugs.python.org/issue22359 Xavier
On 03/14/2016 05:34 PM, Xavier de Gaye wrote:
The pgen dependencies are lost when following my previous suggestion, which is wrong. I have uploaded a patch at issue 22359 that uses a conditional to change the rules, based on whether a cross-compilation is being run. Xavier
On 14 March 2016 at 13:26, R. David Murray <rdmurray@bitdance.com> wrote:
The reason the current Python 3 build regenerates some files, is because of the makefile prerequisites. For example, Include/graminit.h currently depends on Parser/pgen, which needs to be compiled for the native build host.
Okay so it sounds like the generated files (more listed in .hgtouch) have to stay. Reasons given: * Some need Python to generate them (bootstrap problem) * Relied on by Windows build system * General convenience (less build steps, less prerequisites, less things to go wrong) One more idea I am considering is to decouple the makefile rules from the main build. So to update the generated files you would have to run a separate command like “make graminit” or “make frozen”. The normal build would never regenerate them; although perhaps it could still result in an error or warning if they appear out of date.
On 15 March 2016 at 10:49, Martin Panter <vadmium+py@gmail.com> wrote:
Some of them used to work that way and it's an incredible PITA when you actually *are* working on one of the affected bits of the interpreter - working on those parts is rare, so if there are special cases to remember, you can pretty much guarantee we'll have forgotten them by the time we work on that piece again. However, it would be worth reviewing the explicit dependencies on "Makefile" and see whether they could be replaced by dependencies on Makefile.pre.in instead. I'm confident that will work just fine for the importlib bootstrapping, and I suspect it will work for the other pregenerated-and-checked-in files as well. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 15 March 2016 at 04:32, Nick Coghlan <ncoghlan@gmail.com> wrote:
Perhaps if we wrapped them all up in a common “make regenerate” target, so it is only one special case to remember? Maybe you could include other stuff like “make clinic” in that as well. Or you could include the special commands in the warning messages.
The problem is not the reference to Makefile. The graminit files do not depend on Makefile. The bigger problem is that the checked-in files depend on compiled programs. This is a summary of the current rules for importlib: _freeze_importlib.o: _freeze_importlib.c Makefile _freeze_importlib: _freeze_importlib.o [. . .] $(LINKCC) [. . .] importlib_external.h: _bootstrap_external.py _freeze_importlib _freeze_importlib _bootstrap_external.py importlib_external.h importlib.h: _bootstrap.py _freeze_importlib _freeze_importlib _bootstrap.py importlib.h So importlib.h depends on the _freeze_importlib compiled program (and only indirectly on Makefile). The makefile says we have to compile _freeze_importlib before checking if importlib.h is up to date. Gnu Make has order-only prerequisites <https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html>, which it seems we could abuse to do most of what we want. But (1) I’m not sure if we can restrict ourselves to Gnu Make, and (2) it is a horrible hack and would always compile _freeze_importlib even if it is never run.
On 15 March 2016 at 15:15, Martin Panter <vadmium+py@gmail.com> wrote:
Ah, I understand now - the fundamental problem is with a checked in file depending on a non-checked-in file, so if you clean out all the native build artifacts when cross-compiling, the makefile will attempt to create target versions of all the helper utilities (pgen, _freeze_importlib, argument clinic, etc). Would it help to have a "make bootstrap" target that touched all the checked in generated files with build dependencies on non-checked-in files, but only after first touching the expected locations of the built binaries they depend on? Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 15 March 2016 at 08:04, Nick Coghlan <ncoghlan@gmail.com> wrote:
That sounds similar to “make touch”, with a couple differences. One trouble I forsee is the conflict with shared prerequisites. E.g. “make bootstrap” would have to create some dummy object files as prerequisites of the pgen program, but we would first have build others e.g. Parser/acceler.o properly for the main Python library. It all feels way too complicated to me. The Python build system is complicated enough as it is. Maybe it is simplest to just add something in the spirit of Xavier’s suggested patch. This would mean that we keep the generated files checked in (to help with Windows and cross compiled builds), we keep the current rules that force normal makefile builds to blindly regenerate the files, but we add some flag or configure.ac check to disable this regeneration if desired.
Simply removing them would break the Windows build, and it may not be easily fixable as the dependency system we use doesn't allow building a project twice. Currently we fail the build if a generated file changes and then the user has to trigger a second build with the new file, but typically they're fine and the first build succeeds. Cheers, Steve Top-posted from my Windows Phone -----Original Message----- From: "Martin Panter" <vadmium+py@gmail.com> Sent: 3/13/2016 19:43 To: "Russell Keith-Magee" <russell@keith-magee.com>; "python-dev" <python-dev@python.org> Cc: "antoine@python.org" <antoine@python.org> Subject: Re: [Python-Dev] Bug in build system for cross-platform builds On 13 March 2016 at 01:13, Russell Keith-Magee <russell@keith-magee.com> wrote:
Thanks very much for pointing that out. This has helped me understand a lot more things. Only now do I realize that the four files generated by pgen and _freeze_importlib are actually already committed into the Mercurial repository: Include/graminit.h Python/graminit.c Python/importlib.h Python/importlib_external.h A question for other Python developers: Why are these generated files stored in the repository? The graminit ones seem to have been there since forever (1990). It seems the importlib ones were there due to a bootstrapping problem, but now that is solved. Antoine <https://bugs.python.org/issue14928#msg163048> said he kept them in the repository on purpose, but I want to know why. If we ignore the cross compiling use case, would there be any other consequences of removing these generated files from the repository? E.g. would it affect the Windows build process? I have two possible solutions in mind: either remove the generated files from the repository and always build them, or keep them but do not automatically regenerate them every build. Since they are generated files, not source files, I would prefer to remove them, but I want to know the consequences first.
After my realization about the generated files, I think I can solve this one. Before the changes you identified, the build process probably thought the generated files were up to date, so it didn't need to cross-compile pgen or _freeze_importlib. If the generated file timestamps were out of date (e.g. depending on the order they are checked out or extracted), the first native build stage would have fixed them up. _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/steve.dower%40python.org
participants (6)
-
Gregory P. Smith -
Martin Panter -
Nick Coghlan -
R. David Murray -
Steve Dower -
Xavier de Gaye