Issue 15623: Init time relative imports no longer work from init.so modules (original) (raw)
Created on 2012-08-11 15:02 by scoder, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (20)
Author: Stefan Behnel (scoder) * 
Date: 2012-08-11 15:02
Since CPython 2.5, relative imports could be used from init.so package modules. This stopped working in CPython 3.3.
This is a follow-up ticket to .
This feature is exercised by Cython's initial_file_path test, which now gives this result:
Traceback (most recent call last): File "", line 1, in 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
It is meant to check that Cython provides a fake path for the package module for the init function (as long as isn't resolved). With that, relative imports worked before.
The test code is here, failing in line 21 (each section is a separate file), when it tries to do a relative import at the module level, i.e. at module init time.
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 tried printing sys.modules and the package really isn't registered there yet at the time of the import.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-11 23:35
The trigger of that exception is importlib._bootstrap._sanity_check() (http://hg.python.org/cpython/file/5e025dc7d728/Lib/importlib/_bootstrap.py#l1466). It's called very early on to verify certain things, including that the parent package is already loaded when importing a submodule. It's that last bit that's failing.
If you look at 3.2 vs. 3.3 for imp.load_dynamic() which does extension module loading for importlib, there is essentially no change, even as far as looking into Python/importdl.c:_PyImport_LoadDynamicModule() (http://hg.python.org/cpython/file/5e025dc7d728/Python/import.c#l1773 vs. http://hg.python.org/cpython/file/3654c711019a/Python/import.c#l3446).
The problem is that the check for the parent module is also in Python/import.c from 3.2 so this isn't a new check. Is it possible that Cython is doing something differently now that it didn't do before? I know you said this worked in 3.2 and earlier, Stefan, but was that with the same version of Cython? Did the actual C call and setup for that call change? Otherwise I can't think of how anything specifically changed between 3.2 and 3.3 that would fundamentally change this.
Author: Stefan Behnel (scoder) *
Date: 2012-08-12 04:35
We are continuously testing Cython against all CPython versions starting from 2.4, so I can assure you that it's still working for all other versions. Here's our CI server:
https://sage.math.washington.edu:8091/hudson/job/cython-devel-tests/618/
I just tried it and it definitely works for me in 3.2 and prints the right module names.
I attached the unfolded test directory. The only changes I applied were to change the setup.py script so that you can build it without Cython, and to replace the absolute paths stored in the C files by relative paths (which happily continue to work). Just run "python3.3 setup.py build_ext -i" and then "PYTHON=python3.3 ./test.sh".
Note that the output for "FILE" will be the original Python source file. Cython sets this at init time because CPython fails to provide the correct import file path at that point. The value is actually written down verbatimly in the C sources.
It prints sys.modules at module init time, so you can see that it contains "my_test_package" in Py<3.3 but does not contain it in Py3.3.
The code for the relative import in init.c starts in line 1097. It actually calls "import()" internally.
Author: Eric Snow (eric.snow) *
Date: 2012-08-12 06:18
I was able to reproduce the error using a fresh build from tip (34d5ec8a1019):
/tmp/test$ PYTHON=/opt/python3.3/bin/python3 ./test.sh ['main', '_bisect', '_codecs', '_collections', '_frozen_importlib', '_functools', '_heapq', '_imp', '_io', '_locale', '_sre', '_sysconfigdata', '_thread', '_warnings', '_weakref', '_weakrefset', 'abc', 'bisect', 'builtins', 'codecs', 'collections', 'collections.abc', 'copyreg', 'encodings', 'encodings.aliases', 'encodings.latin_1', 'encodings.utf_8', 'errno', 'functools', 'genericpath', 'heapq', 'io', 'itertools', 'keyword', 'locale', 'marshal', 'operator', 'os', 'os.path', 'posix', 'posixpath', 're', 'reprlib', 'signal', 'site', 'sre_compile', 'sre_constants', 'sre_parse', 'stat', 'sys', 'sysconfig', 'weakref', 'zipimport'] Traceback (most recent call last): File "", line 1, in File "init.py", line 11, in init my_test_package (my_test_package/init.c:1106) SystemError: Parent module 'my_test_package' not loaded, cannot perform relative import
Author: Eric Snow (eric.snow) *
Date: 2012-08-12 06:21
The files, post-run:
$ find /tmp/test/ /tmp/test/ /tmp/test/my_test_package /tmp/test/my_test_package/init.cpython-33dm.so /tmp/test/my_test_package/a.cpython-33dm.so /tmp/test/my_test_package/a.c /tmp/test/my_test_package/a.py /tmp/test/my_test_package/init.py /tmp/test/my_test_package/init.c /tmp/test/test.tgz /tmp/test/build /tmp/test/build/temp.linux-x86_64-3.3-pydebug /tmp/test/build/temp.linux-x86_64-3.3-pydebug/my_test_package /tmp/test/build/temp.linux-x86_64-3.3-pydebug/my_test_package/a.o /tmp/test/build/temp.linux-x86_64-3.3-pydebug/my_test_package/init.o /tmp/test/test.sh /tmp/test/setup.py
Author: Eric Snow (eric.snow) *
Date: 2012-08-12 06:31
Traceback when run verbosely:
Traceback (most recent call last): File "", line 1, in File "", line 1529, in _find_and_load File "", line 1496, in _find_and_load_unlocked File "", line 583, in _check_name_wrapper File "", line 498, in set_package_wrapper File "", line 511, in set_loader_wrapper File "", line 1109, in load_module File "", line 310, in _call_with_frames_removed File "init.py", line 11, in init my_test_package (my_test_package/init.c:1106) SystemError: Parent module 'my_test_package' not loaded, cannot perform relative import
Author: Eric Snow (eric.snow) *
Date: 2012-08-12 07:11
in _init.c:963+, prior to executing the module proper (Python2 and PyPy handling removed):
/--- Module creation code ---/ __pyx_m = PyModule_Create(&__pyx_moduledef); if (!__pyx_m) {...}; __pyx_b = PyImport_AddModule(__Pyx_NAMESTR(__Pyx_BUILTIN_MODULE_NAME)); if (!__pyx_b) {...}; if (__Pyx_SetAttrString(__pyx_m, "builtins", __pyx_b) < 0) {...}; /--- Initialize various global constants etc. ---/ if (unlikely(__Pyx_InitGlobals() < 0)) {...} if (__pyx_module_is_main_my_test_package) { if (__Pyx_SetAttrString(__pyx_m, "name", pyx_n_s____main) < 0) {...}; } if (__Pyx_SetAttrString(__pyx_m, "file", __pyx_kp_u_6) < 0) {...}; __pyx_t_1 = Py_BuildValue("[O]", __pyx_kp_u_7); if (unlikely(!__pyx_t_1)) {...} __Pyx_GOTREF(__pyx_t_1); if (__Pyx_SetAttrString(__pyx_m, "path", __pyx_t_1) < 0) {...}; __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0; /--- Builtin init code ---/ if (unlikely(__Pyx_InitCachedBuiltins() < 0)) {...} /--- Constants init code ---/ if (unlikely(__Pyx_InitCachedConstants() < 0)) {...}
Author: Stefan Behnel (scoder) *
Date: 2012-08-12 07:35
That's the module init code, yes, including the setting of file and path that I mentioned (due to ). Setting at least one of the two is required in previous CPython versions to make relative imports work.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-12 16:39
I don't have time to delve into C debugging right now, but the question is what changed between 3.2 and 3.3 in terms of when an extension module is inserted into sys.modules since that is the issue ATM. Probably some time in gdb starting in the module's init code and following it to when it is inserted into sys.modules should answer the question (hopefully).
Author: Eric Snow (eric.snow) *
Date: 2012-08-12 17:25
This smells like a case of where sys.modules got replaced by another mapping, but import.c continues using interp->modules. I won't be able to investigate until tomorrow.
Author: Meador Inge (meador.inge) *
Date: 2012-08-13 14:30
I debugged this a bit by comparing the behavior of 3.3 against 3.2. For both cases I used the following code and debugged it in Python via pdb*:
import importlib importlib.import('my_test_package')
ISTM that the difference in behavior is a result of what loader gets chosen for the initial import 'my_test_package'.
With 3.2 a importlib._bootstrap._SourceFileLoader loader gets created against 'my_test_package/init.py'. This works fine because _SourceFileLoader fixes up sys.modules when it loads.
With 3.3 a _frozen_importlib.ExtensionFileLoader loader gets created against 'my_test_package/init.so'. This doesn't work because ExtensionFileLoader does not fixup sys.module when it loads.
I hope that helps some.
- Which was a real pain for 3.3 since you are debugging the frozen importlib. You get line numbers at least, but it would be really nice if you could tell pdb a source file to use when you are dealing with bytecode only objects and\or you could disassemble the bytecode from pdb.
Author: Eric Snow (eric.snow) *
Date: 2012-08-13 15:29
That's helpful, Meador. With regards to the following, there's more to the story:
With 3.3 a _frozen_importlib.ExtensionFileLoader loader gets created against 'my_test_package/init.so'. This doesn't work because ExtensionFileLoader does not fixup sys.module when it loads.
In the example Stefan provided, look at my_test_package/init.c:921. You'll find PyInit_my_test_package(). The module creation code from comes from that function. There PyImport_AddModule gets called, which adds the module to interp->modules. I haven't had time to run this through gdb to see what's happening sys.modules in this case.
In general, I'm not familiar enough with extension modules to know if they are actually responsible for adding themselves to sys.modules. I do see a number of places in Python/import.c and friends that touch sys.modules.
If importlib is in charge of adding it, though, I'd think that ExtensionFileLoader.load_module would need the module_for_loader decorator.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-13 17:18
So imp.load_dynamic() does add a module to sys.modules (I'm using Python 3.2 here because that's what I have access to, but I verified this yesterday at home)::
Python 3.2.3 (default, May 3 2012, 15:51:42) [GCC 4.6.3] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import imp import sys 'resource' in sys.modules False mod = imp.load_dynamic('resource', '/usr/lib/python3.2/lib-dynload/resource.cpython-32mu.so') mod <module 'resource' from '/usr/lib/python3.2/lib-dynload/resource.cpython-32mu.so'> 'resource' in sys.modules True
IOW it is not needed for ExtensionFileLoader to add the module explicitly to sys.modules.
As for Meador's notice that init.py was being imported, I believe Stefan is saying it should work without that file. So deleting init.py and only having init.so in Python 3.2 should work. If not then this has been a false alarm.
Author: Meador Inge (meador.inge) *
Date: 2012-08-13 17:31
On Mon, Aug 13, 2012 at 12:18 PM, Brett Cannon <report@bugs.python.org> wrote:
So deleting init.py and only having init.so in Python 3.2 should work.
It doesn't work:
quicksilver:bugs meadori$ python.exe --version Python 3.2.3+ quicksilver:bugs meadori$ PYTHON=python.exe ./test.sh ['main', '_abcoll', '_bisect', '_codecs', '_collections', '_functools', '_heapq', '_io', '_locale', '_sre', '_thread', '_weakref', '_weakrefset', 'abc', 'bisect', 'builtins', 'codecs', 'collections', 'copyreg', 'encodings', 'encodings.aliases', 'encodings.latin_1', 'encodings.utf_8', 'errno', 'functools', 'genericpath', 'heapq', 'io', 'itertools', 'keyword', 'linecache', 'locale', 'my_test_package', 'operator', 'os', 'os.path', 'posix', 'posixpath', 're', 'reprlib', 'signal', 'site', 'sre_compile', 'sre_constants', 'sre_parse', 'stat', 'sys', 'sysconfig', 'token', 'tokenize', 'traceback', 'weakref', 'zipimport'] FILE: my_test_package/init.py PATH: ['my_test_package'] Real package file used: my_test_package/init.so [36494 refs] quicksilver:bugs meadori$ rm my_test_package/init.py quicksilver:bugs meadori$ PYTHON=python.exe ./test.sh Traceback (most recent call last): File "", line 1, in ImportError: No module named my_test_package [36287 refs]
Author: Stefan Behnel (scoder) *
Date: 2012-08-13 17:59
Interesting. I didn't know that. The .py file is always installed automatically next to the .so file by distutils.
Here's what strace gives me:
Python 2.7:
stat("my_test_package", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 stat("my_test_package/init.py", {st_mode=S_IFREG|0664, st_size=557, ...}) = 0 stat("my_test_package/init", 0x7fffdc9dd320) = -1 ENOENT (No such file or directory) open("my_test_package/init.so", O_RDONLY) = 3 open("my_test_package/init.so", O_RDONLY|O_CLOEXEC) = 4
Python 3.2:
stat("my_test_package", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 stat("my_test_package/init.py", {st_mode=S_IFREG|0664, st_size=557, ...}) = 0 stat("my_test_package/init", 0x7fff9d99d700) = -1 ENOENT (No such file or directory) stat("my_test_package/init.cpython-32mu.so", {st_mode=S_IFREG|0775, st_size=82517, ...}) = 0 open("my_test_package/init.cpython-32mu.so", O_RDONLY) = 3 open("my_test_package/init.cpython-32mu.so", O_RDONLY|O_CLOEXEC) = 4
Python 3.3:
stat("./my_test_package", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 stat("./my_test_package/init.cpython-33dm.so", {st_mode=S_IFREG|0775, st_size=36119, ...}) = 0 open("./my_test_package/init.cpython-33dm.so", O_RDONLY|O_CLOEXEC) = 3
That's the difference then.
Ok, I think we'll have to emulate this for older CPython versions as well for the case that the .py file is not there. So it's likely best to let Cython register the package in sys.modules at init time, right after calling AddModule().
Still - can this be implemented in CPython for 3.3? Or 3.4, given that it might appear like a new feature? There shouldn't be all that much to change.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-13 19:01
My guess is import.c is noticing the init.py, creating the module for the package, and then somehow it cascades into importing init.so which essentially does a reload on the module object that is already in sys.modules and thus doesn't cause the parent module check to fail.
The problem with that is those semantics suck as that assumes cascading semantics which would cross the finder/loader barrier. So we can't keep the exact semantics in Python 3.3.
But I do see three possible solutions to fixing this. One is testing if having ExtensionFileLoader insert a practically empty module into sys.modules before calling imp.load_dynamic(). This would actually be easy to test by using importlib.util.module_for_loader on ExtensionFileLoader.load_module(). This might be a slight change in semantics (and assumes imp.load_dynamic() will do a reload as appropriate instead of blindly creating a new module), but then if anyone is going to notice it will be Cython so if it works in this case and Cython doesn't fail further it is probably safe.
The second option is to tweak the extension module initialization process to make sure the module being initialized is inserted into sys.modules early enough and still cleaned up properly. The question, though, is what is it doing now as the module is not passed in directly into the PyInit function but by PyModule_Create instead and I don't know who handles cleanup of the module if something fails and the PyInit returns NULL.
Third is having Cython tweak itself for 3.3 to just directly inject the module object into sys.modules before it does any relative imports. It's the least graceful solution and puts the onus on Cython for a change in semantics, but I don't see why that would break anything.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-14 16:29
I had a minute free so I just tried inserting an empty module into sys.modules and then importing an extension module to see if it would get reused. It doesn't. imp.load_dynamic() actually just blindly overwrites what is in sys.modules. I'm willing to bet it just assumes whatever is in the special extensions module cache is the canonical module and just re-inserts blindly into sys.modules.
So that leaves needing to diagnose where Python 3.3 inserts an extension into sys.modules and if it can somehow be moved up. Probably should also see how the heck 3.2 is doing it to know where the difference occurs to make sure that it isn't some silly oversight somehow.
Author: Stefan Behnel (scoder) *
Date: 2012-08-15 06:15
I tried it and it works to just insert the package module into sys.modules after creation if it's not there yet:
#if PY_MAJOR_VERSION < 3 __pyx_m = Py_InitModule4(__Pyx_NAMESTR("my_test_package"), __pyx_methods, 0, 0, PYTHON_API_VERSION); #else __pyx_m = PyModule_Create(&__pyx_moduledef); #endif if (!__pyx_m) {...};
PyObject *modules = PyImport_GetModuleDict(); if (unlikely(!modules)) {...} if (!PyDict_GetItemString(modules, "my_test_package")) if (unlikely(PyDict_SetItemString(modules, "my_test_package", __pyx_m) < 0)) {...}
So this is a work-around that we can use in Cython in any case.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-15 14:48
I'm glad the work-around at least works, but I would still like to find out what the heck 3.2 is doing to see if it's reasonable to replicate in 3.3.
Author: Brett Cannon (brett.cannon) *
Date: 2012-08-17 19:51
So I can't reproduce under 3.2. First off, building the example code in test.tgz fails thanks to __Py_ZeroStruct not being found (this is with using both an installed Python 3.2 and a checkout build).
Second, when I just make audioop a package in Python 3.2 it still grabs the init.py file, so I can't reproduce that way either.
Third, I hand-traced the import code starting in load_module() in Python 3.2 and if you follow::
import.c:load_module() -> importdl.c:_PyImport_GetDynLoadFunc() -> import.c:_PyImport_FixupExtensionUnicode()
you will find where a new module gets set in sys.modules (through PyImport_GetModuleDict()) and it's after the PyInit function for the extension module is called. So if there is some magical path that is deep in import.c that is setting the module in sys.modules when there is a init.py next to an init.so I will need someone who has actually made this work with an empty init.py figure out how it's all happening since _PyImport_GetDynLoadFunc() would short-circuit if the module was already in sys.modules and entirely skip executing the PyInit for the extension module.
Fourth, if you go with the work-around, Stefan, just make sure that on error anywhere in your PyInit you remove the module from sys.modules that you injected.
Because of all of this I am making this pending as "won't fix". If someone can figure out what is happening in Python 3.2 I will leave it open until we decide how to handle this, else I will close this before rc1.
History
Date
User
Action
Args
2022-04-11 14:57:34
admin
set
github: 59828
2012-11-17 16:45:31
brett.cannon
set
status: pending -> closed
2012-08-17 19:51:21
brett.cannon
set
status: open -> pending
resolution: wont fix
messages: +
2012-08-15 14:48:28
brett.cannon
set
messages: +
2012-08-15 06:15:43
scoder
set
messages: +
2012-08-14 16:29:57
brett.cannon
set
messages: +
2012-08-13 19:01:12
brett.cannon
set
messages: +
2012-08-13 17:59:38
scoder
set
messages: +
2012-08-13 17:31:15
meador.inge
set
messages: +
2012-08-13 17🔞41
brett.cannon
set
messages: +
2012-08-13 15:29:15
eric.snow
set
messages: +
2012-08-13 14:30:03
meador.inge
set
messages: +
2012-08-12 21:34:29
meador.inge
set
nosy: + meador.inge
2012-08-12 17:41:46
georg.brandl
set
nosy: + georg.brandl
2012-08-12 17:25:49
eric.snow
set
messages: +
2012-08-12 16:39:49
brett.cannon
set
messages: +
2012-08-12 09:42:28
georg.brandl
set
priority: normal -> deferred blocker
2012-08-12 07:35:14
scoder
set
messages: +
2012-08-12 07:11:32
eric.snow
set
messages: +
2012-08-12 06:31:08
eric.snow
set
messages: +
2012-08-12 06:21:47
eric.snow
set
messages: +
2012-08-12 06:19:00
eric.snow
set
nosy: + eric.snow
messages: +
2012-08-12 04:35:33
scoder
set
files: + test.tgz
messages: +
2012-08-11 23:35:17
brett.cannon
set
nosy: + brett.cannon
messages: +
2012-08-11 15:04:14
Arfrever
set
nosy: + Arfrever
2012-08-11 15:02:04
scoder
create