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

add 'winpty' support when available #82612

Merged

Conversation

selescop
Copy link
Contributor

@selescop selescop commented Dec 5, 2024

Hello,

Some Python scripts need to access the underlying OS, like using redirections. These interactions go through TTY interface. On MinGW, these interactions/interfaces are called 'winpty'. => use them when available (harmless on platform *NIX platforms)

OS: Windows
env: MinGW
I rebuilt the windows-curses and overwrite the built-in Python ncurses libraries.
When calling:

make/ninja menuconfig

You get an "Redirection is not supported" error.
kconfig python script uses curses to draw in the console. As in MinGW, the default TTY is not an interactive one, cbreak() fails. To overcome this "lack", you must use pdcurses. To make the redirection work, you must call Python on top of the winpty interface.

This proposed patch looks for winpty and, if installed, use it when calling 1 one the 3 Kconfig targets (menuconfig guiconfig hardenconfig). This should be harmless on system without winpty installed: *NIX ones

Some Python scripts need to access to the underlying OS, like
using redirections. These interactions go through TTY interface.
On MinGW, these interactions/interfaces are called 'winpty'.
=> use them when available (harmless on platform *NIX platforms)

Signed-off-by: Cedric Lescop <[email protected]>
@selescop selescop force-pushed the hotfix/add_winpty_support branch from 6b8061c to 6b5e02e Compare December 5, 2024 15:11
@tejlmand
Copy link
Collaborator

tejlmand commented Dec 6, 2024

MinGW is not officially supported, and it takes a much larger effort to truly support building Zephyr under MinGW.
This has been discussed in various places, for example like here: #45383 (comment)

This PR might look like a small addition, but the main issue / risk I see is that we might end-up with several small adjustments like this but without a proper plan / path to fully support MinGW.

How will this code behave for users who might have MinGW and winpty installed, but is otherwise building using windows command-line ?
I have a windows setup, but not a mingw / winpty, so I can't test or verify.

Based on description I found of winpty, then afaict it should also be possible for the user to directly specify the use of winpty when running ninja (or make), like this:

$ winpty ninja menuconfig

which again means one can use the alias feature, like this:

$ alias menuconfig="winpty ninja menuconfig"

and then just run menuconfig.

So far I see no good arguments on just this single patch, but opinions from others are more than welcome.

@selescop
Copy link
Contributor Author

selescop commented Dec 6, 2024

Hello @tejlmand,

MinGW is not officially supported, and it takes a much larger effort to truly support building Zephyr under MinGW.
This has been discussed in various places, for example like here: #45383 (comment)

Yes, as in my comment, everything is working fine now. I'm working with it for almost 2 years now and I didn't see any problems. Only a limitation on 'make menuconfig' because of curses/tty.

How will this code behave for users who might have MinGW and winpty installed, but is otherwise building using windows command-line ?

I just tried it. cmake.exe installed in MS Windows does not find winpty.exe (because it is not installed in MS Windows, but in MinGW). So, it will be harmless for these users. Also, I did not test all environments (I do not have MacOS for example)

$ winpty ninja menuconfig
Yes, this does work but this is more a workaround. Doing this will fix the issue for one user, not all of them. Someone who does not know about winpty might never get it to work...

@tejlmand
Copy link
Collaborator

tejlmand commented Dec 9, 2024

Yes, as in my #45392 (comment), everything is working fine now.

Thanks for the reference, and great to hear that it actually is working, although care must be taken.

If you want, then perhaps a page here: https://github.com/zephyrproject-rtos/zephyr/discussions/categories/show-and-tell would be good for others.
I think that's the second best, compared to being officially supported.

I just tried it. cmake.exe installed in MS Windows does not find winpty.exe

Thanks for verifying.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Comment by @selescop indicates that MinGW is working, #82612 (comment).

MSYS / MinGW is not officially supported, and making it officially supported would increase maintenance / test burden, but as @selescop has a working setup and this patch does improve and make things easier / simpler for MinGW users, then PR is approved.

Hopefully @selescop continues with a working MinGW setup, and will let us know if something no longer works.

@kartben kartben merged commit af9ae5b into zephyrproject-rtos:main Dec 12, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants