Skip to content

Don't have newly added files trigger a coarse-grained rebuild in fine-grained cache mode #4669

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

Merged
merged 4 commits into from
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ def __init__(self, data_dir: str,
self.data_dir = data_dir
self.errors = errors
self.errors.set_ignore_prefix(ignore_prefix)
self.only_load_from_cache = options.use_fine_grained_cache
self.lib_path = tuple(lib_path)
self.source_set = source_set
self.reports = reports
Expand Down Expand Up @@ -1674,6 +1675,13 @@ def __init__(self,
for id, line in zip(self.meta.dependencies, self.meta.dep_lines)}
self.child_modules = set(self.meta.child_modules)
else:
# In fine-grained cache mode, pretend we only know about modules that
# have cache information and defer handling new modules until the
# fine-grained update.
if manager.only_load_from_cache:
manager.log("Deferring module to fine-grained update %s (%s)" % (path, id))
raise ModuleNotFound

# Parse the file (and then some) to get the dependencies.
self.parse_file()
self.compute_dependencies()
Expand Down Expand Up @@ -2093,6 +2101,15 @@ def dispatch(sources: List[BuildSource], manager: BuildManager) -> Graph:
manager.log("Mypy version %s" % __version__)
t0 = time.time()
graph = load_graph(sources, manager)

# This is a kind of unfortunate hack to work around some of fine-grained's
# fragility: if we have loaded less than 50% of the specified files from
# cache in fine-grained cache mode, load the graph again honestly.
if manager.options.use_fine_grained_cache and len(graph) < 0.50 * len(sources):
manager.log("Redoing load_graph because too much was missing")
manager.only_load_from_cache = False
graph = load_graph(sources, manager)

t1 = time.time()
manager.add_stats(graph_size=len(graph),
stubs_found=sum(g.path is not None and g.path.endswith('.pyi')
Expand Down Expand Up @@ -2206,7 +2223,7 @@ def load_graph(sources: List[BuildSource], manager: BuildManager,
there are syntax errors.
"""

graph = old_graph or {} # type: Graph
graph = old_graph if old_graph is not None else {} # type: Graph

# The deque is used to implement breadth-first traversal.
# TODO: Consider whether to go depth-first instead. This may
Expand Down
1 change: 1 addition & 0 deletions mypy/server/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def __init__(self,
# this directly reflected in load_graph's interface.
self.options.cache_dir = os.devnull
manager.saved_cache = {}
manager.only_load_from_cache = False
# Active triggers during the last update
self.triggered = [] # type: List[str]

Expand Down
17 changes: 14 additions & 3 deletions mypy/test/testfinegrained.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
return

main_src = '\n'.join(testcase.input)
sources_override = self.parse_sources(main_src)
step = 1
sources_override = self.parse_sources(main_src, step)
messages, manager, graph = self.build(main_src, testcase, sources_override,
build_cache=self.use_cache,
enable_cache=self.use_cache)
Expand All @@ -92,6 +93,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
steps = testcase.find_steps()
all_triggered = []
for operations in steps:
step += 1
modules = []
for op in operations:
if isinstance(op, UpdateFile):
Expand All @@ -102,6 +104,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
# Delete file
os.remove(op.path)
modules.append((op.module, op.path))
sources_override = self.parse_sources(main_src, step)
if sources_override is not None:
modules = [(module, path)
for module, path in sources_override
Expand Down Expand Up @@ -181,14 +184,22 @@ def format_triggered(self, triggered: List[List[str]]) -> List[str]:
result.append(('%d: %s' % (n + 2, ', '.join(filtered))).strip())
return result

def parse_sources(self, program_text: str) -> Optional[List[Tuple[str, str]]]:
def parse_sources(self, program_text: str,
incremental_step: int) -> Optional[List[Tuple[str, str]]]:
"""Return target (module, path) tuples for a test case, if not using the defaults.

These are defined through a comment like '# cmd: main a.py' in the test case
description.
"""
# TODO: Support defining separately for each incremental step.
m = re.search('# cmd: mypy ([a-zA-Z0-9_./ ]+)$', program_text, flags=re.MULTILINE)
regex = '# cmd{}: mypy ([a-zA-Z0-9_./ ]+)$'.format(incremental_step)
alt_m = re.search(regex, program_text, flags=re.MULTILINE)
if alt_m is not None and incremental_step > 1:
# Optionally return a different command if in a later step
# of incremental mode, otherwise default to reusing the
# original cmd.
m = alt_m

if m:
# The test case wants to use a non-default set of files.
paths = m.group(1).strip().split()
Expand Down
118 changes: 118 additions & 0 deletions test-data/unit/fine-grained-modules.test
Original file line number Diff line number Diff line change
Expand Up @@ -971,3 +971,121 @@ x = Foo()
==
==
main:2: error: Too few arguments for "foo" of "Foo"

-- This series of tests is designed to test adding a new module that
-- doesn't appear in the cache, for cache mode. They aren't run only
-- in cache mode, though, because they are still perfectly good
-- regular tests.
[case testAddModuleAfterCache1]
# cmd: mypy main a.py
# cmd2: mypy main a.py b.py
# cmd3: mypy main a.py b.py
import a
[file a.py]
pass
[file a.py.2]
import b
b.foo(0)
[file b.py.2]
def foo() -> None: pass
[file b.py.3]
def foo(x: int) -> None: pass
[out]
==
a.py:2: error: Too many arguments for "foo"
==

[case testAddModuleAfterCache2]
# cmd: mypy main a.py
# cmd2: mypy main a.py b.py
# cmd3: mypy main a.py b.py
# flags: --ignore-missing-imports --follow-imports=skip
import a
[file a.py]
import b
b.foo(0)
[file b.py.2]
def foo() -> None: pass
[file b.py.3]
def foo(x: int) -> None: pass
[out]
==
a.py:2: error: Too many arguments for "foo"
==

[case testAddModuleAfterCache3]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have more tests around this. For example, test that errors are correctly generated in all the new modules and existing modified modules, and make sure that we can correctly bind references to things defined in all three classes of modules:

  1. Cached but unmodified
  2. Cached and modified
  3. New modules

# cmd: mypy main a.py
# cmd2: mypy main a.py b.py c.py d.py e.py f.py g.py
# flags: --ignore-missing-imports --follow-imports=skip
import a
[file a.py]
import b, c, d, e, f, g
[file b.py.2]
[file c.py.2]
[file d.py.2]
[file e.py.2]
[file f.py.2]
[file g.py.2]
[out]
==

[case testAddModuleAfterCache4]
# cmd: mypy main a.py
# cmd2: mypy main a.py b.py
# cmd3: mypy main a.py b.py
# flags: --ignore-missing-imports --follow-imports=skip
import a
import b
[file a.py]
def foo() -> None: pass
[file b.py.2]
import a
a.foo(10)
[file a.py.3]
def foo(x: int) -> None: pass
[out]
==
b.py:2: error: Too many arguments for "foo"
==

[case testAddModuleAfterCache5]
# cmd: mypy main a.py
# cmd2: mypy main a.py b.py
# cmd3: mypy main a.py b.py
# flags: --ignore-missing-imports --follow-imports=skip
import a
import b
[file a.py]
def foo(x: int) -> None: pass
[file a.py.2]
def foo() -> None: pass
[file b.py.2]
import a
a.foo(10)
[file a.py.3]
def foo(x: int) -> None: pass
[out]
==
b.py:2: error: Too many arguments for "foo"
==

[case testAddModuleAfterCache6]
# cmd: mypy main a.py
# cmd2: mypy main a.py b.py
# cmd3: mypy main a.py b.py
# flags: --ignore-missing-imports --follow-imports=skip
import a
[file a.py]
import b
b.foo()
[file a.py.2]
import b
b.foo(0)
[file b.py.2]
def foo() -> None: pass
[file b.py.3]
def foo(x: int) -> None: pass
[out]
==
a.py:2: error: Too many arguments for "foo"
==