Skip to content

Commit a0cd71b

Browse files
committed
Fix skipped imports considered stale
When the option `follow_imports = skip` is used, dependency states that are initially considered skipped might have their reason changed to not found in `load_graph`. If on the first mypy run, a given suppressed module is written to cache with its suppression reason set to skipped, and it's overridden to not found in a subsequent mypy run, then `suppressed_deps_opts` between the cache and the current run won't match and the module will be considered stale. To fix the issue, change the unconditional overwrite to `setdefault`. This also affects mypyc as the dependency being considered stale means that mypyc will needlessly recompile it instead of reading from cache. I have added a unit test to confirm that the dependency output files are not overwritten with the fix.
1 parent 3179030 commit a0cd71b

3 files changed

Lines changed: 121 additions & 2 deletions

File tree

mypy/build.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4345,7 +4345,7 @@ def load_graph(
43454345
for dep in st.ancestors + dependencies + st.suppressed:
43464346
ignored = dep in st.suppressed_set and dep not in entry_points
43474347
if ignored and dep not in added:
4348-
manager.missing_modules[dep] = SuppressionReason.NOT_FOUND
4348+
manager.missing_modules.setdefault(dep, SuppressionReason.NOT_FOUND)
43494349
# TODO: for now we skip this in the daemon as a performance optimization.
43504350
# This however creates a correctness issue, see #7777 and State.is_fresh().
43514351
if not manager.use_fine_grained_cache() or manager.options.warn_unused_configs:

mypy/test/testgraph.py

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,24 @@
22

33
from __future__ import annotations
44

5+
import os
56
import sys
7+
import tempfile
68
from collections.abc import Set as AbstractSet
79

8-
from mypy.build import BuildManager, BuildSourceSet, State, order_ascc, sorted_components
10+
import mypy.build as build_module
11+
from mypy.build import (
12+
BuildManager,
13+
BuildSourceSet,
14+
State,
15+
SuppressionReason,
16+
order_ascc,
17+
sorted_components,
18+
)
919
from mypy.errors import Errors
1020
from mypy.fscache import FileSystemCache
1121
from mypy.graph_utils import strongly_connected_components, topsort
22+
from mypy.main import process_options
1223
from mypy.modulefinder import SearchPaths
1324
from mypy.options import Options
1425
from mypy.plugin import Plugin
@@ -108,6 +119,50 @@ def _make_manager(self) -> BuildManager:
108119
)
109120
return manager
110121

122+
def test_fine_grained_cache_preserves_suppression_reason(self) -> None:
123+
with tempfile.TemporaryDirectory() as tmp:
124+
with open(os.path.join(tmp, "mypy.ini"), "w", encoding="utf-8") as f:
125+
f.write("[mypy]\ncache_fine_grained = True\nfollow_imports = skip\n")
126+
with open(os.path.join(tmp, "skipped.py"), "w", encoding="utf-8") as f:
127+
f.write("def ignored() -> int:\n return 1\n")
128+
with open(os.path.join(tmp, "dep.py"), "w", encoding="utf-8") as f:
129+
f.write("import skipped\n\ndef value() -> int:\n return 1\n")
130+
with open(os.path.join(tmp, "main.py"), "w", encoding="utf-8") as f:
131+
f.write("import dep\n\ndef value() -> int:\n return dep.value()\n")
132+
with open(os.path.join(tmp, "seed.py"), "w", encoding="utf-8") as f:
133+
f.write("import skipped\n")
134+
135+
old_cwd = os.getcwd()
136+
os.chdir(tmp)
137+
try:
138+
139+
def run_mypy(
140+
args: list[str], *, server_options: bool = False
141+
) -> build_module.BuildResult:
142+
sources, options = process_options(args, server_options=server_options)
143+
options.use_builtins_fixtures = True
144+
result = build_module.build(sources=sources, options=options)
145+
assert_equal(result.errors, [])
146+
return result
147+
148+
result = run_mypy(["dep.py", "main.py"])
149+
assert_equal(result.graph["dep"].suppressed, ["skipped"])
150+
assert_equal(result.manager.missing_modules["skipped"], SuppressionReason.SKIPPED)
151+
152+
result = run_mypy(["seed.py", "skipped.py"])
153+
assert_equal(result.graph["seed"].dependencies, ["skipped", "builtins"])
154+
assert_equal(result.graph["seed"].suppressed, [])
155+
156+
result = run_mypy(
157+
["--use-fine-grained-cache", "seed.py", "dep.py", "main.py"],
158+
server_options=True,
159+
)
160+
finally:
161+
os.chdir(old_cwd)
162+
163+
assert_equal(result.manager.missing_modules["skipped"], SuppressionReason.SKIPPED)
164+
assert result.graph["dep"].is_fresh()
165+
111166
def test_sorted_components(self) -> None:
112167
manager = self._make_manager()
113168
graph = {

mypyc/test/test_misc.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import os
44
import tempfile
55
import unittest
6+
from unittest.mock import patch
67

8+
import mypyc.build as mypyc_build_module
79
from mypyc.build import get_header_deps, resolve_cfile_deps
810
from mypyc.ir.ops import BasicBlock
911
from mypyc.ir.pprint import format_blocks, generate_names_for_ir
@@ -25,6 +27,68 @@ def test_debug_op(self) -> None:
2527
assert code[:-1] == ["L0:", " r0 = 'foo'", " CPyDebug_PrintObject(r0)"]
2628

2729

30+
class TestIncrementalOutputFiles(unittest.TestCase):
31+
def test_cached_dependency_output_file_not_overwritten_for_preserved_suppression(self) -> None:
32+
with tempfile.TemporaryDirectory() as tmp:
33+
build_dir = os.path.join(tmp, "build")
34+
with open(os.path.join(tmp, "mypy.ini"), "w", encoding="utf-8") as f:
35+
f.write("[mypy]\nfollow_imports = skip\n")
36+
os.mkdir(os.path.join(tmp, "skipped"))
37+
with open(os.path.join(tmp, "skipped", "__init__.py"), "w", encoding="utf-8") as f:
38+
f.write("x = 1\n")
39+
with open(os.path.join(tmp, "skipped", "child.py"), "w", encoding="utf-8") as f:
40+
f.write("import skipped\n\ndef child() -> int:\n return 1\n")
41+
with open(os.path.join(tmp, "dep.py"), "w", encoding="utf-8") as f:
42+
f.write("import skipped\n\ndef value() -> int:\n return 1\n")
43+
with open(os.path.join(tmp, "main.py"), "w", encoding="utf-8") as f:
44+
f.write("import dep\n\ndef value() -> int:\n return dep.value()\n")
45+
46+
compiler_options = CompilerOptions(separate=True, target_dir=build_dir)
47+
48+
old_cwd = os.getcwd()
49+
os.chdir(tmp)
50+
try:
51+
files = ["dep.py", "skipped/child.py", "main.py"]
52+
53+
groups, group_cfilenames, _ = mypyc_build_module.mypyc_build(
54+
files, compiler_options, separate=True
55+
)
56+
57+
dep_group = next(
58+
i
59+
for i, (group_sources, _) in enumerate(groups)
60+
if [source.module for source in group_sources] == ["dep"]
61+
)
62+
dep_output_files = {
63+
os.path.abspath(path)
64+
for paths in group_cfilenames[dep_group]
65+
for path in paths
66+
}
67+
68+
# Only main changed; dep should be loaded from the mypyc IR cache and reuse
69+
# its existing C outputs.
70+
with open("main.py", "w", encoding="utf-8") as f:
71+
f.write("import dep\n\ndef value() -> int:\n return dep.value() + 1\n")
72+
73+
written_paths: set[str] = set()
74+
original_write_file = mypyc_build_module.write_file
75+
76+
def recording_write_file(path: str, contents: str) -> None:
77+
written_paths.add(os.path.abspath(path))
78+
original_write_file(path, contents)
79+
80+
with patch.object(
81+
mypyc_build_module, "write_file", side_effect=recording_write_file
82+
):
83+
# skipped.child imports its skipped package ancestor; loading that cached
84+
# state should not make cached dep stale or rewrite dep's generated outputs.
85+
mypyc_build_module.mypyc_build(files, compiler_options, separate=True)
86+
finally:
87+
os.chdir(old_cwd)
88+
89+
assert written_paths.isdisjoint(dep_output_files)
90+
91+
2892
class TestHeaderDeps(unittest.TestCase):
2993
"""
3094
Tests for the header-dependency tracking used to build `Extension.depends`, which drives

0 commit comments

Comments
 (0)