Hi Takashi,

On Fri, 19 Dec 2025, Takashi Yano wrote:

> From: Johannes Schindelin <[email protected]>
> 
> When I introduced support 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 v4:
> 
>  * Split 5/5 patch into two patches: one is for changing argument type,
>    the other is for fixing a bug.
>  * Improve commit message of 5/5 patch.

Thank you so much for indulging me! Both changes look very good to me. I
verified that the cumulative diff is identical, and the commit messages
look good to me, too.

Here is the range-diff:

-- snip --
1:  58110721b7 ! 1:  23748c821a Cygwin: termios: Make is_console_app() return 
true for unknown
    @@ Commit message
         GUI apps is poinless indeed, but not unsafe.
     
         Fixes: bb4285206207 ("Cygwin: pty: Implement new pseudo console 
support.")
    -    Reviewed-by:
    +    Reviewed-by: Johannes Schindelin <[email protected]>
         Signed-off-by: Takashi Yano <[email protected]>
         Signed-off-by: Johannes Schindelin <[email protected]>
     
2:  554422de5f = 2:  dd892f002a Cygwin: is_console_app(): do handle errors
3:  1f87881d8d ! 3:  697457a34b Cygwin: is_console_app(): deal with the 
`.bat`/`.cmd` file extensions first
    @@ Commit message
         early returns first.
     
         Fixes: bb4285206207 (Cygwin: pty: Implement new pseudo console 
suppot., 2020-08-19)
    +    Reviewed-by: Takashi Yano <[email protected]>
         Signed-off-by: Johannes Schindelin <[email protected]>
     
      ## winsup/cygwin/fhandler/termios.cc ##
4:  284feb0e78 ! 4:  835bcd9d01 Cygwin: path: Implement 
path_conv::is_app_execution_alias()
    @@ Commit message
         This patch adds new api path_conv::is_app_execution_alias() for
         that purpose.
     
    -    Reviewed-by:
    +    Reviewed-by: Johannes Schindelin <[email protected]>
         Signed-off-by: Takashi Yano <[email protected]>
         Signed-off-by: Johannes Schindelin <[email protected]>
     
5:  cf0d96afcd ! 5:  a18fe29ee4 Cygwin: termios: Handle app execution alias in 
is_console_app()
    @@ Metadata
     Author: Takashi Yano <[email protected]>
     
      ## Commit message ##
    -    Cygwin: termios: Handle app execution alias in is_console_app()
    +    Cygwin: termios: Change argument of fhandler_termios::spawn_worker()
     
    -    After the commit f74dc93c6359, WSL cannot start by distribution name
    -    such as debian.exe, which has '.exe' extention but actually is an app
    -    execution alias. This is because the commit f74dc93c6359 disabled to
    -    follow windows reparse point by adding PC_SYM_NOFOLLOW_REP flag in
    -    spawn.cc, that path is used for sapwning a process. As a result, the
    -    path, that is_console_app () received, had been the reparse point of
    -    app execution alias, then it returned false for the the path due to
    -    open-failure because CreateFileW() cannot open an app execution alias,
    -    while it can open normal reparse point.  If is_console_app() returns
    -    false, standard handles for console app (such as WSL) would not be
    -    setup. This causes that the console input cannot be transfered to the
    -    non-cygwin app.
    +    This patch changes the argument for passsing a path to an app
    +    to fhandler_termios::spawn_worker() from const WCHAR *runpath
    +    to path_conv &pc. The purpose of this patch is to prepare for
    +    a subsequent patch, that is intended to fix a bug in executing
    +    Microsoft Store apps.
     
    -    This patch fixes the issue by locally converting the path, which is
    -    a path to the app execution alias, once again using PC_SYM_FOLLOW
    -    (without PC_SYM_NOFOLLOW_REP) option path_conv for using inside of
    -    is_console_app() to resolve the reparse point here, if the path is
    -    an app execution alias.
    -
    -    Fixes: f74dc93c6359 ("fix native symlink spawn passing wrong arg0")
         Reviewed-by: Johannes Schindelin <[email protected]>
         Signed-off-by: Takashi Yano <[email protected]>
         Signed-off-by: Johannes Schindelin <[email protected]>
    @@ winsup/cygwin/fhandler/termios.cc: fhandler_termios::fstat (struct stat 
*buf)
     +  wchar_t *e = wcsrchr (native_path, L'.');
        if (e && (wcscasecmp (e, L".bat") == 0 || wcscasecmp (e, L".cmd") == 0))
          return true;
    -+
    -+  if (pc.is_app_execution_alias ())
    -+    {
    -+      UNICODE_STRING upath;
    -+      RtlInitUnicodeString (&upath, native_path);
    -+      path_conv target (&upath, PC_SYM_FOLLOW);
    -+      target.get_wide_win32_path (native_path);
    -+    }
    -+
        HANDLE h;
     -  h = CreateFileW (filename, GENERIC_READ, FILE_SHARE_READ,
     +  h = CreateFileW (native_path, GENERIC_READ, FILE_SHARE_READ,
-:  ---------- > 6:  f27c915a94 Cygwin: termios: Handle app execution alias in 
is_console_app()
-- snap --

Again: Thank you very much!
Johannes

> 
> 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.
> 
> 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 (4):
>   Cygwin: termios: Make is_console_app() return true for unknown
>   Cygwin: path: Implement path_conv::is_app_execution_alias()
>   Cygwin: termios: Change argument of fhandler_termios::spawn_worker()
>   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