Repository navigation
Re-implement parts of imp in pure Python #58167
Description
| BPO | 13959 |
|---|---|
| Nosy | @brettcannon, @birkenfeld, @pitrou, @vstinner, @benjaminp, @merwok, @meadori, @ericsnowcurrently, @berkerpeksag |
| PRs | |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/brettcannon'
closed_at = <Date 2012-07-09.20:23:48.007>
created_at = <Date 2012-02-07.00:51:29.679>
labels = ['library', 'release-blocker']
title = 'Re-implement parts of imp in pure Python'
updated_at = <Date 2017-12-06.16:25:57.617>
user = 'https://github.com/brettcannon'bugs.python.org fields:
activity = <Date 2017-12-06.16:25:57.617>
actor = 'vstinner'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2012-07-09.20:23:48.007>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2012-02-07.00:51:29.679>
creator = 'brett.cannon'
dependencies = ['2377', '14605', '14657', '15166', '15167']
files = ['24846', '24979', '24980', '25250', '25302', '25303', '25308', '25309', '25335', '25336', '25834', '26058']
hgrepos = []
issue_num = 13959
keywords = ['patch']
message_count = 76.0
messages = ['152800', '155790', '156473', '156474', '156475', '156527', '156530', '156964', '158269', '158362', '158364', '158365', '158370', '158371', '158377', '158383', '158398', '158474', '158522', '158531', '158555', '158575', '158593', '158594', '158622', '158628', '158704', '158895', '158908', '158930', '158931', '158939', '158941', '158951', '158955', '158968', '158971', '158985', '158986', '158995', '158998', '158999', '159118', '159119', '159628', '159630', '159631', '159641', '159644', '159757', '159758', '159960', '159962', '159969', '160424', '160433', '160434', '160439', '160493', '160495', '160524', '162276', '162356', '162357', '162364', '162384', '162940', '163212', '163232', '163569', '163798', '164102', '165116', '165120', '165148', '307747']
nosy_count = 12.0
nosy_names = ['brett.cannon', 'georg.brandl', 'jpe', 'pitrou', 'vstinner', 'benjamin.peterson', 'eric.araujo', 'Arfrever', 'meador.inge', 'python-dev', 'eric.snow', 'berker.peksag']
pr_nums = ['4736']
priority = 'release blocker'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue13959'
versions = ['Python 3.3']
Activity
brettcannon commentedon Feb 7, 2012
A bunch of code in Python/import.c exists purely for the imp module, but a large chunk of it does not need to be implemented in C but instead can be implemented in Python code.
Once importlib is bootstrapped in then an attempt to scale back the C code should be done by re-implementing parts of imp in pure Python (either in some _imp module or directly in _importlib itself).
brettcannon commentedon Mar 14, 2012
Attached is a start for re-implementing imp. Still need code for cache_from_source, source_from_cache, find_module, load_module, reload, load_compiled, load_source, and load_package.
ericsnowcurrently commentedon Mar 21, 2012
Here's an incomplete (though essentially test-passing) patch that flips more of imp into importlib. It builds on Brett's patch. The gist is that imp.py is a minimal wrapper around importlib, which uses _imp directly.
Ultimately the things implemented in Lib/importlib/_bootstrap.py should be yanked out of Python/import.c (and Python/dynload_*.c). I expect that this will wait for the bootstrap merge (bpo-2377) to land.
ericsnowcurrently commentedon Mar 21, 2012
Here's a listing of the places where import-ish modules are used in CPython. Once things all the import stuff currently on the table is wrapped up, I'll probably work on switching things over to using importlib directly.
ericsnowcurrently commentedon Mar 21, 2012
Of note for my patch is the addition of SUFFIXES and MODE to the 3 main "file loader" classes. I did this to reverse the dependence on imp.get_suffixes().
As well, a couple of extra functions are added to Python/importlib/_bootstrap.py for a similar reason, but should also be meaningful externally in their own right.
load_module_from_file(), is one of these, though the current incarnation relies on a mythical new method on loaders that may not pass muster.
(FWIW, I accidently included the use_of_imp.txt file in the patch and don't intend it to get included)
merwok commentedon Mar 22, 2012
Please leave distutils unchanged; we don’t clean it up or improve it in any way, it’s bugfixes only. Changing packaging is fine; I’ll just keep using the imp module in the distutils2-python3 backport.
ericsnowcurrently commentedon Mar 22, 2012
Sounds good. It will be a while before we get there, but at that point I was already planning on getting in touch with the maintainers of relevant modules before messing with them.
ericsnowcurrently commentedon Mar 28, 2012
(Éric, I'd reply in the review if reitveld recognized my current username -- see http://psf.upfronthosting.co.za/roundup/meta/issue402. FYI, I also did not get an email for your review, which is likely related to that issue.)
Regarding __all__, I didn't realize that about being a tuple. I'll fix it.
As to the "dm" in the SOABI value, that is definitely something we'll have to factor in. There are actually a few such places in the current patch that rely on problematic data. It's either hidden in C or not available during bootstrapping. That's something we'll have to iron out before the solution for this issue gets committed.
pitrou commentedon Apr 14, 2012
I think this should be a blocker for 3.3.
brettcannon commentedon Apr 15, 2012
Just because I was thinking about it, I wonder if necessarily all the frozen stuff really needs to stay in import.c. I mean a frozen module is really just an entry in an array of structs that has a name of an char*[]. I don't see why one couldn't simply have a get_frozen_bytes() method to convert that char*[] into a bytes object and use that to construct a module in pure Python code.
python-dev commentedon Apr 15, 2012
New changeset d777f854a66e by Brett Cannon in branch 'default':
Issue bpo-13959: Rename imp to _imp and add Lib/imp.py and begin
http://hg.python.org/cpython/rev/d777f854a66e
brettcannon commentedon Apr 15, 2012
OK, so I have started to check this stuff in, but I think it's best to do it piecemeal. Going forward I would like to commit in units of functions being replaced, and prioritize stuff that cuts out C code (e.g. the load_*() methods, find_module(), etc.). That way it's clear that progress is being made. Obviously the best way to tell if code is hanging on just because of imp is to comment out the interface code and see what your compiler complains about in terms of dead code (or at least clang is good at this).
brettcannon commentedon Apr 15, 2012
It looks like in order to get a clear sign of what it will take to remove various parts of import.c, imp.load_module() needs to go along with imp.load_package() (since they call each other in the C code). You also have to take care of imp.reload(), but I am simplifying the C code greatly and will take care of referencing other code in the module.
55 remaining items
birkenfeld commentedon Jun 19, 2012
Which parts are still missing here?
ericsnowcurrently commentedon Jun 19, 2012
Here are the things I'm aware of:
From my discussions with Brett, there was a laundry list he was aiming to get in for 3.3 (for import stuff in general). I know much of it got done, but not all of it. Of the remainder, I'm not sure how much of that he still wants to see get in. Some of it pertains to this issue, though not all.
birkenfeld commentedon Jun 23, 2012
OK, sounds like none of it would block beta1.
brettcannon commentedon Jun 24, 2012
Sorry, been on vacation the past week.
Georg is right, nothing left is a b1 blocker, just stuff I want to get in before 3.3 ships.
birkenfeld commentedon Jun 26, 2012
Moving back to blocker for beta2.
python-dev commentedon Jul 9, 2012
New changeset efb5e6ab10f4 by Brett Cannon in branch 'default':
Issue bpo-15167 (as part of bpo-13959): imp.get_magic() is no implemented in
http://hg.python.org/cpython/rev/efb5e6ab10f4
brettcannon commentedon Jul 9, 2012
Since the final issue is just a nicety, I am considering this issue closed.
ericsnowcurrently commentedon Jul 10, 2012
hurray!
vstinner commentedon Dec 6, 2017
New changeset 672b6ba by Victor Stinner in branch 'master':
bpo-32030: pass interp to _PyImport_Init() (bpo-4736)
672b6ba