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

mingw-w64-x86_64-gcc's output is not highlighted #14087

Closed
ghost opened this issue Nov 15, 2022 · 18 comments · Fixed by #15036
Closed

mingw-w64-x86_64-gcc's output is not highlighted #14087

ghost opened this issue Nov 15, 2022 · 18 comments · Fixed by #15036
Labels

Comments

@ghost
Copy link

ghost commented Nov 15, 2022

Only when using the Mintty terminal by the MSYS2 MINGW64 shortcut. If using Windows Command Prompt (adding C:\msys64\mingw64\bin to PATH) then it's correctly highlighted. How could I have it correctly highlighted with the Mintty terminal? Thanks.

@Biswa96
Copy link
Member

Biswa96 commented Nov 15, 2022

Can you provide any example or steps to reproduce your issue? Sometimes, uncolored output is an expected behavior.

@ghost
Copy link
Author

ghost commented Nov 15, 2022

Can you provide any example or steps to reproduce your issue? Sometimes, uncolored output is an expected behavior.

It's always uncolored when using on the Mintty terminal. You can reproduce it by compiling any source code that trigger warnings when compiling. In my case it's CopperSpice.

Update: if CopperSpice is too big, this is a more lightweight codebase to reproduce the problem: https://github.com/brwhale/KataScript

@lazka
Copy link
Member

lazka commented Nov 17, 2022

Which Windows version? Is you MSYS2 install up-to-date?

@ghost
Copy link
Author

ghost commented Nov 17, 2022

Which Windows version? Is you MSYS2 install up-to-date?

Windows 8.1 64 bit. My MSYS2 is up-to-date.

@revelator
Copy link
Contributor

revelator commented Nov 19, 2022

does copperspice use msys make or meson/ninja i noticed make correctly highlights the output when msys make is used and colormake is installed but meson and ninja builds are allways mono colored.

@lazka
Copy link
Member

lazka commented Nov 20, 2022

I think the cygwin pipe name patch was never updated for the new (pipe) naming: https://github.com/msys2/MINGW-packages/blob/ec4adeaf30323d25f694c51c729bd288e4f9b002/mingw-w64-gcc/0140-gcc-8.2.0-diagnostic-color.patch

@revelator
Copy link
Contributor

whoops x-)

@Zapeth
Copy link
Contributor

Zapeth commented Dec 31, 2022

Since this issue is still open and I get the same behavior with current mingw-w64-gcc (12.2.0-7) I'm guessing the patch still needs updating?

@lazka Do you have any links/more information about the new pipe naming?

Also found this older issue that might be a duplicate (just to link them) -> #8600

@Biswa96
Copy link
Member

Biswa96 commented Dec 31, 2022

Try to add -fdiagnostics-color=always compiler flag. Did it work?

@Zapeth
Copy link
Contributor

Zapeth commented Dec 31, 2022

Yes that works, but I'd prefer not having to explicitly set that flag

@Biswa96
Copy link
Member

Biswa96 commented Dec 31, 2022

I think the cygwin pipe name patch was never updated for the new (pipe) naming

@lazka Is that patch required now? I have created a small program with that patch's code. Using STD_ERROR_HANDLE as handle, the OBJECT_NAME_INFORMATION::Name.Buffer returns

  1. \Device\ConDrv by default as conpty is enabled.
  2. \Device\NamedPipe\msys-81cdd03249b980f0-pty1-to-master-nat when conpty is disabled.

In the gcc code, the return value of GetConsoleMode is checked. That check will return true because the std i/o handles are now console handles by default (case 1).

@Biswa96 Biswa96 added the bug label Dec 31, 2022
@Zapeth
Copy link
Contributor

Zapeth commented Dec 31, 2022

In the gcc code, the return value of GetConsoleMode is checked. That check will return true because the std i/o handles are now console handles by default (case 1).

If I understand it correctly this is only the case for Win10 and newer. I tried the code on Win7 and GetConsoleMode fails with GetLastError returning ERROR_INVALID_HANDLE (though I'm still checking if I did something wrong) Apparently an alternative to get the stderr handle is with CreateFile("CONOUT$",..) which seems to work, but then OBJECT_NAME_INFORMATION::Name.Buffer returns \KnownDlls, so not sure whats up with that.

I know Win7 isn't supported anymore, but at least Win8 might have the same behavior.

@Biswa96
Copy link
Member

Biswa96 commented Dec 31, 2022

I tried the code on Win7 and GetConsoleMode fails with GetLastError returning ERROR_INVALID_HANDLE

You are correct, Windows 7 does not have ConPTY feature. So, the standard handles are pipes. If we consider all supported Windows versions the 0140-gcc-8.2.0-diagnostic-color.patch has to also check for Windows version + \Device\ConDrv cases.

@Zapeth
Copy link
Contributor

Zapeth commented Jan 1, 2023

If we consider all supported Windows versions the 0140-gcc-8.2.0-diagnostic-color.patch has to also check for Windows version + \Device\ConDrv cases.

Assuming GetConsoleMode doesn't fail in case of ConPTY then a version check shouldn't be necessary.

Anyway, I updated the patch to check for -to-master-nat postfix instead -> https://gist.github.com/Zapeth/10c4f4cd10be575cea447cf1f31feb13

I also tried to hack the binary by forcing the check to always return true, but for some reason it didn't have any effect (and I'm pretty sure I modified the correct function).

But maybe I overlooked something, so I'm hoping this small adjustment to the patch will suffice.

@Zapeth
Copy link
Contributor

Zapeth commented Jan 7, 2023

I have tested a version compiled with the above patch and can confirm that the output is coloured again on Win7 without having to force it.

Though maybe checking instead for just \Device\NamedPipe\msys-, or changing the memcmp for -to-master to a wcsstr would be more reliable, any thoughts?

@Biswa96
Copy link
Member

Biswa96 commented Jan 17, 2023

Windows 7 support has been discontinued https://www.msys2.org/news/#2023-01-15-dropping-support-for-windows-7-and-80. The 0140-gcc-8.2.0-diagnostic-color.patch is no longer required and can be removed.

@lazka
Copy link
Member

lazka commented Jan 17, 2023

(Windows 8.1 would still require it)

@Zapeth
Copy link
Contributor

Zapeth commented Jan 18, 2023

Unfortunately I only have a Win7 system available for mingw testing, maybe someone with a Win8.1/Win10 can give some feedback what the current behavior is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants