classification
Title: _frozen_importlib should not have a __file__ attribute
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Arfrever, Zvezdoslovec, brett.cannon, eric.snow, ncoghlan, python-dev
Priority: normal Keywords: patch

Created on 2014-03-16 05:42 by ncoghlan, last changed 2014-04-04 20:22 by Arfrever. This issue is now closed.

Files
File name Uploaded Description Edit
no_frozen__file__.diff brett.cannon, 2014-03-21 18:36 review
no_frozen__file__.diff brett.cannon, 2014-03-28 13:32 review
Messages (15)
msg213719 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-03-16 05:42
As part of the PEP 451 changes, frozen imports were updated to avoid setting the __file__ attribute, as that attribute makes no sense for frozen modules.

However, _frozen_importlib isn't loaded through the normal mechanisms (they don't exist yet!), so it is still getting a __file__ attribute set. (This was discovered while investigating issue 20884)

This is a fairly harmless state of affairs, so I suggest we leave it alone in the 3.4 maintenance releases, and just tidy it up in 3.5. However, I'm also open to fixing it in 3.4.1.
msg214399 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-03-21 18:36
Here is a patch to change PyImport_ImportFrozenModuleObject() to not set __file__. Had to refactor some things as PyImport_ExecCodeModuleObject() was setting __file__ no matter what, so to avoid it even being used during import I had to change some things.
msg214926 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-03-27 01:11
Review posted.
msg215044 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-03-28 13:32
New version based on Eric's review.
msg215045 - (view) Author: Boštjan Mejak (Zvezdoslovec) Date: 2014-03-28 13:41
This patch can easily be incorporated into Python 3.4.1. It doesn't break backwards compatibility.
msg215046 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-03-28 13:49
It actually does break backwards-compatibility as an attribute will disappear on frozen modules for anyone who uses the imp API. While the chances of actually breaking someone's code is very small, there is still a chance. But I'm only -0 on backporting so other core devs could convince me that I'm being overly cautious.
msg215053 - (view) Author: Boštjan Mejak (Zvezdoslovec) Date: 2014-03-28 15:40
I would say go for it. I don't think anyone's code would break.

Anyway, if people see that something in the new version of Python breaks their code, they down-grade; otherwise they read the changelog and fix their code accordingly. So no worries here. :)
msg215060 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-03-28 17:28
Latest patch LGTM.
msg215061 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-03-28 17:57
As I've thought about it, I've gone from -0 to +1 on applying this to both 3.4 and 3.5.  The bug and fix should impact very little* code.  In fact, I doubt anyone would have noticed if Nick hadn't. :)  It would be nice to have this right in 3.4.

* Effectively this is limited to consumers of imp.init_frozen(), PyImport_ImportFrozenModule(), and PyImport_ImportFrozenModule().  That includes importing frozen importlib and Py_FrozenMain().
msg215064 - (view) Author: Boštjan Mejak (Zvezdoslovec) Date: 2014-03-28 18:31
I'm setting Python 3.4 to also be affected by this issue. Thank you, Brett, for making the patch. I hope this gets commited soon.
msg215090 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-03-28 23:18
Yeah, as noted in my original comment, +0 for "it's just a bug" from me, as it was a quirk of the way this particular frozen module is initialised.
msg215122 - (view) Author: Boštjan Mejak (Zvezdoslovec) Date: 2014-03-29 07:59
Haven't we forget something in the patch for the docs?

+   .. versionchanged:: 3.5

+      The ``__file__`` attribute is no longer set on the module.

Where's 3.4 mentioned? Should we add the mention? Like...
+   .. versionchanged:: 3.4
msg215394 - (view) Author: Boštjan Mejak (Zvezdoslovec) Date: 2014-04-02 18:57
No one interested in fixing this anymore, despite the patch and all?
msg215395 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-04-02 19:11
My Python free time is on Fridays, which is when I plan to make a final call and either apply to Python 3.4 and 3.5 or just Python 3.5.
msg215524 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-04 14:20
New changeset fef890bd60b1 by Brett Cannon in branch '3.4':
Issue #20942: PyImport_ImportFrozenModuleObject() no longer sets
http://hg.python.org/cpython/rev/fef890bd60b1

New changeset a11ec7aaac10 by Brett Cannon in branch 'default':
merge of fix for issue #20942
http://hg.python.org/cpython/rev/a11ec7aaac10
History
Date User Action Args
2014-04-04 20:22:50Arfreversetstage: patch review -> resolved
2014-04-04 14:22:05brett.cannonsetstatus: open -> closed
resolution: fixed
2014-04-04 14:20:36python-devsetnosy: + python-dev
messages: + msg215524
2014-04-02 19:11:45brett.cannonsetassignee: brett.cannon
messages: + msg215395
2014-04-02 18:57:13Zvezdoslovecsetmessages: + msg215394
2014-03-29 07:59:25Zvezdoslovecsetmessages: + msg215122
2014-03-28 23:18:36ncoghlansetmessages: + msg215090
2014-03-28 18:31:21Zvezdoslovecsetmessages: + msg215064
versions: + Python 3.4
2014-03-28 17:57:14eric.snowsetmessages: + msg215061
2014-03-28 17:28:10eric.snowsetmessages: + msg215060
2014-03-28 15:40:30Zvezdoslovecsetmessages: + msg215053
2014-03-28 13:49:16brett.cannonsetmessages: + msg215046
versions: - Python 3.4
2014-03-28 13:41:08Zvezdoslovecsetnosy: + Zvezdoslovec

messages: + msg215045
versions: + Python 3.4
2014-03-28 13:32:02brett.cannonsetfiles: + no_frozen__file__.diff

messages: + msg215044
2014-03-27 01:11:06eric.snowsetmessages: + msg214926
2014-03-21 18:36:07brett.cannonsetfiles: + no_frozen__file__.diff
keywords: + patch
messages: + msg214399

stage: test needed -> patch review
2014-03-16 16:47:05Arfreversetnosy: + Arfrever
2014-03-16 05:42:27ncoghlancreate