-m switch regression in Python 3.5.2 (under rare circumstances)

[wm75]

BPO	27487
Nosy	@brettcannon, @ncoghlan, @larryhastings, @ned-deily, @ericsnowcurrently, @vadmium, @wm75, @lzkelley
Files	warn-imported.patch warn-imported.v2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-08-21.05:45:24.570>
created_at = <Date 2016-07-11.20:18:11.100>
labels = ['type-bug', 'library', 'release-blocker']
title = '-m switch regression in Python 3.5.2 (under rare circumstances)'
updated_at = <Date 2017-07-13.02:50:13.345>
user = 'https://github.com/wm75'

bugs.python.org fields:

activity = <Date 2017-07-13.02:50:13.345>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2016-08-21.05:45:24.570>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2016-07-11.20:18:11.100>
creator = 'wolma'
dependencies = []
files = ['43745', '43773']
hgrepos = []
issue_num = 27487
keywords = ['patch']
message_count = 23.0
messages = ['270202', '270225', '270279', '270287', '270288', '270300', '270301', '270302', '270324', '270356', '270359', '270379', '270381', '270427', '270457', '270471', '270547', '270640', '270725', '273266', '273267', '298246', '298256']
nosy_count = 9.0
nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'ned.deily', 'python-dev', 'eric.snow', 'martin.panter', 'wolma', 'lzkelley']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue27487'
versions = ['Python 3.5', 'Python 3.6']

[wm75]

As a result of bpo-14285 Python 3.5.2 now imports packages in runpy. _get_module_details before calling importlib.util.find_spec.

Although I'm not sure how important this is, I wanted to report that this new behaviour can have a side-effect under pretty exotic circumstances. When __init__.py imports the same module that is supposed to be invoked via the -m switch and that module replaces its own entry in sys.modules with another object, this causes importlib.util.find_spec to fail with a *very* cryptic:

Error while finding spec for 'package.module' (ValueError: package.module.__spec__ is not set)

without an exception traceback.

I have no idea whether any other package would be affected by this, but it took me quite some time today to trace the cause of this and it is not what you'd expect from a maintenance release. I think the changed behaviour in runpy should at least be documented (beyond just mentioning the issue in the changelog).

[ncoghlan]

Since this worked in 3.5.1, and fails in 3.5.2, I think it's reasonable to consider if it makes sense to find a way to make it work again in 3.5.3 (and then decide separately whether or not we want to preserve the capability in 3.6.0).

Specifically, restoring the old behaviour would mean that:

1. *If* the -m target module already exists in sys.modules after importing the parent module; and
2. The already imported target module doesn't have a __spec__ attribute; then
3. Ignore that already imported artifact when figuring out what to run as __main__

However, the problem with doing that is that this *is* an instance of code that falls into the double-import trap: http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html#executing-the-main-module-twice

Even in 3.5.1, the affected module is getting executed twice - once from the package's __init__.py, and once via the "-m" switch. For cases that *don't* overwrite the module in sys.modules, a solution that resolves the issue in a backwards and forwards compatible way is to split the module being executed via -m as follows:

• Move the pieces needed by mypkg.__init__ into a "mypkg._mymain_support" module
• In that module, set "__name__ = 'mypkg.mymain'" to preserve pickle compatibility and docs introspection
• Change both "mypkg.__init__" and "mypkg.mymain" to import the shared components from the "mypkg._mymain_support" module

However, you still run into a problem with figuring out when the replacement module should be written into sys.modules - if that stays in "mypkg.mymain" it will no longer get done as a side effect of importing "mypkg", while if it moves into "mypkg._mymain_support", you'll still run into the problem reported here in 3.5.2

[vadmium]

In trying to understand this, I built a package with two simple files:

$ cat package/*.py
# package/__init__.py:
from . import module
# package/module.py:
import sys
sys.modules[__name__] = object()

With this I can reproduce your __spec__ error, and I see it works without error in 3.5.0 and 2.7.11. FWIW, with the current 3.3 branch, I get a different error:

/media/disk/home/proj/python/cpython/python: Error while finding loader for 'package.module' (<class 'AttributeError'>: 'object' object has no attribute '__loader__')

The revision that causes the regression is 3202d143a194. Since I made that change, I feel responsible for it. But I don’t have much time ATM to come up with an ideal solution. One quick solution would be to revert my change, but since this is an obscure use case, I am not enthusiastic about doing that :)

Is it possible to set the __spec__ attribute to work around the problem? I don’t really understand the “module spec” business, but “runpy” seems to rely on that attribute. Is replacing sys.modules entries like this even supported usage?

Even in 3.5.0, I can still produce the error by importing the package before using runpy:

$ python3.5 -c 'import runpy, package; runpy.run_module("package.module")'
Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 104, in _get_module_details
    spec = importlib.util.find_spec(mod_name)
  File "/usr/lib/python3.5/importlib/util.py", line 99, in find_spec
    raise ValueError('{}.__spec__ is not set'.format(name)) from None
ValueError: package.module.__spec__ is not set

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3.5/runpy.py", line 178, in run_module
    mod_name, mod_spec, code = _get_module_details(mod_name)
  File "/usr/lib/python3.5/runpy.py", line 110, in _get_module_details
    raise ImportError(msg.format(mod_name, type(ex), ex)) from ex
ImportError: Error while finding spec for 'package.module' (<class 'ValueError'>: package.module.__spec__ is not set)
[Exit 1]

Wolfgang: Do you want to propose a specific wording to add to the documentation? Maybe something like “runpy and the ‘python -m’ option require that after the module is imported, it should have a __spec__ attribute”. Where should this go? In the runpy, command line, and/or What’s New documentation?

FWIW, 2.7.11 gives an even stranger error; perhaps this is a different bug:

$ python2.7 -c 'import runpy, package.module; runpy.run_module("package.module")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python2.7/runpy.py", line 170, in run_module
    mod_name, loader, code, fname = _get_module_details(mod_name)
  File "/usr/lib/python2.7/runpy.py", line 101, in _get_module_details
    loader = get_loader(mod_name)
  File "/usr/lib/python2.7/pkgutil.py", line 464, in get_loader
    return find_loader(fullname)
  File "/usr/lib/python2.7/pkgutil.py", line 474, in find_loader
    for importer in iter_importers(fullname):
  File "/usr/lib/python2.7/pkgutil.py", line 424, in iter_importers
    if fullname.startswith('.'):
AttributeError: 'object' object has no attribute 'startswith'
[Exit 1]

[ncoghlan]

Breaking down a few of the moving parts here:

• Yes, modules replacing themselves in sys.modules as a side effect of import is supported behaviour (it's why the import system looks them up by name in sys.modules as the final step, rather than just returning whatever the loader returns)

• the dependency on __spec__ attributes is actually in importlib, where following PEP-351 Brett has been pushing hard to ensure modules always have a __spec__ attribute so the rest of the import system (including, indirectly, runpy) can rely on having it available (however, as seen here, not everything in sys.modules is going to be a module, so we may need to either keep more of the workarounds that help tolerate folks breaking that assumption, or else find a way to try to enforce it even for custom objects)

• explicitly copying __spec__ from the original module object to the replacement object (in Wolfgang's code) should indeed be enough to get the use case reported here restored to the way it behaved in 3.5.1

[ncoghlan]

Oops, that PEP reference was meant to be PEP-451 (which added the __spec__ attribute and the concept of module specs as something distinct from the modules themselves)

[wm75]

@martin and regarding Python3.3: Right, you cannot do the replacement when
running the module as __main__. For my use case that's not required though so in
the module I can just do:

if __name__ == '__main__':
    print('running module as a script')
else:
    # do not attempt this when __name__ == '__main__' in Python 3.3
    sys.modules[__name__] = object()
    
Apart from this limitation, such code is compatible with Python 3.2 - 3.5.1, but 
breaks under 3.5.2. It's the fact that it breaks in a maintenance release that 
I find disturbing. In fact, I learnt about the breakage only from an end-user 
who bought a new laptop with OS X, downloaded the latest Python release and 
suddenly could not get our package to run that had worked on his previous 
machine. Turns out, none of our test machines was running 3.5.2 yet, but were 
running 3.5.0 and 3.5.1.

OTOH, I agree with you that the circumstances under which your change causes
trouble are pretty exotic. As Nick points out, modules replacing themselves in
sys.modules are not that special. What *makes* the situation special is that

1. the package __init__.py has to import the self-replacing module and that
2. the self-replacing module is intended to be run via python3 -m package.module

That's why I wrote initially that I don't know if many (or, in fact, any other
package would be affected by this).

Nick's concern about the double import situation is more general, but since
this is something earlier releases have been doing, an improvement here would
be relevant only for a major release (3.6 or 3.7).

Regarding 3.5.3, I'm not sure what the best solution would be:

• the suggestion to copy over the __spec__ attribute to the replacement object
  is putting the burden on package maintainers. Given that probably only few
  packages will be affected that may well be acceptable. In fact, for my own
  case I have now refactored things a bit so I can avoid the import from
  __init__.py.

• from a strict versioning perspective, you may also argue like Nick that if it
  was working in 3.5.0-1 it should work again in 3.5.3.

I'm personally pretty neutral here. I just thought it may be good to report the
issue so that it becomes easier for others to understand what is going on if
they encounter the same error. Maybe even the fact that this issue exists now
is serving this purpose as well as documenting the changed behaviour?

[ncoghlan]

Aye, the report is definitely appreciated - the interaction between "__main__" execution and normal module imports has always been tricky, so it's always good to be informed of new problems folks encounter.

As an example illustrating the underlying problem here without involving the "-m" switch at all:

>>> from importlib.util import find_spec
>>> find_spec("dis")
ModuleSpec(name='dis', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fa7ba859fd0>, origin='/usr/lib64/python3.5/dis.py')
>>> import sys
>>> sys.modules["dis"] = object()
>>> find_spec("dis")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.5/importlib/util.py", line 99, in find_spec
    raise ValueError('{}.__spec__ is not set'.format(name)) from None
ValueError: dis.__spec__ is not set

The difference between 3.5.1 and 3.5.2 is that if you do "find_spec('mypkg.mymod')" *before* "mypkg" is imported, then find_spec will use the original module object rather than the replacement inserted into sys.modules when it recurses down to look for "mypkg.mymod". However, if "mypkg" is already in sys.modules before find_spec is called, then find_spec will attempt to use that existing object, rather than the (potentially no longer available) original module object.

[ncoghlan]

Sorry, I got the levels slightly confused there - a bit more experimentation shows it isn't the parent package that needs a __spec__ attribute, but only the package being executed.

# Parent package needs __path__ rather than __spec__
>>> find_spec("unittest.mock")
ModuleSpec(name='unittest.mock', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fa7b6434b38>, origin='/usr/lib64/python3.5/unittest/mock.py')
>>> sys.modules["unittest"] = object()
>>> find_spec("unittest.mock")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.5/importlib/util.py", line 89, in find_spec
    return _find_spec(fullname, parent.__path__)
AttributeError: 'object' object has no attribute '__path__'

# Leaf package needs __spec__
>>> del sys.modules["unittest"]
>>> find_spec("unittest.mock")
ModuleSpec(name='unittest.mock', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fa7b64344e0>, origin='/usr/lib64/python3.5/unittest/mock.py')
>>> import unittest.mock
>>> del sys.modules["unittest.mock"].__spec__
>>> find_spec("unittest.mock")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.5/importlib/util.py", line 99, in find_spec
    raise ValueError('{}.__spec__ is not set'.format(name)) from None
ValueError: unittest.mock.__spec__ is not set

[brettcannon]

So what I'm gathering is that Martin fixed a bug that accidentally introduced a new one in 3.5.2 (which Martin should not feel responsible for; this import stuff is all very subtle and people use every nuance of it). To me, the question is whether there is a way to not revert Martin's fix -- or any other fixes -- while fixing Wolfgang's problem.

If the answer is yes, then great and we just need to know what the answer is in the form of a patch. :) My hope is someone will be inspired enough to figure out a solution and we can fix this for Wolfgang.

But if the answer is no, then we need to decide what the lesser of two bugs are. For me, I see Wolfgang's issue is the lesser of two bugs and thus the one that stays broken if we can't fix it reasonably (sorry, Wolfgang). Because we are trying to eliminate the double-import as that causes serious problems for anyone using import side-effects, I don't want to suddenly slide backwards in 3.5.3 by letting that happen again (and I think Nick agrees w/ me on that point). I also don't want people continuing down a path where they think that they can blank out critical import metadata and have import continue to work (not that I'm blaming Wolfgang for relying on it, just that it won't hold in the future). And since the reliance on __spec__ is just going to grow thanks to the elimination of double imports and slowly making that attribute the ground truth for import metadata, Wolfgang's code would need to be updated for Python 3.6 or later anyway.

But as I said, I'm sure everyone's utmost preference is if someone can figure out how to fix this in 3.5.3 without undoing another fix.

[ncoghlan]

In http://bugs.python.org/issue27487#msg270300, Wolfgang noted: "In fact, for my own case I have now refactored things a bit so I can avoid the import from __init__.py."

So I take that to mean that Wolfgang's particular project is now compatible with the new 3.5.2 behaviour, or at least will be as of its next release (Wolfgang, please correct me if I've misinterpreted your comment).

The "double import" implications involved here aren't entirely clearcut though, as even in 3.5.2 if a leaf module invoked via "-m" is a normal module, importing it from the parent package's __init__ module (or any other part of the startup process, like a .pth file) will still lead to it being executed twice.

What's changed is that if the leaf module replaces itself in sys.modules with an object that doesn't have a __spec__ attribute, then executing it with -m will fail, rather than re-running the module level code as __main__ the way it will for a normal module.

[ncoghlan]

Perhaps a suitable outcome here would be to just add an unconditional RuntimeWarning when the target of "-m" is already in sys.modules after the parent package import, along the lines of:

"RuntimeWarning: '<module>' already in sys.modules prior to '__main__' execution; this may result in unpredictable behaviour"

Then if folks get the "ValueError: package.module.__spec__ is not set" message, that would be an example of the noted "unpredictable behaviour", and the suggested resolution would be the path Wolfgang took: find a way to eliminate the warning when executing that particular module as __main__.

[wm75]

A warning like this sounds good to me though I'd prefer it to be slightly more verbose, like:

"RuntimeWarning: '<module>' found in sys.modules after import of package '<package>', but prior to execution of '<module>' as '__main__'; this may result in unpredictable behaviour"

or it wouldn't really have helped me much to understand what was happening.

P.S.: You understood correctly that my own code is fixed (patch sent to the affected user, next release will have the fix). Learnt something about the whole __spec__ and double import subject along the way so it wasn't completely wasted time.