Skip to content

Fix crash in dmypy when packages are removed during dmypy run #19229

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix crash in dmypy when packages are removed during dmypy run #19229

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 9 additions & 5 deletions mypy/dmypy_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,9 @@ def fine_grained_increment_follow_imports(
worklist = changed.copy()
while worklist:
module = worklist.pop()
if module[0] not in graph:
continue
sources2 = self.direct_imports(module, graph)
if sources2 is None:
continue
# Filter anything already seen before. This prevents
# infinite looping if there are any self edges. (Self
# edges are maybe a bug, but...)
Expand Down Expand Up @@ -770,10 +770,14 @@ def find_reachable_changed_modules(

def direct_imports(
self, module: tuple[str, str], graph: mypy.build.Graph
) -> list[BuildSource]:
) -> list[BuildSource] | None:
"""Return the direct imports of module not included in seen."""
state = graph[module[0]]
return [BuildSource(graph[dep].path, dep, followed=True) for dep in state.dependencies]
try:
state = graph[module[0]]
return [BuildSource(graph[dep].path, dep, followed=True) for dep in state.dependencies]
except KeyError:
# Dependency not found in graph, it was probably removed while dmypy was running.
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm afraid this will result in inconsistent state eventually. Would be really helpful to see a reproducer, but in general something like this (modules changed while (d)mypy is running) is really difficult to support. I can envision a solution that restarts the whole processing upon encountering such condition (and that should be moderately easy to support), we need to be really careful with such patching.

Copy link
Author

Choose a reason for hiding this comment

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

I can try to find a more general fix, but just so it's a 100% clear, is dmypy supposed to support the following use case?

  • I run dmypy run ., and see the results.
  • I do various changes in my codebase (possibly switching branches), which results in different packages being removed or added
  • I run dmypy run . again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Definitely yes, and we have a bunch of tests attesting that. I don't use dmypy personally, but I've made a couple of fixes related to such module structure changes in 1.16. Does your problem reproduce on latest mypy version?

I must have misinterpreted your PR description as "file is deleted while dmypy is running", which is indeed likely unsupported.

Copy link
Author

Choose a reason for hiding this comment

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

Just tested with 1.16.0 and sadly the issue is still there.

Thanks for the feedback, I'll see whether I can make a reproducible test and potentially a better fix.


def find_added_suppressed(
self, graph: mypy.build.Graph, seen: set[str], search_paths: SearchPaths
Expand Down