Skip to content

Importing packages can poison the mypy cache. #5534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TheKevJames opened this issue Aug 28, 2018 · 10 comments
Closed

Importing packages can poison the mypy cache. #5534

TheKevJames opened this issue Aug 28, 2018 · 10 comments

Comments

@TheKevJames
Copy link

TheKevJames commented Aug 28, 2018

Ok -- this is a weird one, but I think I've got it down to the smallest possible repro.

I first noticed this issue with the aiohttp library as of version 3.4.0 (when they introduced type-hinting), but I'm cross-posting this issue to both projects since the issue seems to run deeper than just a library issue. Importing the aiohttp library, even if it is unused, causes the mypy cache to occasionally get poisoned.

Here's the repro case:

import aiohttp
def trigger() -> dict:
    return 'foo'

Running python -m mypy repro.py against this file works as expected (eg. returns an incompatible return type error). Replacing the -> dict with -> str throws the following assertion (attached at bottom of report) on re-running the type check. Note that this holds true in the opposite direction -- if we begin with -> str, the first typecheck is successful, but replacing -> str with -> dict causes the same assertion error. Either way, the situation can only be resolved by clearing the .mypy_cache folder, at which point mypy acts as expected once more.

Interestingly, removing the aiohttp import prevents this poisoning from occurring. In addition, removing the import once the cache is poisoned also fixes the issue.

I speculate there are two issues here, one with aiohttp and one with mypy:

  • aiohttp seems to have some weirdness going on in their type hinting
  • regardless of what a library does, the above repro case should not cause mypy issues when dealing only with stdlib types.

Error Trace:

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/__main__.py", line 11, in <module>
    main(None)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/main.py", line 91, in main
    res = type_check_only(sources, bin_dir, options, flush_errors, fscache)  # noqa
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/main.py", line 148, in type_check_only
    fscache=fscache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 177, in build
    flush_errors, fscache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 350, in _build
    graph = dispatch(sources, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2560, in dispatch
    process_graph(graph, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2846, in process_graph
    process_fresh_modules(graph, prev_scc, manager)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 2932, in process_fresh_modules
    graph[id].fix_cross_refs()
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/build.py", line 1975, in fix_cross_refs
    self.options.use_fine_grained_cache)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 24, in fixup_module
    node_fixer.visit_symbol_table(tree.names)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 89, in visit_symbol_table
    value.node.accept(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/nodes.py", line 2539, in accept
    return visitor.visit_type_alias(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 135, in visit_type_alias
    a.target.accept(self.type_fixer)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/types.py", line 511, in accept
    return visitor.visit_instance(self)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 149, in visit_instance
    inst.type = lookup_qualified_typeinfo(self.modules, type_ref, self.quick_and_dirty)
  File "/Users/kevin/coding/mypy-bug/venv/lib/python3.7/site-packages/mypy/fixup.py", line 239, in lookup_qualified_typeinfo
    assert quick_and_dirty, "Should never get here in normal mode"
AssertionError: Should never get here in normal mode

Versions:

(venv) kevin@box ~/c/mypy-bug » python --version
Python 3.7.0
(venv) kevin@box ~/c/mypy-bug » python -m mypy --version
mypy 0.620
(venv) kevin@box ~/c/mypy-bug » python -m pip freeze | grep aiohttp
aiohttp==3.4.0
@gvanrossum
Copy link
Member

What do you get without --ignore-missing-imports?

@TheKevJames
Copy link
Author

TheKevJames commented Aug 28, 2018

@gvanrossum I have verified the behavior is the same without that flag. I will edit my repro case to remove the flag, since it is unrelated.

@gvanrossum
Copy link
Member

OK, so then it would be interesting to see which part of aiohttp 3.4.0 causes the problem. That will be a difficult bisection job (remove code from aiohttp until the problem no longer occurs).

But first, it's been over a month since mypy 0.620 was released, and the bugfix elves have been hard at work all that time. Does the problem still occur with mypy master from GitHub?

@TheKevJames
Copy link
Author

As of the current latest on master (f0b26f9), this is indeed still reproducible. For the record, the verified reproduction was after running:

(venv) » pip install -U git+git://github.com/python/mypy.git
Collecting git+git://github.com/python/mypy.git
  Cloning git://github.com/python/mypy.git to /private/var/folders/qs/b8cnmbcd1szb8fv4j56_6p_w0000gn/T/pip-req-build-pf17uyhk
  Installing build dependencies ... done
Requirement already satisfied, skipping upgrade: typed-ast<1.2.0,>=1.1.0 in ./venv/lib/python3.7/site-packages (from mypy==0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3) (1.1.0)
Collecting mypy_extensions<0.5.0,>=0.4.0 (from mypy==0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3)
  Downloading https://files.pythonhosted.org/packages/4d/72/8d54e2b296631b9b14961d583e56e90d9d7fba8a240d5ce7f1113cc5e887/mypy_extensions-0.4.1-py2.py3-none-any.whl
Installing collected packages: mypy-extensions, mypy
  Found existing installation: mypy 0.620
    Uninstalling mypy-0.620:
      Successfully uninstalled mypy-0.620
  Running setup.py install for mypy ... done
Successfully installed mypy-0.630+dev.f0b26f9e6197eeb2f755fae73c949ff01384ded3 mypy-extensions-0.4.1

in the original venv.

@gvanrossum
Copy link
Member

Thanks for confirming -- then bisecting by removing large parts of aiohttp is in order...

@ilevkivskyi
Copy link
Member

There is a hint in the traceback: the problematic type is aliased somewhere at a module level. This should narrow down the search area.

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Aug 29, 2018

Here is a minimalist repro:

# file main.py
import lib
x = 1

# file lib.py
class Slow:
    pass

s: Slow  # could be anything that uses 'Slow' as a type: function, type alias, etc.
from cext import Slow  # type: ignore

Run mypy main.py once, make any change to main.py, run again and see the crash.

And this is what is going on: first mypy creates an Instance of Slow (TypeInfo at this time) as a type for s, but then the ignored import actually overwrites Slow in the symbol table making it a Var. Then when loading the cache mypy panics when it sees an Instance whose .type is a Var.

The best solution IMO is that import over an existing name should never shadow/overwrite an existing node in the symbol table. The original node should be the "wining" one.

Later we can support conditional imports by creating a "union" of two nodes, but we have a separate issue for this.

@ilevkivskyi
Copy link
Member

Another thing pointed out by @asvetlov is that the assert message could be a bit more informative. I will improve it as a part of the PR (just showing a full name and type of problematic node would be much better).

@asvetlov
Copy link

Interesting. Thanks.
The minimalist repro is exactly the aiohttp case.
We use it for optional C Extention optimizations.
CPython itself does the same but without type annotations.

@asvetlov
Copy link

asvetlov commented Sep 4, 2018

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants