classification
Title: Move path-based bootstrap code to a separate frozen file.
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: brett.cannon, eric.snow, larry, ncoghlan, ned.deily, python-dev, raulcd
Priority: normal Keywords: patch

Created on 2015-04-11 01:29 by eric.snow, last changed 2015-05-13 05:35 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
path-based-importlib.diff eric.snow, 2015-04-11 01:29 review
path-based-importlib.diff eric.snow, 2015-04-20 03:23 review
path-based-importlib.diff eric.snow, 2015-05-02 21:57
path-based-importlib.diff eric.snow, 2015-05-03 00:05 review
Messages (18)
msg240457 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-04-11 01:29
The bootstrap code has a clear division between the core import functionality and the path-based import machinery.  The attached patch makes that division explicit by moving the latter into its own module.  The module is also frozen, necessarily.  In addition to clearly distinguishing the two parts, this division will help with some later work that I'd like to do later with an encapsulated import system abstraction.

The patch uses the name "pathy" throughout, which I'll change to something more descriptive later.  I'll also add the news entry then.
msg241592 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-04-20 03:23
Here's an updated patch, with the PEP 489 changes merged in.  Only one test isn't passing and it is due to something in the pip that is bundled into ensurepip.  I'll work on fixing that when I have some time.  I'm sure there's documentation near the bundle that explains how to update it.
msg241593 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-04-20 03:24
s/PEP 489/PEP 488/
msg242001 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-04-25 07:31
Nice, this ties directly into one of the thornier problems in the PEP 432 bootstrapping work, which needs to set up the core import system in Py_BeginInitialization, but delay setting up the rest of it until Py_EndInitialization.

Your patch achieves this by moving everything that belongs in the latter part of the operation to the "import _frozen_path_importer" step.

As far as naming goes, I'd suggest referring to the two import subsystems as "internal imports" and "external imports". The key aspect of builtin and frozen modules is that they're an inherent part of the interpreter itself - if you have an interpreter, you have all of its buitin and frozen modules natively available. By contrast, setting up the external import machinery requires that the interpreter first be configured to appropriately communicate with the host system.
msg242160 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-04-27 22:35
Glad to hear the patch is conceptually consistent with other components. :)  And the "internal"/"external" suggestion is a good one.  I'll update the patch when I have a minute.
msg242433 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-02 20:47
Here's an updated patch with "_pathy.py" changed to "_bootstrap_external.py" (and similar changes with freezing).  The patch does not include fixing the venv test (i.e. the bundled pip).

Also, I'll be adding a note to NEWS.
msg242434 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-02 20:52
As I mentioned, I'm pretty sure that the failing venv test is due to the bundled pip.  Here's the test output:

test test_venv failed -- Traceback (most recent call last):
  File "/home/esnow/projects/cpython/Lib/test/test_venv.py", line 356, in test_with_pip
    with_pip=True)
subprocess.CalledProcessError: Command '['/tmp/tmphxh1zztf/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/esnow/projects/cpython/Lib/test/test_venv.py", line 362, in test_with_pip
    self.fail(msg.format(exc, details))
AssertionError: Command '['/tmp/tmphxh1zztf/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1

**Subprocess Output**
Traceback (most recent call last):
  File "/home/esnow/projects/cpython/Lib/runpy.py", line 170, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/esnow/projects/cpython/Lib/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/esnow/projects/cpython/Lib/ensurepip/__main__.py", line 4, in <module>
    ensurepip._main()
  File "/home/esnow/projects/cpython/Lib/ensurepip/__init__.py", line 209, in _main
    default_pip=args.default_pip,
  File "/home/esnow/projects/cpython/Lib/ensurepip/__init__.py", line 116, in bootstrap
    _run_pip(args + [p[0] for p in _PROJECTS], additional_paths)
  File "/home/esnow/projects/cpython/Lib/ensurepip/__init__.py", line 40, in _run_pip
    import pip
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load
  File "<frozen importlib._bootstrap>", line 947, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 625, in _load_backward_compatible
  File "/tmp/tmp57j9vkrt/pip-6.1.1-py2.py3-none-any.whl/pip/__init__.py", line 13, in <module>
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load
  File "<frozen importlib._bootstrap>", line 947, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 625, in _load_backward_compatible
  File "/tmp/tmp57j9vkrt/pip-6.1.1-py2.py3-none-any.whl/pip/utils/__init__.py", line 22, in <module>
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load
  File "<frozen importlib._bootstrap>", line 947, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 625, in _load_backward_compatible
  File "/tmp/tmp57j9vkrt/pip-6.1.1-py2.py3-none-any.whl/pip/_vendor/pkg_resources/__init__.py", line 1712, in <module>
AttributeError: module 'importlib._bootstrap' has no attribute 'SourceFileLoader'
msg242435 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-02 21:25
Looks like setuptool's pkg_resources is directly importing importlib._bootstrap.  I've filed a bug: https://bitbucket.org/pypa/setuptools/issue/378.  In the meantime, what are our options for getting that test passing?
msg242437 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-02 21:57
Here's the correct patch.
msg242440 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-05-02 23:31
As a compatibility hack for setuptools versions with the issue, I'd suggest
making importlib._bootstrap.setup alias SourceFileLoader back into
importlib._bootstrap, with an explanation and link to back to this issue in
a comment.
msg242441 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-02 23:56
Gah.  I had tried exactly that but did it in the wrong spot.  Here's an updated patch which fixes the test.
msg242444 - (view) Author: Roundup Robot (python-dev) Date: 2015-05-03 01:23
New changeset 02e3bf65b2f8 by Eric Snow in branch 'default':
Issue #23911: Move path-based bootstrap code to a separate frozen module.
https://hg.python.org/cpython/rev/02e3bf65b2f8
msg242557 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-05-04 13:55
This checkin broke the buildbots.  If you build trunk then run

./python -bb -m test test_site

the test fails.  "-bb" is used by the normal test runner ("make test").

The problem is in the lines

        self.assertTrue(os.path.isabs(os__file__),
                        "expected absolute path, got {}".format(os__file__))
        self.assertTrue(os.path.isabs(os__cached__),
                        "expected absolute path, got {}".format(os__cached__))

os__file__ and os__cached__ are bytes but you're passing them into .format() on a str.
msg242565 - (view) Author: Roundup Robot (python-dev) Date: 2015-05-04 17:48
New changeset 36b902bbc992 by Eric Snow in branch 'default':
Issue #23911: Fix mixed bytes/strings.
https://hg.python.org/cpython/rev/36b902bbc992
msg242635 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2015-05-06 01:00
This checkin also breaks OS X framework builds.  For some reason, framework builds are compiled with the gcc -fno-common option.  The code in configure.ac to add that option dates back to the initial OS X framework support (c3c87ce4afdc from 2001).  It's not clear to me why we use that option just in the case of framework builds (I suspect it may no longer be necessary) but the fact is that we have been and currently still do.  And, in any case, the GCC description of -fno-common (https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html) suggests that there may be valid reasons to use it on some platforms or that other compilers might behave this way by default.  A simple way to produce the error on Linux is:

./configure OPT="-fno-common" && make

[...]
gcc -pthread   -o Programs/_freeze_importlib Programs/_freeze_importlib.o Modules/getbuildinfo.o Parser/acceler.o Parser/grammar1.o Parser/listnode.o Parser/node.o Parser/parser.o Parser/bitset.o Parser/metagrammar.o Parser/firstsets.o Parser/grammar.o Parser/pgen.o Parser/myreadline.o Parser/parsetok.o Parser/tokenizer.o Objects/abstract.o Objects/accu.o Objects/boolobject.o Objects/bytes_methods.o Objects/bytearrayobject.o Objects/bytesobject.o Objects/cellobject.o Objects/classobject.o Objects/codeobject.o Objects/complexobject.o Objects/descrobject.o Objects/enumobject.o Objects/exceptions.o Objects/genobject.o Objects/fileobject.o Objects/floatobject.o Objects/frameobject.o Objects/funcobject.o Objects/iterobject.o Objects/listobject.o Objects/longobject.o Objects/dictobject.o Objects/memoryobject.o Objects/methodobject.o Objects/moduleobject.o Objects/namespaceobject.o Objects/object.o Objects/obmalloc.o Objects/capsule.o Objects/rangeobject.o Objects/setobject.o Objects/sliceobject.o Objects/structseq.o Objects/tupleobject.o Objects/typeobject.o Objects/unicodeobject.o Objects/unicodectype.o Objects/weakrefobject.o Python/_warnings.o Python/Python-ast.o Python/asdl.o Python/ast.o Python/bltinmodule.o Python/ceval.o Python/compile.o Python/codecs.o Python/dynamic_annotations.o Python/errors.o Python/frozenmain.o Python/future.o Python/getargs.o Python/getcompiler.o Python/getcopyright.o Python/getplatform.o Python/getversion.o Python/graminit.o Python/import.o Python/importdl.o Python/marshal.o Python/modsupport.o Python/mystrtoul.o Python/mysnprintf.o Python/peephole.o Python/pyarena.o Python/pyctype.o Python/pyfpe.o Python/pyhash.o Python/pylifecycle.o Python/pymath.o Python/pystate.o Python/pythonrun.o Python/pytime.o Python/random.o Python/structmember.o Python/symtable.o Python/sysmodule.o Python/traceback.o Python/getopt.o Python/pystrcmp.o Python/pystrtod.o Python/pystrhex.o Python/dtoa.o Python/formatter_unicode.o Python/fileutils.o Python/dynload_shlib.o   Python/thread.o Modules/config.o Modules/getpath.o Modules/main.o Modules/gcmodule.o  Modules/_threadmodule.o  Modules/signalmodule.o  Modules/posixmodule.o  Modules/errnomodule.o  Modules/pwdmodule.o  Modules/_sre.o  Modules/_codecsmodule.o  Modules/_weakref.o  Modules/_functoolsmodule.o  Modules/_operator.o  Modules/_collectionsmodule.o  Modules/itertoolsmodule.o  Modules/atexitmodule.o  Modules/_stat.o  Modules/timemodule.o  Modules/_localemodule.o  Modules/_iomodule.o Modules/iobase.o Modules/fileio.o Modules/bytesio.o Modules/bufferedio.o Modules/textio.o Modules/stringio.o  Modules/zipimport.o  Modules/faulthandler.o  Modules/_tracemalloc.o Modules/hashtable.o  Modules/symtablemodule.o  Modules/xxsubtype.o -lpthread -ldl  -lutil   -lm
Python/pylifecycle.o:(.bss+0x20): multiple definition of `Py_FrozenFlag'
Programs/_freeze_importlib.o:(.data+0x0): first defined here
collect2: error: ld returned 1 exit status
Makefile:710: recipe for target 'Programs/_freeze_importlib' failed
make: *** [Programs/_freeze_importlib] Error 1

The OS X version of ld helpfully lists the modules:

duplicate symbol _Py_FrozenFlag in:
    Programs/_freeze_importlib.o
    Python/pylifecycle.o

I'm also not sure why the Py_FrozenFlag definition was added to _freeze_importlib.c in the first place.  Reverting it with the following patch seems to solve the problem in the few configurations I tried (OS X framework, OS X non-shared, Linux non-shared, Linux shared).

diff Programs/_freeze_importlib.c
--- a/Programs/_freeze_importlib.c      Tue May 05 12:04:35 2015 -0700
+++ b/Programs/_freeze_importlib.c      Tue May 05 17:10:39 2015 -0700
@@ -12,8 +12,6 @@
 #include <unistd.h>
 #endif

-int Py_FrozenFlag = 1;  /* Suppress errors from getpath.c */
-
 /* To avoid a circular dependency on frozen.o, we create our own structure
    of frozen modules instead, left deliberately blank so as to avoid
    unintentional import of a stale version of _frozen_importlib. */
msg242639 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-06 03:28
Thanks for pointing this out, Ned.  Early on I ran into a problem when running _freeze_importlib without the flag set.  However, I expect that it was not necessary after a certain point (e.g. once I had a valid _importlib_external.h).  I'll remove the flag as suggested.
msg242640 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-05-06 03:31
changeset:   95887:3bea670c9830
user:        Eric Snow <ericsnowcurrently@gmail.com>
date:        Tue May 05 21:29:31 2015 -0600
summary:     Remove an unnecessary flag.
msg243040 - (view) Author: Roundup Robot (python-dev) Date: 2015-05-13 05:35
New changeset cc2e52878393 by Zachary Ware in branch 'default':
Issue #23911: Fix ctypes test on Windows.
https://hg.python.org/cpython/rev/cc2e52878393
History
Date User Action Args
2015-05-13 05:35:07python-devsetmessages: + msg243040
2015-05-06 03:31:10eric.snowsetstatus: open -> closed
resolution: fixed
messages: + msg242640

stage: needs patch -> resolved
2015-05-06 03:28:53eric.snowsetmessages: + msg242639
2015-05-06 01:00:20ned.deilysetstatus: closed -> open

nosy: + ned.deily
messages: + msg242635

resolution: fixed -> (no value)
stage: resolved -> needs patch
2015-05-04 19:17:21eric.snowsetstatus: pending -> closed
2015-05-04 17:53:13eric.snowsetstatus: open -> pending
resolution: fixed
2015-05-04 17:48:56python-devsetmessages: + msg242565
2015-05-04 13:55:45larrysetstatus: closed -> open

nosy: + larry
messages: + msg242557

resolution: fixed -> (no value)
2015-05-03 01:24:31eric.snowsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2015-05-03 01:23:58python-devsetnosy: + python-dev
messages: + msg242444
2015-05-03 00:05:47eric.snowsetfiles: + path-based-importlib.diff
2015-05-03 00:05:32eric.snowsetfiles: - path-based-importlib.diff
2015-05-03 00:03:19eric.snowsetfiles: + path-based-importlib.diff
2015-05-03 00:02:28eric.snowsetfiles: - path-based-importlib.diff
2015-05-02 23:56:13eric.snowsetfiles: + path-based-importlib.diff

messages: + msg242441
2015-05-02 23:31:39ncoghlansetmessages: + msg242440
2015-05-02 21:57:57eric.snowsetfiles: - path-based-importlib.diff
2015-05-02 21:57:44eric.snowsetfiles: + path-based-importlib.diff

messages: + msg242437
2015-05-02 21:25:37eric.snowsetmessages: + msg242435
2015-05-02 20:52:41eric.snowsetmessages: + msg242434
2015-05-02 20:47:49eric.snowsetfiles: + path-based-importlib.diff

messages: + msg242433
2015-04-27 22:35:53eric.snowsetmessages: + msg242160
2015-04-25 07:31:36ncoghlansetmessages: + msg242001
2015-04-20 03:24:02eric.snowsetmessages: + msg241593
2015-04-20 03:23:21eric.snowsetfiles: + path-based-importlib.diff

messages: + msg241592
2015-04-13 21:34:15raulcdsetnosy: + raulcd
2015-04-11 01:29:38eric.snowcreate