classification
Title: Importing "imp" will fail if dynamic loading not supported
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Jeffrey.Armstrong, brett.cannon, christian.heimes, ncoghlan, python-dev, terry.reedy
Priority: normal Keywords: patch

Created on 2013-01-06 15:20 by Jeffrey.Armstrong, last changed 2013-03-25 21:30 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
imp_dynamic.patch christian.heimes, 2013-01-06 15:49 review
Messages (17)
msg179186 - (view) Author: Jeffrey Armstrong (Jeffrey.Armstrong) * Date: 2013-01-06 15:20
On a platform where dynamic loading is unsupported, the function "imp_load_dynamic" is not compiled (Python/import.c:1775), and the Python function "load_dynamic" (Python/import.c:1845) will not be included in the _imp module.  However, Lib/imp.py attempts to import "load_dynamic" from _imp (Lib/imp.py:9), causing an ImportError if dynamic loading is unsupported.  

The interpreter is unable to start under these circumstances.  The error was encountered on m68k-atari-mint using GCC as the compiler.  This platform is a desktop environment, but has no support for true shared objects and libraries.
msg179191 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-01-06 15:48
The attached patch should fix the issue. Can you give it a try, please?
msg179192 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-01-06 15:50
LGTM
msg179193 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-01-06 15:50
I should say LGTM in terms of looking at the patch, not actually trying it out. =)
msg179195 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-01-06 15:52
I did just realize, though, there is no test for temporarily setting load_dynamic() to None in imp.py to make sure the expected exception is raised.
msg179196 - (view) Author: Jeffrey Armstrong (Jeffrey.Armstrong) * Date: 2013-01-06 16:58
The patch works with respect to allowing the interpreter to start, and importing modules from the interpreter seems to be working fine.  I can't speak to Brett's concerns about possible side effects of setting load_dynamic to None.
msg179749 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-01-12 01:02
Given the current (3.3) imp.py docstring
"""This module provides the components needed to build your own __import__ function.  Undocumented functions are obsolete.
In most cases it is preferred you consider using the importlib module's
functionality over this module.
"""
I wonder why it is being imported on startup. Is this an obsolete holdover.

---
The patch solves the problem of importing a non-existing load_dynamic, but
-        elif type_ == C_EXTENSION:
+        elif type_ == C_EXTENSION and load_dynamic is not None:
             return load_dynamic(name, filename, file)

With this change, an attempt to import a C_EXTENSION file will fall through to 
        else:
            msg =  "Don't know how to import {} (type code {})".format(name, type_)
            raise ImportError(msg, name=name)

Jeffery: does your m68k-atari-mint have no C_EXTENSION files, so that this will never be a problem? On my Windows system, _tkinter is one such, with the following outcome.

import imp
imp.load_dynamic
>>> imp.load_module('tk2', *imp.find_module('_tkinter'))
Traceback (most recent call last):
  File "<pyshell#13>", line 1, in <module>
    imp.load_module('tk2', *imp.find_module('_tkinter'))
  File "C:\Programs\Python33\lib\imp.py", line 164, in load_module
    return load_dynamic(name, filename, file)
TypeError: 'NoneType' object is not callable

Or is this not a problem because deprecated imp.load_module is never actually used?

___
Brett: by 'expected exception', do you mean the one above? or the one that is caught by the patch?

Another question: load_dynamic has a public name but is un-documented. Does that make it private enough that we can freely rebind it to None? Perhaps it does not matter since we are only doing this on machines where Python does not even start, so we won't disable a working imp.load_dynamic call if there is one somewhere (including the stdlib).

___
If imp.load_dynamic is private, then its import into imp can be removed once deprecated load_module is removed, making this issue moot.
msg179751 - (view) Author: Jeffrey Armstrong (Jeffrey.Armstrong) * Date: 2013-01-12 01:18
Terry:  If I'm understanding this correctly, C_EXTENSION implies a dynamically loaded module authored in C.  The issue on FreeMiNT, the operating system in question, is that there is no support for true shared objects.  Any C modules must be statically linked with the Python executable.  I'm thinking the answer is "No, it won't be a problem on m68k-atari-mint," if I'm understanding the meaning of "C_EXTENSION" correctly.
msg179798 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-01-12 14:17
load_dynamic should probably be documented since it does something you can't do on your own and importlib itself uses it.

As for the exception test, it should be to make sure ImportError is raised (i.e. the 'else' clause is hit).
msg179805 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-01-12 15:01
The misleading docs Terry pointed out should also be fixed to note that "imp" is still used to expose some functionality where importlib really does need help from the underlying platform (such as loading dynamic modules).
msg181813 - (view) Author: Jeffrey Armstrong (Jeffrey.Armstrong) * Date: 2013-02-10 15:19
Is Christian's patch going to be sufficient for the time being?  Just curious.
msg181845 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-10 19:47
Yes, Christian's patch should do the trick for fixing the problem you reported, Jeffrey.
msg181851 - (view) Author: Jeffrey Armstrong (Jeffrey.Armstrong) * Date: 2013-02-10 20:31
Sorry, I think my question was a bit vague.  Christian's patch does, in fact, work fine for fixing the problem as reported.  I was wondering if the patch was sufficient to close the bug with a commit.  I didn't know if other work was ongoing to close this issue.
msg181853 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-10 20:39
A commit to Python 3.3 and 3.4 would be enough to close this bug. Just a matter of Christian or someone else finding the time.
msg184404 - (view) Author: Roundup Robot (python-dev) Date: 2013-03-17 22:50
New changeset d5aa922f97f9 by Brett Cannon in branch '3.3':
Issue #16880: _imp.load_dynamic() is not defined on a platform that
http://hg.python.org/cpython/rev/d5aa922f97f9
msg184406 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-03-17 23:04
In honour of Jeffrey's lightning talk at PyCon 2013, I just committed the fix!
msg184407 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-03-17 23:05
And this was merged into default in http://hg.python.org/cpython/rev/3c3c9ad7c297 but for some reason it didn't show up attached to this issue.
History
Date User Action Args
2013-03-25 21:30:21brett.cannonsetstatus: open -> closed
2013-03-17 23:05:13brett.cannonsetmessages: + msg184407
2013-03-17 23:04:21brett.cannonsetassignee: christian.heimes -> brett.cannon
resolution: fixed
messages: + msg184406
stage: commit review -> resolved
2013-03-17 22:50:40python-devsetnosy: + python-dev
messages: + msg184404
2013-02-10 20:39:54brett.cannonsetmessages: + msg181853
2013-02-10 20:31:27Jeffrey.Armstrongsetmessages: + msg181851
2013-02-10 19:47:59brett.cannonsetmessages: + msg181845
2013-02-10 15:19:36Jeffrey.Armstrongsetmessages: + msg181813
2013-01-12 16:11:07terry.reedysetnosy: - Nurhusien2
2013-01-12 16:10:04terry.reedysetmessages: - msg179816
2013-01-12 16:09:39terry.reedysetmessages: - msg179814
2013-01-12 16:09:17terry.reedysetmessages: - msg179810
2013-01-12 16:08:58terry.reedysetmessages: - msg179806
2013-01-12 16:08:45terry.reedysetmessages: - msg179807
2013-01-12 15:45:23Nurhusien2setmessages: + msg179816
2013-01-12 15:42:57Nurhusien2setmessages: + msg179814
2013-01-12 15:15:43Nurhusien2setmessages: + msg179810
2013-01-12 15:11:57Nurhusien2setmessages: + msg179807
2013-01-12 15:09:47Nurhusien2setnosy: + Nurhusien2
messages: + msg179806
2013-01-12 15:01:55ncoghlansetmessages: + msg179805
2013-01-12 14:17:33brett.cannonsetmessages: + msg179798
2013-01-12 01:18:50Jeffrey.Armstrongsetmessages: + msg179751
2013-01-12 01:02:22terry.reedysetnosy: + terry.reedy
messages: + msg179749
2013-01-06 16:58:35Jeffrey.Armstrongsetmessages: + msg179196
2013-01-06 15:52:55brett.cannonsetmessages: + msg179195
2013-01-06 15:50:59brett.cannonsetmessages: + msg179193
2013-01-06 15:50:38brett.cannonsetmessages: + msg179192
stage: needs patch -> commit review
2013-01-06 15:49:07christian.heimessetfiles: + imp_dynamic.patch
keywords: + patch
2013-01-06 15:48:32christian.heimessetassignee: christian.heimes

messages: + msg179191
nosy: + christian.heimes
2013-01-06 15:29:52pitrousetnosy: + brett.cannon, ncoghlan
stage: needs patch

versions: + Python 3.4
2013-01-06 15:20:25Jeffrey.Armstrongcreate