Skip to content

Fix dmypy on windows when compiled with mypyc #6365

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 1 commit into from
Feb 13, 2019
Merged

Conversation

msullivan
Copy link
Collaborator

dmypy currently crashes on windows when compiled with mypyc
(since 0.660; it has never worked, I believe).

The root cause here is the abusive use of the platform variable,
which we set to 'mypyc', to tell bogus_type.py that mypyc is being used.
This (obviously) interferes with our platform checks, causing mypyc to
spuriously think code is unreachable, but because of the minutae of
how our platform checks were structured and which files are compiled,
this only breaks things for the windows daemon.

Switch to using always_true/always_false instead of abusing platform.

There is a downside to this fix, though (which is the reason I abused platform in the first place instead of using always_true/always_false): mypy will no longer typecheck without its config file, since it needs always_false = MYPYC to be set. If this is a major problem, I'll need to look for another solution.

dmypy currently crashes on windows when compiled with mypyc
(since 0.660; it has *never* worked, I believe).

The root cause here is the abusive use of the platform variable,
which we set to 'mypyc', to tell bogus_type.py that mypyc is being used.
This (obviously) interferes with our platform checks, causing mypyc to
spuriously think code is unreachable, but because of the minutae of
how our platform checks were structured and which files are compiled,
this only breaks things for the windows daemon.

Switch to using always_true/always_false instead of abusing platform.
@msullivan
Copy link
Collaborator Author

If we don't want to require using mypy_self_check.ini for mypy to typecheck, I've got another approach using --shadow-file that will work.

We could also make it so that mypy understands MYPYC and has it default to being always false without it needing to be specified.

@msullivan
Copy link
Collaborator Author

There's also a question of whether this justifies a 0.671. On one hand, it is really bad. On the other hand, nobody noticed it in 0.660, where it was also present.

@gvanrossum
Copy link
Member

In general our policy is, if it's not a regression it doesn't deserve a bugfix release.

I don't mind that it requires a config file to type-check -- in fact I've frequently forgotten to pass that and found that I'd made some typing mistake only when CI called it out.

@msullivan
Copy link
Collaborator Author

Thinking about it more I seem to recall there might be some typeshed-related issue? @JelleZijlstra do you remember if that is the case?

@JelleZijlstra
Copy link
Member

Do you mean whether there is a typeshed issue that might justify releasing 0.671? The only problem I've seen reported is the most recent comment at python/typeshed#2760, which doesn't seem too bad.

@msullivan
Copy link
Collaborator Author

No, I mean an issue with the typeshed tests if mypy only typechecks with a configuration file present

@JelleZijlstra
Copy link
Member

Oh, sorry. I'm not aware of anything specific, but the typeshed tests do run some of the mypy test suite, including the self check. If anything breaks there's it's surely fixable by tweaking the Travis configuration.

@msullivan
Copy link
Collaborator Author

All right I think I want to merge this

@msullivan msullivan requested a review from gvanrossum February 13, 2019 22:52
Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

I think this makes sense. I look forward to being able to use mypyc-dmypy on Windows :P

@msullivan msullivan merged commit 385a27a into master Feb 13, 2019
@msullivan msullivan deleted the fix-platform-checks branch February 13, 2019 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants