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: PyImport_ImportModuleEx always fails in 3.3 with "ValueError: level must be>= 0"
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, dmalcolm, eric.snow, georg.brandl, larry, pitrou, python-dev, vstinner
Priority: release blocker Keywords: patch

Created on 2012年08月09日 20:04 by dmalcolm, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
amazing_patch.patch vstinner, 2012年08月09日 20:54 review
Messages (11)
msg167828 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) 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) (Python committer) 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) * (Python committer) Date: 2012年08月09日 20:54
Can you please try my amazing patch?
msg167834 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) Date: 2012年08月10日 22:56
Hopefully the 3rd-party code using PyImport_ImportModuleEx will work as expected with a 'level' of 0.
History
Date User Action Args
2022年04月11日 14:57:34adminsetnosy: + larry
github: 59815
2012年08月10日 22:56:42brett.cannonsetstatus: open -> closed
messages: + msg167925

assignee: brett.cannon
resolution: fixed
stage: resolved
2012年08月10日 22:55:15python-devsetnosy: + python-dev
messages: + msg167924
2012年08月10日 15:03:30brett.cannonsetmessages: + msg167874
2012年08月09日 22:53:52eric.snowsetnosy: + eric.snow
messages: + msg167849
2012年08月09日 22:07:42vstinnersetmessages: + msg167846
2012年08月09日 21:59:42pitrousetpriority: normal -> release blocker
nosy: + georg.brandl, pitrou
messages: + msg167841

2012年08月09日 21:07:08dmalcolmsetmessages: + msg167835
2012年08月09日 21:04:16vstinnersetmessages: + msg167834
2012年08月09日 20:54:21vstinnersetfiles: + amazing_patch.patch

nosy: + vstinner
messages: + msg167833

keywords: + patch
2012年08月09日 20:53:10vstinnersetnosy: + brett.cannon
2012年08月09日 20:15:14dmalcolmsetmessages: + msg167829
2012年08月09日 20:04:08dmalcolmcreate

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