Skip to content
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

Incorrect platform detection when using MSYS2 MINGW64 #2355

Closed
dlly11 opened this issue Sep 18, 2022 · 9 comments · Fixed by #2554
Closed

Incorrect platform detection when using MSYS2 MINGW64 #2355

dlly11 opened this issue Sep 18, 2022 · 9 comments · Fixed by #2554
Milestone

Comments

@dlly11
Copy link

dlly11 commented Sep 18, 2022

Description:

When running click in MSYS2 MINGW64 Python environment, click is trying to import tty. This fails due to termios not being a package supported by Windows.
This happens because of the platform detection in _compat.py. The following code detects the platform as being MSYS2 which then tries to use MSYS2 compatible packages:
MSYS2 = sys.platform.startswith("win") and ("GCC" in sys.version)

This does not work when using python packaged with MINGW64 as it still detects the platform as being MSYS2.

Reproduce:

Open a progress bar in MSYS2 MINGW64 Python3 i.e.
with click.progressbar(length=10, label="Downloading") as pbar:

Traceback (most recent call last):
File "C:\msys64\mingw64\lib\python3.10\site-packages\apio\managers\installer.py", line 200, in install
dlpath = self._download(platform_download_url)
File "C:\msys64\mingw64\lib\python3.10\site-packages\apio\managers\installer.py", line 382, in _download
filed.start()
File "C:\msys64\mingw64\lib\python3.10\site-packages\apio\managers\downloader.py", line 68, in start
with click.progressbar(length=chunks, label="Downloading") as pbar:
File "C:\msys64\mingw64\lib\python3.10\site-packages\click\termui.py", line 394, in progressbar
from ._termui_impl import ProgressBar
File "C:\msys64\mingw64\lib\python3.10\site-packages\click_termui_impl.py", line 626, in
import tty
File "C:\msys64\mingw64\lib\python3.10\tty.py", line 5, in
from termios import *
ModuleNotFoundError: No module named 'termios'

Expected Behaviour:

Expect the platform detection to detect the platform as Windows and not MSYS2.

Environment:

  • Platform: MSYS2 MINGW64
  • Python version: 3.10
  • Click version: 7.1.2
@davidism
Copy link
Member

Happy to review a PR. Note that both mingw and msys are not officially supported platforms by us, and are not guaranteed to work.

@dlly11
Copy link
Author

dlly11 commented Sep 18, 2022

Thanks, I may have a fix that involves using platform.system() instead. Just need to check it across different Windows platforms.

This comment is what I am basing it off. I did just install python in the MSYS2 environment and sys.platform is returning cygwin for it so that might mean something has changed with how python is packaged in MSYS.

@dlly11
Copy link
Author

dlly11 commented Sep 18, 2022

@davidism I see that the change introduced here is now causing this fail. Simply reverting this change seems to work on MSYS2 MINGW64.

I ran the Pytests with the regular Windows python 3.10, the MSYS2 python 3.10 and the MINGW64 python 3.10.
Windows and MINGW64 passed. MSYS2 failed before and after the reverted change however there was no difference what failed.

@davidism
Copy link
Member

Thanks for looking at this. However, the previous issue and PR indicate that it was correct at the time because MSYS2 used Bash. Why is removing this correct now? What has changed, and why?

@dlly11
Copy link
Author

dlly11 commented Sep 18, 2022

Hi, I'll have to look through the changes in the MSYS2 repo to see if anything has changed.

I'm seeing python report sys.platform on MSYS as being cygwin so maybe the package maintainers changed how the platform appears to python to resolve issues like this?

I don't think checking for the string GCC is the correct way to go about checking for the platform. Both MSYS and MINGW64 compile python with GCC from what I've gathered. MSYS executables use the POSIX emulation layer but I don't think the MINGW ones do. I haven't checked the clang repos either.

I'll get back to you with what I can find.

@3b
Copy link

3b commented Nov 11, 2022

Ran into this from platformio/platformio-core#3597 (not sure why they blamed cygwin) and investigated a bit. I think reverting #1393 is the correct solution. I think #1338 was fixed more correctly by #1135, and #1393 was misguided to start with.

There are 2 distinct issues:

For this bug, the question is whether the running python can or should try to load tty/termios libraries. Msys and cygwin provide a compatibility layer that would make that work, so loading them in a python installed from msys/python or from cygwin is valid. Other pythons on windows (in particular mingw64/mingw-w64-x86_64-python from msys) don't have that and will fail as above. #1393 doesn't distinguish between msys/ and mingw64/ and loads termios on both, so if it were kept more tests would have to be done to do this correctly if you really wanted to load termios on msys/python.

For #1338, the question is whether stdout is a "windows console" as you would get from conhost.exe or a few other terminal programs, or if it is a named pipe as you would get from various ports of unix terminals, in particular the mintty from msys. That is mostly independent of which python you are using, since you can run msys bash in a windows console, and you can run cmd.exe in a msys mintty, or you could run either python directly in either terminal without any shell. Msys users are probably more likely to run unix-like terminals, so there is some correlation, but #1393 still gets that wrong since msys2 also has ucrt64/mingw-w64-ucrt-x86_64-python and clang64/mingw-w64-clang-x86_64-python which it would miss. #1135 just directly checks for whether we have a console or not, which seems like a better and more general solution.

To complicate it further, on recent win 10 conhost can be made to understand utf8+vt100 instead of using console color API (colorama does this if possible now i think?), and recent mintty uses the ConPTY API to allow it to understand the console APIs. The new windows terminal similarly works with either. None of that helps with the original reporter using 8.1, but it does make it harder to test things without an actual 8.1 install. Testing in mintty with ConPTY disabled does seem to show that #1338 is still a problem without either #1135 or #1393 though, and that #1135 is enough to fix it.

@davidism davidism added this to the 8.1.4 milestone Jul 4, 2023
@davidism
Copy link
Member

davidism commented Jul 6, 2023

@3b thanks for the detailed analysis. It sounds like we should revert #1393 and make sure #1135 is still intact.

Testing in mintty with ConPTY disabled

Could you describe how to do that?


I'm not a Windows user or developer, all these additional systems are not familiar to me. From some research, it seems like the modern system is msys2, which uses mingw-w64. I'm not sure how accurate all of the following is, but it's what I found trying to figure out the different ways Python might be available on Windows.

  • Python's official Windows build is a native Windows program.
    • py -c 'import sys, os; print(sys.platform, os.name)' outputs win32 nt.
    • py -c 'import tty' fails.
    • The Microsoft Store also provides python (and asks to install it if you type python instead of py). It behaves the same.
  • msys2 provides different environments, which target an architecture and C runtime. It provides packages built for each environment, including Python. It uses mingw-w64 to build those packages.
    • Install msys2 using scoop install msys2.
    • Run ucrt64 to get a terminal with the ucrt environment.
    • Run pacman -S mingw-w64-ucrt-x86_64-python to install the python command in that environment.
    • python -c 'import sys, os; print(sys.platform, os.name)' outputs win32 nt.
    • python -c 'import tty' fails.
  • Git for Windows, and its Git Bash program, use msys2 and mingw-w64.
  • msys2 also has an "internal" environment based on cygwin.
    • Run msys2 to get a terminal.
    • Run pacman -S python to install the python command.
    • python -c 'import sys, os; print(sys.platform, os.name)' outputs cygwin posix.
    • python -c 'import tty' does not fail.
  • msys2 is an updated fork of msys, and mingw-w64 is an updated fork of mingw.
    • Install msys using scoop install msys.
    • Run msys to get a terminal.
    • It's not clear how to install Python with msys and mingw, so I didn't test that.
  • cygwin is the oldest project. msys and msys2, and the packages they build, are modified from cygwin.
    • Install cygwin using scoop install cygwin.
    • From the Start menu, select "Cygwin Setup" to get the package installer. Search for "python3" and select a version to install.
    • Run cygwin to get a terminal.
    • python -c 'import sys, os; print(sys.platform, os.name)' outputs cygwin posix.
    • python -c 'import tty' does not fail.
  • It's possible to run py from within all these different environments. It still behaves the same as running from a Windows terminal, tty is not available.
  • It looks like Python built with cygwin sees a Linux-like environment, and Python built with mingw-w64 sees a Windows-like environment.

@davidism
Copy link
Member

davidism commented Jul 6, 2023

I've confirmed that click.echo and click.progressbar continue to work after reverting #1393, in both cygwin and win32 environments.

@3b
Copy link

3b commented Jul 6, 2023

Testing in mintty with ConPTY disabled

Could you describe how to do that?

I think i was using mintty --pcon off,

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants