-
Notifications
You must be signed in to change notification settings - Fork 263
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
Improve IS_WINDOWS_SYSTEM check #539
Comments
I have yet another use case to consider. From Git bash (a variant of MSYS/MINGW) I run the normal Windows python installation through winpty. In this scenario, VUnit should act exactly as it would running with CMD, but since #539 MSYSTEM is detected and unprocessed Linux escape codes are displayed instead. |
@ryanfitzsimon, that's interesting... I'd say that Git bash is "the same" as MSYS/MINGW. Therefore, your case should be the same as MSYS/MINGW with 'regular' Python artifacts. On the one hand, I'd like to know which simulator are you testing this with. On the other hand, would you mind checking the following?
We should fill the following tables:
*1 winpty required |
I agree. In the context of this issue, I think Git bash and MSYS2 should be effectively identical. Sorry if I didn't make this clear, but the important point of difference is the use of winpty.
Running under Git bash means that the environment variable The issue is that when running under winpty, VUnit should behave as it would in CMD even though it's actually being run from Git bash (i.e. |
Actually, I had to use winpty to mingw-native, and I forgot to mention it. I edited the tables now.
The point is: how can we know when is python being executed under winpty? Both, mingw-native and mingw-pacman, return 'nt', and MSYSTEM is set in both contexts. |
It seems that @ryanfitzsimon can you try |
This does work in my specific environment, but I don't know if relying on the absence of Perhaps additional option/s could be added to allow forcing a specific color printer, extending or replacing the existing Most programs I'm familiar with treat empty/unset environment variables the same way, so could |
We can check both MSYSTEM and TERM, so that TERM is only used to detect when winpty is being used. Anyway, let's ping @kraigher.
To preserve backwards compatibility,
I do think so. It would preserve the current behaviour. Do you want to submit a PR? |
We are hitting a similar issue when using |
I found that the terminal colors on Windows 10/Git Bash is fine. However, on Windows 7/Git Bash, they appear in the terminal like so:
In both cases, |
I submitted a PR to fix colour support on MSYS2/MINGW: #538.
The Python installation available on my Windows 10 machine was installed through pacman (MSYS' package manager). I'm using VUnit from MINGW32/MINGW64 shells, where ghdl is installed from a PKGBUILD file. Hence, everything works as expected in these environments.
Nonetheless, I found that the same Python installation can be used from either cmd or powershell, because
C:\msys64\usr\bin
is available in the PATH. Surprisingly, in these contextsos.name == 'posix'
. Therefore, I'd say that VUnit won't work as expected. I am unsure, because I have not tried with any simulator yet.For completeness, I think that the current check (
IS_WINDOWS_SYSTEM = os.name == 'nt'
) is only valid for cmd/poweshell as long as 'regular' Python artifacts are used (https://www.python.org/downloads/release/python-374/). I checked thatpython-3.7.4-embed-amd64.zip
returnsos.name == 'nt'
indeed.As a result, I'd say that VUnit might need to be revisited/fixed to support 'mixed' environemtns:
IS_WINDOWS_SYSTEM = true
and useWin32ColorPrinter
. I think this is not the case.IS_WINDOWS_SYSTEM = true
and useLinuxColorPrinter
. I think this is correct after merging fix: use LinuxColorPrinter if MSYSTEM envvar is set #538.The text was updated successfully, but these errors were encountered: