classification
Title: Make import machinery explicit
Type: behavior Stage: committed/rejected
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Arfrever, brett.cannon, eric.araujo, eric.smith, eric.snow, lemburg, ncoghlan, python-dev, r.david.murray
Priority: release blocker Keywords: patch

Created on 2012-04-17 15:58 by brett.cannon, last changed 2012-04-27 20:03 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
explicit.diff brett.cannon, 2012-04-23 01:41
explicit_path_hooks.diff brett.cannon, 2012-04-24 00:51
explicit_path_hooks.diff brett.cannon, 2012-04-25 02:52
Messages (33)
msg158552 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-17 15:58
There should no longer be any implicit part of import when there doesn't have to be. To make import fully explicit, some things need to happen (in context to importlib):

* Expose FileLoader
* Expose SourceFileLoader
* Expose PathFinder
* Expose the extension loader
* Expose the default path hook
* Explicitly set sys.meta_path with PathFinder
* Explicitly set sys.path_hooks with the default path hook and zipimport
* Make sys.path_importer_cache view None as no finder discovered
* Insert None into sys.path_importer_cache instead of NullImporter

I am not exposing SourcelessFileLoader because importlib publicly tries to discourage the shipping of .pyc files w/o their corresponding source files. Otherwise all objects as used by importlib for performing imports will become public.
msg158996 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-23 01:41
Attached is my (failing) attempt at making import explicit. Unfortunately I have four failing tests, 3 of which revolve around __main__ (which is why I added Nick to see if he had any ideas):

test_cmd_line_script test_runpy test_threaded_import test_zipimport_support
msg158997 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-23 01:43
New changeset 1da623513b26 by Brett Cannon in branch 'default':
Issue #14605: Expose importlib.abc.FileLoader and
http://hg.python.org/cpython/rev/1da623513b26
msg159107 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-24 00:51
Updated patch for an explicit sys.path_hooks that simply tweaks the test_cmd_line_script tests to stop requiring an absolute path on the files and instead verifies that the relative paths are legit. It also adds warnings when None is found in sys.path_importer_cache and then subsequently just tries sys.path_hooks again. It also warnings when sys.path_hooks is empty.
msg159114 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-04-24 04:33
I put those explicit path checks in there deliberately.

I really don't want to go back to having any __file__ attributes that are sensitive to sys.path changes.
msg159115 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-04-24 04:35
Oops: s/sensitive to sys.path changes/sensitive to current working directory changes/
msg159145 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-24 14:39
Of course you did because you just like making my life a living hell when it comes to the __file__ attribute. =)

OK, I will have another look when I get home, but last time I dealt with this the issue is some tests expect a relative path while others expect an absolute one. And allowing an empty string for the current directory is just evil.
msg159149 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-24 14:52
Brett Cannon wrote:
> I am not exposing SourcelessFileLoader because importlib publicly tries to discourage the shipping of .pyc files w/o their corresponding source files. Otherwise all objects as used by importlib for performing imports will become public.

What's the reasoning behind this idea ? Is Python 3.3 no longer meant to
be used for closed source applications ?
msg159157 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-24 15:12
That initial comment is out-of-date. If you look that the commit I made I  documented importlib.machinery._SourcelessFileLoader. I am continuing the discouragement of using bytecode files as an obfuscation technique (because it's a bad one), but I decided to at least document the class so people can use it at their own peril and know about it if they happen to come across the object during execution.
msg159177 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-24 17:55
Brett Cannon wrote:
> 
> That initial comment is out-of-date. If you look that the commit I made I  documented importlib.machinery._SourcelessFileLoader. I am continuing the discouragement of using bytecode files as an obfuscation technique (because it's a bad one), but I decided to at least document the class so people can use it at their own peril and know about it if they happen to come across the object during execution.

It's not a perfect obfuscation technique, but a pretty simple and
(legally) effective one to use.

FWIW, I don't think the comment in the check-in is appropriate:

"""
   1.127 +   It is **strongly** suggested you do not rely on this loader (hence the
   1.128 +   leading underscore of the class). Direct use of bytecode files (and thus not
   1.129 +   source code files) inhibits your modules from being usable by all Python
   1.130 +   implementations. It also runs the risk of your bytecode files not being
   1.131 +   usable by new versions of Python which change the bytecode format. This
   1.132 +   class is only documented as it is directly used by import and thus can
   1.133 +   potentially have instances show up as a module's ``__loader__`` attribute.
"""

The "risks" you mention there are really up to the application developers
to decide how to handle, not the Python developers. Python has a long
tradition of being friendly to commercial applications and I don't see
any reason why we should stop that.

If you do want this to change, please write a PEP. This may appear
to be a small change in direction, but it does in fact have quite
some impact on the usefulness of CPython in commercial settings.

I also think that the SourcelessFileLoader loader should be first class
citizen without the leading underscore if the importlib is to completely
replace the current import mechanism. Why force developers to write their
own loader instead of using the standard one just because of the leading
underscore, when it's only 20 lines of code ?

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com

________________________________________________________________________
2012-04-28: PythonCamp 2012, Cologne, Germany               4 days to go

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
msg159195 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-24 19:15
I documented it explicitly so people can use it if they so choose (e.g. look at sys._getframe()). If you want to change this that's fine, but I am personally not going to put the effort in to rename the class, update the tests, and change the docs for this (we almost stopped allowing the importation of bytecode directly not so long ago but got push-back so we backed off).
msg159234 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-24 23:37
New changeset 8aa4737d67d2 by Marc-Andre Lemburg in branch 'default':
Issue #14605: Rename _SourcelessFileLoader to SourcelessFileLoader
http://hg.python.org/cpython/rev/8aa4737d67d2
msg159236 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-24 23:39
Brett Cannon wrote:
> 
> I documented it explicitly so people can use it if they so choose (e.g. look at sys._getframe()). If you want to change this that's fine, but I am personally not going to put the effort in to rename the class, update the tests, and change the docs for this (we almost stopped allowing the importation of bytecode directly not so long ago but got push-back so we backed off).

I renamed the loader and reworded the notice in the docs.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com

________________________________________________________________________
2012-04-28: PythonCamp 2012, Cologne, Germany               3 days to go

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
msg159237 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-04-24 23:52
Hmm.  Some at least of the buildbots have failed to build after that patch:

./python ./Python/freeze_importlib.py \
    ./Lib/importlib/_bootstrap.py Python/importlib.h
make: ./python: Command not found
make: *** [Python/importlib.h] Error 127
program finished with exit code 2

(http://www.python.org/dev/buildbot/all/builders/AMD64%20Gentoo%20Wide%203.x/builds/3771)
msg159240 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-25 00:11
New changeset e30196bfc11d by Marc-Andre Lemburg in branch 'default':
Issue #14605: Revert renaming of _SourcelessFileLoader, since it caused
http://hg.python.org/cpython/rev/e30196bfc11d
msg159242 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-25 00:27
R. David Murray wrote:
> 
> R. David Murray <rdmurray@bitdance.com> added the comment:
> 
> Hmm.  Some at least of the buildbots have failed to build after that patch:
> 
> ./python ./Python/freeze_importlib.py \
>     ./Lib/importlib/_bootstrap.py Python/importlib.h
> make: ./python: Command not found
> make: *** [Python/importlib.h] Error 127
> program finished with exit code 2
> 
> (http://www.python.org/dev/buildbot/all/builders/AMD64%20Gentoo%20Wide%203.x/builds/3771)

Thanks for mentioning this. I've reverted the change for now and
will have a look tomorrow.

The logs of the failing bots are not very informative about what
is going on:

gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/dynamic_annotations.o Python/dynamic_annotations.c
gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/errors.o Python/errors.c
./python ./Python/freeze_importlib.py \
    ./Lib/importlib/_bootstrap.py Python/importlib.h
make: ./python: Command not found
make: *** [Python/importlib.h] Error 127
program finished with exit code 2

vs.

gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/dynamic_annotations.o Python/dynamic_annotations.c
gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/errors.o Python/errors.c
gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/frozen.o Python/frozen.c
gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/frozenmain.o Python/frozenmain.c
gcc -pthread -c -Wno-unused-result -g -O0 -Wall -Wstrict-prototypes    -I. -I./Include
-DPy_BUILD_CORE -o Python/future.o Python/future.c

I guess some commands are not printed to stdout.

Looking at the buildbots again: reverting the patch has not caused
the lights to go green again. Very strange indeed.

Looking further I found this line in the Makefile:

############################################################################
# Importlib

Python/importlib.h: $(srcdir)/Lib/importlib/_bootstrap.py $(srcdir)/Python/freeze_importlib.py
        ./$(BUILDPYTHON) $(srcdir)/Python/freeze_importlib.py \
            $(srcdir)/Lib/importlib/_bootstrap.py Python/importlib.h

Since the patch modified _bootstrap.py, make wants to recreate importlib.h,
but at that time $(BUILDPYTHON) doesn't yet exist.
msg159244 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-25 00:32
New changeset a2cf07135e4f by Marc-Andre Lemburg in branch 'default':
Issue #14605: Rename _SourcelessFileLoader to SourcelessFileLoader.
http://hg.python.org/cpython/rev/a2cf07135e4f
msg159245 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-25 00:39
Marc-Andre Lemburg wrote:
> Looking further I found this line in the Makefile:
> 
> ############################################################################
> # Importlib
> 
> Python/importlib.h: $(srcdir)/Lib/importlib/_bootstrap.py $(srcdir)/Python/freeze_importlib.py
>         ./$(BUILDPYTHON) $(srcdir)/Python/freeze_importlib.py \
>             $(srcdir)/Lib/importlib/_bootstrap.py Python/importlib.h
> 
> Since the patch modified _bootstrap.py, make wants to recreate importlib.h,
> but at that time $(BUILDPYTHON) doesn't yet exist.

I now ran 'make' after applying the patches to have the importlib.h
recreated.

This setup looks a bit fragile to me.

I think it would be better to make creation of the importlib.h an
explicit operation that has to be done in case the Python code
changes (e.g. by creating a make target build-importlib.h),
with the Makefile only warning about a needed update instead
of failing completely.
msg159246 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-25 01:10
You can see a little discussion in http://bugs.python.org/issue14642, but it has been discussed elsewhere and the automatic rebuilding was preferred (but it is not a requirement to build as importlib.h is in hg).
msg159248 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-25 02:52
I found out why runpy was giving back an absolute file path for __file__; because pkgutil was doing that through its ImpLoader, unlike what import does by default which is just taking what path it has and appending file names (and thus not making anything absolute).

So I have simply forced runpy to make absolute paths of the files it is given. Nick, can you have a look and let me know if this fix is reasonable, or if another os.path.abspath() call is needed somewhere in runpy?
msg159253 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-04-25 06:05
Yeah, I was actually going to suggest forcing an absolute path for __main__.__file__ in runpy if you didn't want to do it in importlib itself.

I'm much happier with that approach than changing the tests, so the updated patch looks good to me.
msg159262 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-25 08:54
New changeset acfdf46b8de1 by Marc-Andre Lemburg in branch 'default':
Issue #14605 and #14642:
http://hg.python.org/cpython/rev/acfdf46b8de1
msg159263 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-25 08:55
Brett Cannon wrote:
> 
> You can see a little discussion in http://bugs.python.org/issue14642, but it has been discussed elsewhere and the automatic rebuilding was preferred (but it is not a requirement to build as importlib.h is in hg).

An automatic rebuild is fine, but only as long as the local ./python
actually exists.

I was unaware of make rule, so did not run make to check things before
the checkin. As a result, the bootstrap module received a more recent
timestamp than importlib.h and this caused all the buildbots to
force a rebuild of importlib.h - which failed, since they didn't
have a built ./python at that stage.

I checked in a fix and added a warning to the bootstrap script.
msg159327 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-25 17:45
New changeset 5fea362b92fc by Marc-Andre Lemburg in branch 'default':
Issue #14605 and #14642: Issue a warning in case Python\importlib.h needs to
http://hg.python.org/cpython/rev/5fea362b92fc
msg159334 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-25 20:06
While not in the initial list, _find_module() would be really handy.  Perhaps we could call it "get_loader" instead.  "find_module" is a misleading name and I don't see any parallel with imp.find_module as something to aspire to.
msg159337 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-25 20:52
importlib.find_module() (or get_loader() as it would replace pkgutil.get_loader() as well) is definitely planned. So is importlib.util.resolve_name() (although maybe that is basic enough to want top-level?).
msg159343 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-26 00:19
New changeset 8dab93ec19de by Brett Cannon in branch 'default':
Issue #14605: Insert to the front of sys.path_hooks instead of appending.
http://hg.python.org/cpython/rev/8dab93ec19de
msg159346 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-26 00:55
New changeset 57d558f1904d by Brett Cannon in branch 'default':
Issue #14605: Make explicit the entries on sys.path_hooks that used to
http://hg.python.org/cpython/rev/57d558f1904d
msg159347 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-26 00:58
Just to document why my explicit sys.path_hooks patch didn't quite change the meaning of None in sys.path_importer_cache, I found a bunch of places in the stdlib and in Modules/main.c where NullImporter is explicitly expected to be returned, so I wanted to get this initial patch in before I saw what would happen if None didn't change its meaning.

I think I will try a patch to have None mean "no finder" instead of the current "retry sys.path_hooks" and see how that goes. The real trick is whether stopping the use of NullImporter will be easy or not. That will be what really decides if it stops being on sys.path_hooks by default.
msg159482 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-27 18:02
New changeset c18256de00bb by Brett Cannon in branch 'default':
Issue #14605: Insert to the front of sys.meta_path, don't append.
http://hg.python.org/cpython/rev/c18256de00bb

New changeset 3bd60cc27664 by Brett Cannon in branch 'default':
Issue #14605: Stop having implicit entries for sys.meta_path.
http://hg.python.org/cpython/rev/3bd60cc27664
msg159486 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-27 19:31
New changeset 7025ee00dbf6 by Brett Cannon in branch 'default':
Issue #14605: Use None in sys.path_importer_cache to represent no
http://hg.python.org/cpython/rev/7025ee00dbf6
msg159487 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-27 19:45
New changeset 141ed4b426e1 by Brett Cannon in branch 'default':
Issue #14605: Don't error out if get_importer() returns None.
http://hg.python.org/cpython/rev/141ed4b426e1
msg159488 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-27 19:49
OK, so the todo of this issue is now finished. I am just waiting for the buildbots to come back green before I close this issue fully.
History
Date User Action Args
2012-04-27 20:03:12brett.cannonsetstatus: pending -> closed
2012-04-27 19:49:02brett.cannonsetstatus: open -> pending

messages: + msg159488
stage: commit review -> committed/rejected
2012-04-27 19:45:28python-devsetmessages: + msg159487
2012-04-27 19:31:52python-devsetmessages: + msg159486
2012-04-27 18:02:43python-devsetmessages: + msg159482
2012-04-26 00:58:39brett.cannonsetmessages: + msg159347
2012-04-26 00:55:23python-devsetmessages: + msg159346
2012-04-26 00:19:07python-devsetmessages: + msg159343
2012-04-25 20:52:39brett.cannonsetmessages: + msg159337
2012-04-25 20:06:28eric.snowsetmessages: + msg159334
2012-04-25 17:45:29python-devsetmessages: + msg159327
2012-04-25 08:55:34lemburgsetmessages: + msg159263
2012-04-25 08:54:58python-devsetmessages: + msg159262
2012-04-25 06:05:59ncoghlansetassignee: ncoghlan -> brett.cannon
stage: patch review -> commit review
2012-04-25 06:05:28ncoghlansetmessages: + msg159253
2012-04-25 02:54:21brett.cannonsetassignee: brett.cannon -> ncoghlan
stage: patch review
2012-04-25 02:53:17brett.cannonsetfiles: + explicit_path_hooks.diff

messages: + msg159248
2012-04-25 01:10:33brett.cannonsetmessages: + msg159246
2012-04-25 01:08:48eric.smithsetnosy: + eric.smith
2012-04-25 00:39:26lemburgsetmessages: + msg159245
2012-04-25 00:32:26python-devsetmessages: + msg159244
2012-04-25 00:27:53lemburgsetmessages: + msg159242
2012-04-25 00:11:17python-devsetmessages: + msg159240
2012-04-24 23:52:16r.david.murraysetnosy: + r.david.murray
messages: + msg159237
2012-04-24 23:40:00lemburgsetmessages: + msg159236
2012-04-24 23:37:04python-devsetmessages: + msg159234
2012-04-24 19:15:45brett.cannonsetmessages: + msg159195
2012-04-24 17:55:05lemburgsetmessages: + msg159177
2012-04-24 15:12:42brett.cannonsetmessages: + msg159157
2012-04-24 14:52:03lemburgsetnosy: + lemburg
messages: + msg159149
2012-04-24 14:40:00brett.cannonsetmessages: + msg159145
2012-04-24 04:35:34ncoghlansetmessages: + msg159115
2012-04-24 04:33:29ncoghlansetmessages: + msg159114
2012-04-24 00:51:29brett.cannonsetfiles: + explicit_path_hooks.diff

messages: + msg159107
2012-04-23 01:43:07python-devsetnosy: + python-dev
messages: + msg158997
2012-04-23 01:42:09brett.cannonsetnosy: + ncoghlan
2012-04-23 01:41:56brett.cannonsetfiles: + explicit.diff
keywords: + patch
2012-04-23 01:41:24brett.cannonsetmessages: + msg158996
2012-04-22 19:38:23brett.cannonlinkissue2377 dependencies
2012-04-22 19:19:12Arfreversetnosy: + Arfrever
2012-04-21 23:02:37brett.cannonlinkissue13959 dependencies
2012-04-18 00:05:58eric.snowsetnosy: + eric.snow
2012-04-17 16:03:53eric.araujosetnosy: + eric.araujo
2012-04-17 15:58:04brett.cannoncreate