classification
Title: Fix importlib bootstrapping issues
Type: behavior Stage: resolved
Components: Build, Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, benjamin.peterson, brett.cannon, eric.smith, eric.snow, georg.brandl, loewis, meador.inge, ncoghlan, pitrou, python-dev
Priority: release blocker Keywords: patch

Created on 2012-05-27 17:54 by pitrou, last changed 2012-06-19 21:19 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
cfreeze.patch pitrou, 2012-06-16 20:15 review
cfreeze2.patch pitrou, 2012-06-17 16:07 review
Messages (13)
msg161714 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-27 17:54
See http://mail.python.org/pipermail/python-dev/2012-May/119703.html

Two competing proposals:

- Benjamin: "Perhaps freeze_importlib.py could be rewritten in C, so
importlib could be recompiled when the compiler changes?"

- Martin: "Or we support bootstrapping from the source file, e.g. with an
environment variable BOOTSTRAP_PY which points to the _bootstrap.py
source"
msg161725 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-05-27 20:42
I like MvL's approach if we can make it work - then we can set up the importlib.h regeneration to automatically bootstrap itself from the source file and avoid having to add another binary to the build process.
msg161752 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-05-28 03:02
There are some additional challenges potentially posed by suggestions like http://bugs.python.org/issue10399, which would allow the compiler itself to use Python extensions.

However, those could be overcome by requiring that the compiler support running with any such extensions disabled.

Two more possible APIs:
  A -X option: "-Xbootstrap=Lib/importlib/_bootstrap.py"
  Allow frozen modules to be frozen as *source* rather than as a PYC file, then freeze importlib._bootstrap as source rather than as bytecode.
msg162987 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-16 20:15
Here is a patch for the "separate C executable" approach. There are a couple of small complications in order to avoid any circular dependency (either at the Makefile level, or at the _frozen_importlib / Py_Initialize level).

The proposed "-X" or BOOTSTRAP_PY approaches would be difficult, since there doesn't appear to be an easy way to generate a given file (the Python interpreter) twice in a Makefile dependency chain: the first time using the stale importlib, the second with the fresh importlib.
msg163009 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-06-17 03:50
Nice patch Antoine!  The method of partitioning the object sets into frozen and
non-frozen is nice.

> The proposed "-X" or BOOTSTRAP_PY approaches would be difficult, since there
> doesn't appear to be an easy way to generate a given file (the Python
> interpreter) twice in a Makefile dependency chain: the first time using the
> stale importlib, the second with the fresh importlib.

I experimented with the env var approach and came to the exact same conclusion
-- you would have to build the interpreter twice.

This patch looks good to me (I did have to rebase it against trunk, though;
it doesn't apply cleanly right now).  I did some basic regression testing and ran
it through the original scenario from issue12370 that got us thinking this problem.
I saw no issues.  Nice work.
msg163011 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-17 04:02
Looks like a solid improvement to me.

One minor naming quibble is that "FROZENLESS_LIBRARY_OBJS" hurts my brain - would you mind switching it to something like "LIBRARY_OBJS_OMIT_FROZEN"?
msg163025 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-06-17 07:47
IIUC, this patch will cause importlib.h to always be built when building from source, since _freeze_importlib will be built.

Is it then the plan to drop importlib.h from version control? If so, the Windows build process would need to be adjusted as well (with a separate project that is a prerequisite for pythoncore).
msg163048 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-17 09:49
> IIUC, this patch will cause importlib.h to always be built when
> building from source, since _freeze_importlib will be built.

Yes indeed. That's not deliberate, but because of how Makefile
dependencies are specified (importlib.h needs the _freeze_importlib
executable to be rebuilt, but it should really depend on the
_freeze_importlib.c timestamp). Do you have an idea to avoid that?

> Is it then the plan to drop importlib.h from version control?

No.
msg163049 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-06-17 09:56
> Yes indeed. That's not deliberate, but because of how Makefile
> dependencies are specified (importlib.h needs the _freeze_importlib
> executable to be rebuilt, but it should really depend on the
> _freeze_importlib.c timestamp). Do you have an idea to avoid that?

Recursive make invocation may help. Before running _freeze_importlib,
add '$(MAKE) _freeze_importlib'.
msg163076 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-17 16:07
New patch incorporating Martin's and Nick's suggestions.
msg163213 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-19 20:16
Is this ready to go in before beta1?
msg163219 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-19 20:32
New changeset c3616595dada by Antoine Pitrou in branch 'default':
Issue #14928: Fix importlib bootstrap issues by using a custom executable (Modules/_freeze_importlib) to build Python/importlib.h.
http://hg.python.org/cpython/rev/c3616595dada
msg163222 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-06-19 21:19
Should be ok now.
History
Date User Action Args
2012-06-19 21:19:12pitrousetstatus: open -> closed
resolution: fixed
messages: + msg163222

stage: patch review -> resolved
2012-06-19 20:32:59python-devsetnosy: + python-dev
messages: + msg163219
2012-06-19 20:16:36georg.brandlsetnosy: + georg.brandl
messages: + msg163213
2012-06-17 16:07:07pitrousetfiles: + cfreeze2.patch

messages: + msg163076
2012-06-17 09:56:37loewissetmessages: + msg163049
2012-06-17 09:49:54pitrousetmessages: + msg163048
2012-06-17 07:47:49loewissetmessages: + msg163025
2012-06-17 04:02:09ncoghlansetmessages: + msg163011
2012-06-17 03:50:09meador.ingesetmessages: + msg163009
2012-06-16 20:15:48pitrousetfiles: + cfreeze.patch
keywords: + patch
messages: + msg162987

stage: needs patch -> patch review
2012-05-28 19:56:27meador.ingesetnosy: + meador.inge
2012-05-28 03:02:03ncoghlansetmessages: + msg161752
2012-05-28 02:05:47eric.smithsetnosy: + eric.smith
2012-05-27 20:42:58ncoghlansetnosy: + ncoghlan
messages: + msg161725
2012-05-27 17:58:56Arfreversetnosy: + Arfrever
2012-05-27 17:54:00pitroucreate