| msg167828 - (view) |
Author: Dave Malcolm (dmalcolm)  |
Date: 2012-08-09 20:04 |
| I've been testing various 3rd-party python code against 3.3b1, and see ValueError: level must be >= 0 exceptions where C code is using PyImport_ImportModuleEx. PyImport_ImportModuleEx reads as 63 #define PyImport_ImportModuleEx(n, g, l, f) \ 64 PyImport_ImportModuleLevel(n, g, l, f, -1) within http://hg.python.org/cpython/file/aaa68dce117e/Include/import.h as of now (2012-08-09) Within PyImport_ImportModuleLevel there's this check: 1280 if (level < 0) { 1281 PyErr_SetString(PyExc_ValueError, "level must be >= 0"); 1282 goto error; 1283 } which thus always fires. So it would seem that currently any usage of PyImport_ImportModuleEx will fail. |
|
|
| msg167829 - (view) |
Author: Dave Malcolm (dmalcolm) |
Date: 2012-08-09 20:15 |
| (FWIW, this was observed when compiling pygobject-3.3.4 against Python-3.3.0b1) |
|
|
| msg167833 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-08-09 20:54 |
| Can you please try my amazing patch? |
|
|
| msg167834 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-08-09 21:04 |
| Oh, I didn't realize that the documentation says that the default value is -1. http://docs.python.org/library/functions.html#__import__ "level specifies whether to use absolute or relative imports. The default is -1 which indicates both absolute and relative imports will be attempted. 0 means only perform absolute imports. Positive values for level indicate the number of parent directories to search relative to the directory of the module calling __import__()." We should probably tolerate -1, or just drop the exception. |
|
|
| msg167835 - (view) |
Author: Dave Malcolm (dmalcolm) |
Date: 2012-08-09 21:07 |
| On Thu, 2012-08-09 at 21:04 +0000, STINNER Victor wrote: > STINNER Victor added the comment: > > Oh, I didn't realize that the documentation says that the default value is -1. > http://docs.python.org/library/functions.html#__import__ > > "level specifies whether to use absolute or relative imports. The default is -1 which indicates both absolute and relative imports will be attempted. 0 means only perform absolute imports. Positive values for level indicate the number of parent directories to search relative to the directory of the module calling __import__()." That's the python 2 documentation The 3.3 docs here: http://docs.python.org/dev/library/functions.html#__import__ say "Changed in version 3.3: Negative values for level are no longer supported (which also changes the default value to 0)." > We should probably tolerate -1, or just drop the exception. |
|
|
| msg167841 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-08-09 21:59 |
| Sounds like a rather annoying regression. Changing the macro's expansion would be good enough IMO. |
|
|
| msg167846 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-08-09 22:07 |
| > Sounds like a rather annoying regression. PyImport_ImportModuleLevel() is part of the stable API. Is it an acceptable to not only change the default value but also fail with the previous *default* value? Can't we just drop the check "level < 0"? |
|
|
| msg167849 - (view) |
Author: Eric Snow (eric.snow) * |
Date: 2012-08-09 22:53 |
| > Changing the macro's expansion would be good enough IMO. Sounds good to me. > PyImport_ImportModuleLevel() is part of the stable API... From what I understand, as long as the function header has not changed, the stable ABI is still stable. > Can't we just drop the check "level < 0"? In Python 3 a negative value makes no sense, since there are no accommodations for implicit relative imports. The fact that builtins.__import__() accommodated -1 still was an oversight that was corrected in 3.3. Looks like this is just one bit that got missed. |
|
|
| msg167874 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2012-08-10 15:03 |
| OK, the macro expansion should get fixed, a versionchanged should probably be added to the C API docs (for PyImport_ImportModuleLevel()), and a line in What's New for porting C code should be added. We can't go back to -1, as Eric said, because it makes no sense anymore since you can't syntactically do an import that has -1 level semantics in Python 3. The fact that __import__ accepted a negative level was a bug that went unnoticed up until this point since so few people import modules programmatically and want implicit relative imports. |
|
|
| msg167924 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-08-10 22:55 |
| New changeset 9804aec74d4a by Brett Cannon in branch 'default': Issue #15610: The PyImport_ImportModuleEx macro now calls http://hg.python.org/cpython/rev/9804aec74d4a |
|
|
| msg167925 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2012-08-10 22:56 |
| Hopefully the 3rd-party code using PyImport_ImportModuleEx will work as expected with a 'level' of 0. |
|
|