Repository navigation
Restore cached dependency order after suppress/add round trips - #22288
Closed
inchang-ing wants to merge 1 commit into
Closed
inchang-ing wants to merge 1 commit into
inchang-ing wants to merge 1 commit into
Conversation
A dependency that is suppressed during graph loading (the importing
module cannot resolve it) and up-suppressed afterwards (another module
resolved it) is re-added by appending it to the end of the dependency
list. The order-sensitive freshness check in State.is_fresh() then sees
a changed list and marks the module stale on every run, even though the
dependency set is identical to the cached one.
After the up-suppress pass, restore the cached order when the current
dependency set is unchanged; a genuine dependency change keeps the new
order and the module is still rechecked.
Reproducer (from the issue):
site/ns/sub/__init__.py (namespace package, no top-level __init__)
a.py: import ns.missing (missing submodule -> suppressed)
b.py: import ns.sub
e.py: import b
PYTHONPATH=site mypy -v a.py e.py # run twice
On the second run, 'Scheduling SCC singleton (a) as inherently stale'
was always logged: a's cached dependency order is
[os, builtins, ns, abc, typing], but the suppress/add round trip
appends ns at the end. With this change the second run reports no
stale SCCs. The second run also used to lose the 'Skipping analyzing
"ns"' error that the first (and non-incremental) run reports, because
the spurious recheck resolved ns differently; restoring the order fixes
that output inconsistency as well.
Fixes python#22159.
Contributor
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #22159.
During graph loading, a dependency that the importing module cannot resolve is suppressed; when another module later resolves it, the up-suppress pass adds it back — by appending it to the end of the dependency list. The order-sensitive freshness check in
State.is_fresh()then sees a changed list and marks the module stale on every run, even though the dependency set is identical to the cached one.After the up-suppress pass, restore the cached order when the current dependency set is unchanged. A genuine dependency change keeps the new order and the module is still rechecked, so the conservative stale-on-any-difference semantics is preserved.
Reproducer (from the issue)
a's cached dependency order is
[os, builtins, ns, abc, typing], but the suppress→add round trip re-appendsnsat the end, sois_fresh()fails on every run. With this change the second run schedules no stale SCCs.The spurious recheck also caused an output inconsistency: the second run dropped the
Skipping analyzing "ns"" error that the first (and non-incremental) run reports, because the recheck resolvedns` through a different path. Restoring the order fixes that too — incremental output now matches the fresh-run output for this repro.I could not express this layout (a
PYTHONPATH-installed PEP 420 namespace package whose submodule is imported by a different module) in a data-drivencheck-incrementaltest case; the steps above reproduce it in ~30 seconds and are what I used to verify before/after.Relationship to #22170
#22170 approached this by relaxing
is_fresh()to compare dependency membership (closed). This change instead keeps the strict order-sensitive comparison and removes the cause of the order churn inload_graph, so any real difference — including ones the membership check would have missed — still invalidates the cache.Test Plan
aand loses thenserror; after → no stale SCCs, output identical to the first run.pytest mypy/test/testcheck.py -k check-incremental— 357 passed, 4 skipped.pytest mypy/test/testcheck.py -k "check-fine-grained or serialize"— 90 passed.Disclosure
Assisted by AI coding tools (ZCode/GLM); I have reviewed and verified the change and can explain every line.