Hi Takashi,

On Thu, 18 Dec 2025, Takashi Yano wrote:

> When I introduced support

Heh... That's what _I_ wrote ;-) All good, though, I appreciate your
effort to combine your and my patches.

> for executing Microsoft Store applications through their "app execution
> aliases" (i.e. special reparse points installed into
> %LOCALAPPDATA%\Microsoft\WindowsApps) in
> https://inbox.sourceware.org/cygwin-patches/[email protected]/,
> I had missed that it failed to spawn the process with the correct
> handles to the terminal, breaking interactive usage of, say, the Python
> interpreter.
> 
> This was later reported in
> https://inbox.sourceware.org/cygwin/CAAM_cieBo_M76sqZMGgF+tXxswvT=juhl_pshff+arv9p1e...@mail.gmail.com/t/#u,
> and also in https://github.com/python/pymanager/issues/210 (which was then
> re-reported in
> https://github.com/msys2/MSYS2-packages/issues/1943#issuecomment-3467583078).
> 
> The root cause is that the is_console_app() function required quite a bit of
> TLC, which this here patch series tries to provide.
> 
> Changes since v2: (v3 skipped)
> 
>  * Merge Takashi's v3 patch into Johaness's patch series.
>  * is_conslle_app() returns true when error happens.
>  * Implement new API path_conv::is_app_execution_alias().
>  * To determine if the path is an app execution alias in is_console_app(),
>    change argument of fhandler_termis::spawn_worker() and is_console_app()
>    from const WCHAR * to path_conv &, so that is_app_execution_alias()
>    can be called from is_console_app().
>  * Resolve reparse point when the path is an app execution alias.

I have reviewed these patches, and in particular love that you changed the
default return value of `is_console_app()` to true, in particular with the
added explanation in the commit message that it actually does not hurt GUI
apps much at all.

While I still think it would be better to split 5/5 into a patch that
changes the function signature of `is_console_app()` and then a patch that
adds special handling for app execution aliases, and while I still think
that the commit message could be improved, at this point I do not want to
force you to work on this even more than you already have, and therefore I
would be okay with this patch series to be integrated as-is.

I truly appreciate the effort you put into this.

Thank you,
Johannes

> 
> Changes since v1:
> 
>  * Amended the commit messages with "Fixes:" footers.
>  * Added a code comment to is_console_app() to clarify why a simple
>    CreateFile() is not enough in the case of app execution aliases.
> 
> Johannes Schindelin (2):
>   Cygwin: is_console_app(): do handle errors
>   Cygwin: is_console_app(): deal with the `.bat`/`.cmd` file extensions
>     first
> 
> Takashi Yano (3):
>   Cygwin: termios: Make is_console_app() return true for unknown
>   Cygwin: path: Implement path_conv::is_app_execution_alias()
>   Cygwin: termios: Handle app execution alias in is_console_app()
> 
>  winsup/cygwin/fhandler/termios.cc       | 37 +++++++++++++++++++------
>  winsup/cygwin/local_includes/fhandler.h |  2 +-
>  winsup/cygwin/local_includes/path.h     |  5 ++++
>  winsup/cygwin/path.cc                   |  2 +-
>  winsup/cygwin/spawn.cc                  |  2 +-
>  5 files changed, 36 insertions(+), 12 deletions(-)
> 
> -- 
> 2.51.0
> 
> 

Reply via email to