Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Python: Bugs

[issue14657] Avoid two importlib copies

 

 

First page Previous page 1 2 3 Next page Last page  View All Python bugs RSS feed   Index | Next | Previous | View Threaded


report at bugs

Apr 25, 2012, 9:46 AM

Post #51 of 73 (380 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Marc-Andre Lemburg <mal [at] egenix> added the comment:

Nick Coghlan wrote:
>
> Nick Coghlan <ncoghlan [at] gmail> added the comment:
>
> At the very least, failing to regenerate importlib.h shouldn't be a fatal build error. It should just run with what its got, and hopefully you will get a working interpreter out the other end, such that you can regenerate the frozen module on the next pass.
>
> If we change that, then I'm OK with keeping the automatic rebuild.

I fixed that already today.

You now get a warning message from make, but no build error across
all buildbots like I had run into yesterday when working on the code.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Apr 25, 2012, 10:46 AM

Post #52 of 73 (382 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Marc-Andre Lemburg <mal [at] egenix> added the comment:

Marc-Andre Lemburg wrote:
>
> Marc-Andre Lemburg <mal [at] egenix> added the comment:
>
> Nick Coghlan wrote:
>>
>> Nick Coghlan <ncoghlan [at] gmail> added the comment:
>>
>> At the very least, failing to regenerate importlib.h shouldn't be a fatal build error. It should just run with what its got, and hopefully you will get a working interpreter out the other end, such that you can regenerate the frozen module on the next pass.
>>
>> If we change that, then I'm OK with keeping the automatic rebuild.
>
> I fixed that already today.

See http://bugs.python.org/issue14605 and
http://hg.python.org/lookup/acfdf46b8de1 +
http://hg.python.org/cpython/rev/5fea362b92fc

> You now get a warning message from make, but no build error across
> all buildbots like I had run into yesterday when working on the code.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Apr 25, 2012, 6:21 PM

Post #53 of 73 (377 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

My plan would be for the frozen version to be entirely implicit, and have only the subsequent import of the version from disk actually modify the public hooks.

However, I realised today that my current patch would break "stdlib-from-zipfile" approaches, so any bootstrapping of importlib from disk would have to take place after zipimport was put in place. That suggests a few possible changes:
- reordering import_init so zipimport is initialised before the bootstrapping step
- possibly downgrading failure of the bootstrapping step to a warning rather than a fatal error (i.e. continuing with the frozen version if the source version can't be found)

This may still all prove to be too complicated and fragile, but I'm not prepared to give up on it yet - having the interpreter pick up on _bootstrap.py changes for the main import system *without* needing to be rebuilt first seems worth a bit of additional complexity in the bootstrapping mechanism.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

Apr 29, 2012, 1:24 AM

Post #54 of 73 (379 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

Uploaded new bootstrapping patch that handles the fully explicit import machinery.

I also tweaked a couple of things so it plays nicely in terms of building an initial version with the checked in importlib.h. Specifically: pythonrun still calls _frozen_importlib._install and can tolerate that function returning None. Longer term, we'd give the two hooks different names and returning None will become a fatal error, but for the moment, the current behaviour makes the patch much easier to work with.

Order is still wrong relative to the zipimport machinery and I haven't benchmarked the startup time overheads.

----------
Added file: http://bugs.python.org/file25403/issue14657_bootstrap_from_disk_v2.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 4, 2012, 12:20 PM

Post #55 of 73 (382 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Roundup Robot <devnull [at] psf> added the comment:

New changeset 257cbd2fac38 by Brett Cannon in branch 'default':
Issue #13959: Re-implement imp.get_suffixes() in Lib/imp.py.
http://hg.python.org/cpython/rev/257cbd2fac38

----------
nosy: +python-dev

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 5, 2012, 9:50 PM

Post #56 of 73 (384 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Eric Snow <ericsnowcurrently [at] gmail> added the comment:

Here's my take. No one will care about _frozen_importlib vs. importlib._bootstrap normally, right? If __module__/__file__ says _frozen_importlib, it's no big deal. The only time you care about the distiction for importlib._bootstrap is when you're hacking on _bootstrap.py. So let's keep the common case in sight and go from there.

There are two sides to the uncommon case:

1. making sure importlib still works after hacking on _bootstrap.py (test_imp, test_import, test_importlib using your new _bootstrap.py).
2. making sure everything still works after hacking on _bootstrap.py (the whole test suite with a new importlib.h?).

For the first part, let's simply ignore the pure Python importlib._bootstrap by default? Then we stick a context manager in importlib.test.util that enables it. When you're hacking on _bootstrap.py, you switch it over. The common path stays pretty clean.

I've attached a patch for the first part which has similarities to Antoine's. (I didn't apply the context manager to the importlib test cases though.)

For that second part, something along the lines of what Nick has posted would be pretty close, but I'm not sure it's worth it. From what I understand, Nick's patch would add yet another import (importlib) to startup to cover a situation that happens very infrequently (hacking _bootstrap.py). However, I'm torn because...

...dealing with a busted importlib.h is not fun. Is there a different approach we could take for that second part? Perhaps something like this:

1. python starts up normally.
2. we clear out all the entire import state except for builtins.
3. we stick importlib._bootstrap in place.
4. we set builtins.__import__ to importlib.__import__.
5. we re-populate sys.modules by reloading all the modules that were in there before (?).
6. we run the test suite against this new import state.
7. ...
8. profit!

I'm probably missing something here, but I expect we could stick something like that in some place like importlib.test.util. Would that be sufficient to mitigate the chance of breaking importlib.h?


------------------------------------

Example of using my patch:

>>> import sys
>>> import importlib.test.util as util
>>> importlib._bootstrap
<module '_frozen_importlib' from '<frozen>'>
>>> sys.modules['importlib._bootstrap']
<module '_frozen_importlib' from '<frozen>'>
>>> with util.bootstrap_context(importlib, importlib._pure_bootstrap):
... importlib._bootstrap
... sys.modules['importlib._bootstrap']
...
<module 'importlib._bootstrap' from '/home/esnow/projects/cpython/Lib/importlib/_bootstrap.py'>
<module 'importlib._bootstrap' from '/home/esnow/projects/cpython/Lib/importlib/_bootstrap.py'>
>>> importlib._bootstrap
<module '_frozen_importlib' from '<frozen>'>
>>> sys.modules['importlib._bootstrap']
<module '_frozen_importlib' from '<frozen>'>

----------
Added file: http://bugs.python.org/file25479/issue14657_safe_bootstrap.diff

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 12:04 AM

Post #57 of 73 (372 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

The piece you're missing is that the interpreter state holds a direct reference to the import machinery in interp->importlib, and *that's* what gets used by the builtin __import__ implementation.

I'm beginning to think the thing to do is to simply say "yes, there are two copies of importlib._bootstrap". By default, the compiled in one is used, but you can replace it with the on-disk one by appropriately editing sys.meta_path and sys.path_hooks.

Trying to hide it isn't going to eliminate the potential problems - it's just going to move the problems around (and likely make them even more confusing in the process).

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 12:06 AM

Post #58 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

Forgot to add: in our own tests, we should ensure that both the frozen and on-disk versions get executed.

I believe that's already the case, since I don't recall anyone removing the test infrastructure that ensured both import.c and importlib are tested for correct behaviour.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 12:23 AM

Post #59 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> I believe that's already the case, since I don't recall anyone
> removing the test infrastructure that ensured both import.c and
> importlib are tested for correct behaviour.

What do you mean? I think test_importlib only tests the on-disk version.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 12:28 AM

Post #60 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Here's my take. No one will care about _frozen_importlib vs.
> importlib._bootstrap normally, right? If __module__/__file__ says
> _frozen_importlib, it's no big deal.

The reason I'd prefer __file__ to point to the actual Python file is so
that people reading a traceback can find the source code. Of course
that's a bit minor.
(and, incidentally, the traceback itself will display the source code
lines)

> The only time you care about the distiction for importlib._bootstrap
> is when you're hacking on _bootstrap.py. So let's keep the common
> case in sight and go from there.

Agreed.

> For the first part, let's simply ignore the pure Python
> importlib._bootstrap by default? Then we stick a context manager in
> importlib.test.util that enables it. When you're hacking on
> _bootstrap.py, you switch it over. The common path stays pretty
> clean.

Looks good to me.

> I've attached a patch for the first part which has similarities to
> Antoine's. (I didn't apply the context manager to the importlib test
> cases though.)

I think set_bootstrap() looks a bit fragile, since we have to manually
add any importlib attributes that are exported in importlib/__init__.py.
Perhaps we could detect them automatically?

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 9:13 AM

Post #61 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Brett Cannon <brett [at] python> added the comment:

To respond to Nick's "yes, there are two copies of importlib._bootstrap" leanings, distutils2 has actually run into issues with this because they initially made some assumptions about consistency in what importlib returned vs. what import does (Arfrever can explain better than I can since he keeps pointing it out to me =).

If using _frozen_importlib to bootstrap in importlib._bootstrap is looking bad, then I'm fine w/ simply having the tests for importlib and imp use importlib._bootstrap and otherwise just use _frozen_importlib for everything else since I have tried to be diligent to add any and all import-related tests to importlib. Except while developing the code should be exactly the same so hiding the details really won't make much of a difference in the very common case.

If we go this route, though, then we really should take this time to do a proper context manager/decorator/whatever that covers all import state (including uncaching modules and sys.path_importer_cache) that we might care about and put the solution into test.support (what issue #14715 is asking for and I think is reasonable). We should also then add to regrtest detection of stuff left in sys.path_importer_cache or sys.modules that do not come from _frozen_importlib (which should help with the sporadic test_imp failure).

----------
dependencies: +test.support.DirsOnSysPath should be replaced by importlib.test.util.import_state

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 9:24 AM

Post #62 of 73 (389 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Arfrever Frehtes Taifersar Arahesis <Arfrever.FTA [at] GMail> added the comment:

It was distribute (fork of setuptools, with added support for Python 3), not distutils2.
distribute has been changed to directly use _frozen_importlib:
https://bitbucket.org/tarek/distribute/changeset/a2685f3af854
https://bitbucket.org/tarek/distribute/changeset/77c8b155a07d
distribute checks __loader__ and __class__.__mro__ attributes of modules.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 9:33 AM

Post #63 of 73 (372 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> It was distribute (fork of setuptools, with added support for Python 3), not distutils2.
> distribute has been changed to directly use _frozen_importlib:

This sounds like a rather good hint we need to avoid duplicate copies.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 12:05 PM

Post #64 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Brett Cannon <brett [at] python> added the comment:

I think it's beyond a hint and says we need to find a solution or else other people will run into similar issues.

And while I'm thinking about it, there is precedent for exposing modules under a different name than they are actually installed as in the system (e.g. os.path is posixpath), so I don't think we need to bend over backwards to mask every detail if the bootstrap solution is not taken (e.g. if we decided to just paper over _frozen_importlib we don't need to iterate over _frozen_importlib.__dict__ and patch up __module__). But I do think that we need to choose some solution to prevent this "forking" of code in the running interpreter.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 6, 2012, 11:50 PM

Post #65 of 73 (389 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Nick Coghlan <ncoghlan [at] gmail> added the comment:

In that case, how about we go with:

1. By default, importlib._bootstrap is never imported. Instead, it is set to be a reference to _frozen_importlib. However, _frozen_importlib does *not* lie about where it came from (and doesn't assume the on-disk source matches the frozen source).

2. We provide two private functions in importlib.__init__: one that replaces all _frozen_importlib references in the import state with importlib._bootstrap references (retrieving the latter from disk first), and one that reverses the process.

Note that the __import__ builtin should be replaced as well, since that will otherwise call in to the frozen version of the module.

This is basically the same as Eric Snow's suggestion, just with most of the nuts and bolts kept within importlib, so that the testing context manager doesn't need to know the details - it can just call the appropriate importlib functions to change the active implementation.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:01 AM

Post #66 of 73 (372 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Brett Cannon <brett [at] python> added the comment:

Should we have a separate context manager for this, or just make it a flag for a unified import_state() decorator? Or do we want to *always* force the use of the Python code instead of the frozen code?

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:05 AM

Post #67 of 73 (373 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Should we have a separate context manager for this, or just make it a
> flag for a unified import_state() decorator? Or do we want to *always*
> force the use of the Python code instead of the frozen code?

Ideally, we would want to test both versions, so that any oddity in the
freezing mechanism gets exercised and diagnosed properly.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:11 AM

Post #68 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> Ideally, we would want to test both versions, so that any oddity in the
> freezing mechanism gets exercised and diagnosed properly.

(not to mention the speedups in import.c)

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:23 AM

Post #69 of 73 (375 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Eric Snow <ericsnowcurrently [at] gmail> added the comment:

I'm +1 on Nick's recommendation.

@Antoine
> Ideally, we would want to test both versions, so that any oddity in
> the freezing mechanism gets exercised and diagnosed properly.

+1

Does this mean that the whole test suite should be run under both (whenever _bootstrap.py is modified)? Would that warrant a new flag for the test suite or even an automated check?

That's what I was getting at with this:

> 1. python starts up normally.
> 2. we clear out all the entire import state except for builtins.
> 3. we stick importlib._bootstrap in place.
> 4. we set builtins.__import__ to importlib.__import__.
> 5. we re-populate sys.modules by reloading all the modules that
> were in there before (?).
> 6. we run the test suite against this new import state.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:42 AM

Post #70 of 73 (372 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Arfrever Frehtes Taifersar Arahesis <Arfrever.FTA [at] GMail> added the comment:

It might be sufficient to only run tests from the following files with both importlib._bootstrap and _frozen_importlib:
test_imp.py
test_import.py
test_importhooks.py
test_importlib.py
test_pkgimport.py
test_threaded_import.py
test_zipimport.py
test_zipimport_support.py

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 8:47 AM

Post #71 of 73 (371 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Antoine Pitrou <pitrou [at] free> added the comment:

> It might be sufficient to only run tests from the following files with
> both importlib._bootstrap and _frozen_importlib:

I was only thinking about test_importlib myself.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 7, 2012, 12:31 PM

Post #72 of 73 (374 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Brett Cannon <brett [at] python> added the comment:

I would say test_importlib and test_imp (test_import really should just get folded into test_importlib).

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com


report at bugs

May 17, 2012, 8:25 AM

Post #73 of 73 (341 views)
Permalink
[issue14657] Avoid two importlib copies [In reply to]

Brett Cannon <brett [at] python> added the comment:

Realized that any decorator or context manager that is created for swapping _frozen_importlib/importlib._bootstrap should also verify no module is left in sys.modules with a bad loader and that sys.path_importer_cache doesn't have a bad finder (I would say that this would go into test.support.regrtest's state checks, but that seems overkill for only two tests).

And this might be worth doing as a decorator (method or class) to make it easier to make sure the requisite tests always run with both versions (or copying what test_warnings does). I don't want to do anything in a module's test_main() as that precludes using unittest's test discovery for running tests.

----------

_______________________________________
Python tracker <report [at] bugs>
<http://bugs.python.org/issue14657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

First page Previous page 1 2 3 Next page Last page  View All Python bugs RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.