homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: importlib: ExtensionFileLoader not used to load packages from __init__.so files
Type: behavior Stage: test needed
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Arfrever, brett.cannon, eric.snow, georg.brandl, python-dev, r.david.murray, scoder
Priority: release blocker Keywords:

Created on 2012年08月07日 18:35 by scoder, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Messages (14)
msg167630 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月07日 18:35
The new importlib shows a regression w.r.t. previous CPython versions. It no longer recognises an "__init__.so" file as a package. All previous CPython versions have always tested first for an extension file before testing for a .py/.pyc file. The new importlib explicitly excludes the ExtensionFileLoader from loading packages. See importlib/_bootstrap.py, line 1579 onwards:
 1579 def _get_supported_file_loaders():
 1580 """Returns a list of file-based module loaders.
 1581 
 1582 Each item is a tuple (loader, suffixes, allow_packages).
 1583 """
 1584 extensions = ExtensionFileLoader, _imp.extension_suffixes(), False # <== bug here
 1585 source = SourceFileLoader, SOURCE_SUFFIXES, True
 1586 bytecode = SourcelessFileLoader, BYTECODE_SUFFIXES, True
 1587 return [extensions, source, bytecode]
(BTW, I'm not sure what to file this bug against - "Interpreter Core" or "Library"?)
msg167632 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月07日 19:02
Additional info: working around this bug from user code is fairly involved because some FileLoader instances are already created early at initialisation time and the overall configuration of the FileLoaders is kept in the closure of a path hook.
msg167633 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012年08月07日 19:09
I wouldn't worry about working around it from user code...finding and fixing regressions is what the beta period is for, after all.
As for which component, that's a good question with importlib ;) Both, I suppose. Not that components are used for much other than auto-nosy, and there's no auto-nosy for either interpreter core or library.
msg167634 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012年08月07日 19:17
So this was on purpose. At some point over the past five years I was working on importlib I think I was told I shouldn't support it (or something) as I explicitly remember making the decision to not support __init__.so on purpose. Plus the package specification never suggested anything but __init__.py to be used to begin with, so it isn't a bug as much as a design oversight (as is a large portion of import semantics unfortunately). But I take it Cython has been relying on this for years so that kind of kills treating this as part of import semantics cleanup.
Now to reverse this several things need to happen. First the whole third item in the tuples passed to PathFinder is unneeded (it existed purely to block extension module __init__ files) and thus should be removed where necessary. Second, the tests for extensions (test.test_importlib.extension.test_finder) need to be updated, but that gets messy as there is no __init__.so in the stdlib to test against to verify everything works. Tests could cheat and muck with PathFinder's file cache, but that is definitely not a blackbox test anymore. Don't know how best to deal with this short of an xx/__init__.so module being added to the stdlib.
And just for future reference, Stefan, since this is a direct import issue and not something to do with importlib's API I consider it an Interpreter Core bug.
And I'm glad we cache directory stat results now as changing this will have minuscule overhead compared to adding another stat call per package search without the cache.
msg167637 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月07日 19:39
Hi, thanks for bringing in the 'historical details'. It's not so much that "Cython has been relying on it" - it's entirely up to users what they compile and what not. It's more that I don't see anything being wrong with it as a feature and that it worked before.
The reason why I found it was that I'm trying to make Python benchmark suite run better in Cython, and that requires compiling all code, some of which is often hidden in the package file.
I understand that this is probably a rare real-world requirement because things can usually be made to work by moving code into a separate extension module and reimporting it from there into an __init__.py module. I guess that's why people told you to drop the feature.
There's also issue13429, which still restricts the init time compatibility of binary extensions with normal Python modules. Arguably, to make __init__.so work correctly, the "__path__" attribute would have to be set at module creation time as well, otherwise, relative imports won't work during initialisation. I guess I should update my patch in that ticket to reflect that.
msg167638 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012年08月07日 20:30
That's another thing that would need to be changed; ExtensionFileLoader would need to work for is_package() and would need to set __path__ as needed.
msg167756 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012年08月09日 03:44
Addressed question of introspecting loader_details in issue 15600. (see msg167632)
msg167891 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012年08月10日 16:45
To add a nice wrinkle to all of this, Windows debug builds use _d.pyd as the extension suffix. Notice how that doesn't start with a nice '.' to make finding the suffix easy? Yeah, that complicates ExtensionLoader.is_package() as I will have to now directly reference imp.extension_suffixes() which sucks as it hard codes what extensions are used instead of being more flexible and just ignoring that bit of detail.
msg167900 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012年08月10日 17:48
New changeset 1db6553f3f8c by Brett Cannon in branch 'default':
Issue #15576: Allow extension modules to be a package's __init__
http://hg.python.org/cpython/rev/1db6553f3f8c 
msg167933 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月11日 04:28
I understand that this is not trivial to test as part of the regression test suite. However, when I try it now I get this:
Traceback (most recent call last):
 File "<string>", line 1, in <module>
 File "__init__.py", line 8, in init my_test_package (my_test_package/__init__.c:1032)
SystemError: Parent module 'my_test_package' not loaded, cannot perform relative import
This is running Cython's "initial_file_path" test, which is meant to check that we provide a fake __path__ for the package module for the init function (as long as issue13429 isn't resolved).
The test code is here, failing in line 21 (each section is a separate file):
https://github.com/cython/cython/blob/master/tests/run/initial_file_path.srctree
I'm running the test like this:
python3 runtests.py --no-cpp --no-pyregr --no-unit --debug -vv initial_file_path
I also let it print sys.modules and the package isn't registered there yet at the time of the import.
msg167955 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012年08月11日 14:00
So that test case is hard for me to follow since I don't know what a .srctree file is and the file seems to have some magical comments in it.
If my assumptions are correct you are generating a C file that does a relative import in the extension module's init function? Has that actually worked in the past? The error you are seeing suggests that the module is not being inserted into sys.modules before executing the body which I have no control over since that's the purview of imp.load_dynamic(). If you skip the relative imports does the import work otherwise?
msg167956 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012年08月11日 14:04
The reason I ask this is that if I simply move e.g. audioop.so to audioop/__init__.so everything works fine in terms of being able to import that module as a package.
msg167959 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月11日 14:37
Hmm, yes, sounds more like a separate issue than something to add to this ticket. It worked before (starting with Py2.5, which was the first Python version to support relative imports) and only stopped working in 3.3 now.
The .srctree test file basically just unfolds into a file system hierarchy with each file named as in the preceding "magical" comment. The commands in the lines before the first comment line are then executed for the test (usually: build module with distutils, import it, do something with it).
msg167968 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2012年08月11日 15:02
I've created issue15623, so that we can keep this one as fixed.
History
Date User Action Args
2022年04月11日 14:57:33adminsetnosy: + georg.brandl
github: 59781
2012年08月11日 15:02:43scodersetstatus: open -> closed
resolution: fixed
messages: + msg167968
2012年08月11日 14:37:39scodersetmessages: + msg167959
2012年08月11日 14:34:34Arfreversetnosy: + Arfrever
2012年08月11日 14:04:17brett.cannonsetmessages: + msg167956
2012年08月11日 14:00:27brett.cannonsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg167955

stage: resolved -> test needed
2012年08月11日 04:28:43scodersetmessages: + msg167933
2012年08月10日 17:48:52brett.cannonsetstatus: open -> closed
assignee: brett.cannon
resolution: fixed
stage: needs patch -> resolved
2012年08月10日 17:48:03python-devsetnosy: + python-dev
messages: + msg167900
2012年08月10日 16:46:00brett.cannonsetmessages: + msg167891
2012年08月09日 03:44:38eric.snowsetmessages: + msg167756
2012年08月07日 20:30:16brett.cannonsetmessages: + msg167638
2012年08月07日 19:39:13scodersetmessages: + msg167637
components: + Library (Lib), - Interpreter Core
2012年08月07日 19:17:30brett.cannonsetmessages: + msg167634
components: - Library (Lib)
2012年08月07日 19:09:31r.david.murraysetpriority: normal -> release blocker

nosy: + r.david.murray
messages: + msg167633

components: + Interpreter Core
stage: needs patch
2012年08月07日 19:02:30scodersetmessages: + msg167632
2012年08月07日 18:42:33eric.snowsetnosy: + eric.snow
2012年08月07日 18:42:23eric.snowsetnosy: + brett.cannon
2012年08月07日 18:35:49scodercreate

AltStyle によって変換されたページ (->オリジナル) /