Fix interaction between --incremental and --silent-imports. #1383
Conversation
If a dependency appears or disappears we now mark the dependent module as stale.
|
I'm not that familiar with this part of the code, but I wonder if it would be possible (and a bit conceptually cleaner) to stop removing suppressed imports from dependencies and then add a special serialization mode for them. I think that might make it Just Work with the rest of the current logic, which would be pretty nice. |
|
I looked into that, and it wouldn't be that clean -- we'd have to create a
dummy State with a dummy MypyFile (tree + symbol table) AND set a flag
somewhere indicating that all imports of that dummy module should still be
made to return a dummy object of type Any. Currently all of that is handled
by the missing_modules dict.
|
| @@ -318,6 +318,7 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int], | |||
| ('dependencies', List[str]), | |||
| ('data_mtime', float), # mtime of data_json | |||
| ('data_json', str), # path of <id>.data.json | |||
| ('suppressed', List[str]), | |||
| ('data_mtime', float), # mtime of data_json | ||
| ('data_json', str), # path of <id>.data.json | ||
| ('suppressed', List[str]), | ||
| ('suppressed', List[str]), # dependencies that weren't imported |
There was a problem hiding this comment.
This is still a little ambiguous, as we can skip some modules due to conditional imports as well. I suggest explaining what this is used for and when.
|
Looks reasonable to me, other than the nit about a comment. The logic is pretty delicate so it would be nice to have tests for this -- if you don't want to do this now, please create an issue for it. |
| ('data_mtime', float), # mtime of data_json | ||
| ('data_json', str), # path of <id>.data.json | ||
| ('suppressed', List[str]), # dependencies that weren't imported |
There was a problem hiding this comment.
This is still a little ambiguous, as we can skip some modules due to conditional imports as well. I suggest explaining what this is used for and when. [I think that github incorrectly hid my previous comment.]
|
Have a look at the hidden comment just below the second commit -- github is hiding it for me for some reason. |
|
I think I got it now. I added a longer comment explaining dependencies and
suppressed. I also added a note to the existing issue requesting unit tests
for --incremental to specifically test the interaction between -i and -s.
The code I currently have in testcheck.py for incremental tests can't quite
handle this, I think -- I will need to add a way to specify the list of
files included on the command line. That's also needed to test -s without
-i, so it's a good idea to add this anyway.
|
If a dependency appears or disappears we now mark the dependent module as stale.
Tested manually by toggling the presence of a file on the command line in a simple scenario.