Fix OSError when running on MinGW64 with MSYS2

[danyeaw]

Fixes #1338.

In version 6.0, echo and prompt functions were modified to work with full unicode in the windows console by emulating an output stream. Since MSYS2 uses a bash console it was creating an OSError to apply these modifications when running in this environment.

This PR changes these modifications to only apply them if not running in a MSYS2 environment.

[danyeaw]

Thanks for the awesome library! ❤️ Any interest in merging these changes or will we need to fix this downstream?

[davidism]

Thanks for working on this, sorry I didn't comment sooner. This check seems like it could cause false positives. Is there no better way to detect that MinGW is used?

[danyeaw]

@davidism I am not sure how you could have GCC on a Windows platform without using MINGW. Was there a false positive you had in mind?

Downstream, MINGW patches sysconfig, so we could do something like:

try:
     from sysconfig import _POSIX_BUILD
     MSYS2 = _POSIX_BUILD
except ImportError:
     MSYS2 = False

This seems kind of messy compared to the solution I used though. 😄

[davidism]

I'm not super familiar with these types of tools, but what about others like Cygwin, MSYS2, or WSL? Do they require the same fix? Or do they get picked up incorrectly by this check?

[danyeaw]

@davidism I ended up installing Cygwin to test this out. You are correct in that Python installed in Cygwin is compiled with GCC as well. However, sys.platform.startswith('win') returns False, since sys.platform returns cygwin. I don't have access to WSL, but I understand that sys.platform returns linux. Click in Cygwin works fine currently with master and this branch.

I think my changes might be a little more clear if I did:
MSYS2 = sys.platform.startwith('win') and ('GCC' in sys.version)
and then:
WIN = sys.platform.startswith('win') and not APP_ENGINE and not MSYS2

and then remove my changes from the if statement. Do you agree? If so I can update my PR.

[ThiefMaster]

Yes, WSL is Linux as far as userspace is concerned (and sys.platform indeed returns 'linux' there)

[davidism]

I like the suggested more specific version.

[danyeaw]

@davidism I have updated the PR with the discussed changes 👍

[danyeaw]

Thanks @davidism

[davidism]

Thanks for the fix!

Right after I merged this, I noticed that it's very similar to issue #1065, although the fix in #1135 addresses the Windows API rather than detecting MSYS2. Any thoughts on their relation?

[davidism]

It appears that this fixed #1065, so I need to figure out if I should merge #1135 too. cc @segevfiner