msg245696 - (view) |
Author: Armin Rigo (arigo) *  |
Date: 2015-06-23 17:40 |
A regression in 3.5: if we use custom objects as modules (like some projects do), then these custom objects may not have an attribute called "__name__". There is new code in 3.5 added for issue #17636 which tries sometimes to read the "__name__" of a module when executing an import statement, and if this leads to an AttributeError, it will get propagated.
I imagine this could break real code using "try: except ImportError:". It _does_ break the tests of the "cffi" project, which try to check that some import of a nonexisting name correctly gives ImportError. Now it gives AttributeError("__name__") instead.
|
msg245699 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-06-23 19:52 |
What's a sensible approach to ameliorate the problem? Gracefully muddle through without a __name__ on the imported object?
|
msg245725 - (view) |
Author: Armin Rigo (arigo) *  |
Date: 2015-06-24 08:29 |
I'd guess so: if the PyObject_GetAttrId() fails, then ignore the rest of the code that was added in https://hg.python.org/cpython/rev/fded07a2d616 and jump straight to ``PyErr_Format(PyExc_ImportError, ...)``.
|
msg245726 - (view) |
Author: Armin Rigo (arigo) *  |
Date: 2015-06-24 08:35 |
Also, if we want to be paranoid, the _PyObject_GetAttrId() can return anything, not necessarily a string object. This would make the following PyUnicode_FromFormat() fail. So maybe you also want to overwrite failures in PyUnicode_FromFormat() with the final ImportError. (It actually looks like a good--if convoluted--use case for exception chaining at the C level...)
|
msg245869 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2015-06-26 21:05 |
I view having a string __name__ attribute as part of quacking like a module (or class or function). Hence pseudonames like '<lambda>' (lambda expression) or <pyshell#n> (the module name for interactive input, where n is the line number). Is there a good reason for custom 'module' objects to not have a name? If so, the 'graceful muddle' precedent is to give them a pseudoname, such as <module>, perhaps with a prefix or suffix.
|
msg246112 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-07-03 00:52 |
This is
a) marked as release blocker, and
b) is assigned to nobody.
This is not tenable.
While I want this fixed, I'm not going to hold up beta 3 for it.
|
msg248345 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-09 23:39 |
Here is a patch that adds a test, fixes the issue, and for good measure stops assuming that __name__ will always be a string.
|
msg248361 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-10 16:50 |
Larry, does this warrant going into 3.5.0?
|
msg248368 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-08-10 20:44 |
I would like the fix in 3.5. However, I'm not qualified to review the code. Can you get a qualified reviewer in to look over the code?
Once someone suitable has reviewed it, I'll accept a pull request (pasted in here naturally).
|
msg248369 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2015-08-10 21:08 |
patch LGTM. Presumably the divergence between importlib (in _handle_fromlist) and import.c was strictly accidental (i.e. lack of test coverage).
|
msg248371 - (view) |
Author: Armin Rigo (arigo) *  |
Date: 2015-08-10 21:24 |
Patch LGTM too. Optionally a test is needed for each of the other cases in it too, but please don't cause that comment to stop it from getting in.
|
msg248373 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-10 21:36 |
Thanks for the reviews, Eric and Armin. I will get the patch in 3.5.0, 3.5.1, and 3.6 sometime this week and then open another issue for adding more tests for the other branches in the code.
|
msg248377 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-08-10 22:51 |
Yes, Eric and Armin are both qualified reviewers in my book. You have my blessing to send a pull request.
Thanks, everybody!
|
msg248384 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-08-11 00:29 |
My Bitbucket repo is now public.
https://bitbucket.org/larry/cpython350
|
msg248436 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-12 01:10 |
Created https://bitbucket.org/larry/cpython350/pull-requests/2/issue-24492-make-sure-that-from-import/diff without the PyUnicode_FromFormat() change as I realized that was conflating two changes in one patch and it wasn't even being tested (verified all tests still pass and a quick check in the interpreter shows ImportError is still raised).
Larry, let me know when you have accepted the PR and I will commit to 3.5 and default on hg.python.org.
I also created http://bugs.python.org/issue24846 to track adding more tests for the code.
|
msg248541 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-13 21:45 |
I noticed you accepted the PR on Bitbucket, Larry. Should I consider your part done and I can now pull the commit into the 3.5 and default branches on hg.python.org?
|
msg248551 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2015-08-14 01:44 |
Yep. This time I have foisted nearly all the work, including the forward-merging, onto y'all.
*sits back, sips iced coffee*
|
msg248598 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-08-14 18:10 |
New changeset cf3a62a8d786 by Brett Cannon in branch '3.5':
Issue #24492: make sure that ``from ... import ...` raises an
https://hg.python.org/cpython/rev/cf3a62a8d786
New changeset bbe6b215df5d by Brett Cannon in branch '3.5':
Merge from 3.5.0 for issue #24492
https://hg.python.org/cpython/rev/bbe6b215df5d
New changeset b0a6bba16b9c by Brett Cannon in branch 'default':
Merge from 3.5 for issue #24492
https://hg.python.org/cpython/rev/b0a6bba16b9c
|
msg248599 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-08-14 18:11 |
I have merged my 3.5.0 patch into 3.5 and default, so that should fix the issue Armin and CFFI was having.
|
msg249072 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-08-24 20:01 |
New changeset 5a9ac801f9b4 by larry in branch '3.5':
Merged in brettcannon/cpython350/3.5 (pull request #2)
https://hg.python.org/cpython/rev/5a9ac801f9b4
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:18 | admin | set | nosy:
+ ned.deily github: 68680
|
2015-08-24 20:01:24 | python-dev | set | messages:
+ msg249072 |
2015-08-14 18:11:34 | brett.cannon | set | status: open -> closed messages:
+ msg248599
assignee: larry -> brett.cannon resolution: fixed stage: commit review -> resolved |
2015-08-14 18:10:29 | python-dev | set | nosy:
+ python-dev messages:
+ msg248598
|
2015-08-14 01:44:11 | larry | set | messages:
+ msg248551 |
2015-08-13 21:45:26 | brett.cannon | set | messages:
+ msg248541 |
2015-08-12 01:10:28 | brett.cannon | set | assignee: brett.cannon -> larry messages:
+ msg248436 |
2015-08-12 01:10:00 | brett.cannon | link | issue24846 dependencies |
2015-08-11 00:29:18 | larry | set | messages:
+ msg248384 |
2015-08-10 22:51:34 | larry | set | messages:
+ msg248377 |
2015-08-10 21:36:34 | brett.cannon | set | priority: deferred blocker -> release blocker
messages:
+ msg248373 |
2015-08-10 21:24:55 | arigo | set | messages:
+ msg248371 |
2015-08-10 21:08:36 | eric.snow | set | messages:
+ msg248369 stage: patch review -> commit review |
2015-08-10 20:44:36 | larry | set | assignee: larry -> brett.cannon |
2015-08-10 20:44:09 | larry | set | messages:
+ msg248368 |
2015-08-10 16:50:29 | brett.cannon | set | assignee: brett.cannon -> larry messages:
+ msg248361 |
2015-08-09 23:40:00 | brett.cannon | set | files:
+ issue24492.diff keywords:
+ patch messages:
+ msg248345
stage: needs patch -> patch review |
2015-08-09 18:47:40 | brett.cannon | set | assignee: brett.cannon versions:
+ Python 3.6 |
2015-07-21 07:05:36 | ethan.furman | set | nosy:
- ethan.furman
|
2015-07-03 00:52:54 | larry | set | priority: release blocker -> deferred blocker
messages:
+ msg246112 |
2015-06-26 21:07:54 | terry.reedy | set | stage: needs patch |
2015-06-26 21:05:19 | terry.reedy | set | nosy:
+ terry.reedy messages:
+ msg245869
|
2015-06-24 08:35:17 | arigo | set | messages:
+ msg245726 |
2015-06-24 08:29:24 | arigo | set | messages:
+ msg245725 |
2015-06-23 19:52:50 | larry | set | messages:
+ msg245699 |
2015-06-23 18:16:29 | r.david.murray | set | priority: normal -> release blocker |
2015-06-23 17:45:30 | arigo | set | files:
+ test_case.py |
2015-06-23 17:43:59 | ethan.furman | set | nosy:
+ brett.cannon, ncoghlan, ethan.furman, eric.snow
|
2015-06-23 17:40:13 | arigo | create | |