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 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 (3):
  Cygwin: is_console_app(): do handle errors
  Cygwin: is_console_app(): deal with the `.bat`/`.cmd` file extensions
    first
  Cygwin: is_console_app(): handle app execution aliases

 winsup/cygwin/fhandler/termios.cc | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)


base-commit: 7d43942e7c3b56799e9e46c4701f86a8eb0ed579
Published-As: 
https://github.com/cygwingitgadget/cygwin/releases/tag/pr-5%2Fdscho%2Ffix-stdio-with-app-exec-aliases-v2
Fetch-It-Via: git fetch https://github.com/cygwingitgadget/cygwin 
pr-5/dscho/fix-stdio-with-app-exec-aliases-v2
Pull-Request: https://github.com/cygwingitgadget/cygwin/pull/5

Range-diff vs v1:

 1:  7edad15ac ! 1:  af87bd1d4 Cygwin: is_console_app(): do handle errors
     @@ Metadata
       ## Commit message ##
          Cygwin: is_console_app(): do handle errors
      
     -    When that function was introduced in bb42852062 (Cygwin: pty: 
Implement
     +    When that function was introduced in bb4285206207 (Cygwin: pty: 
Implement
          new pseudo console support., 2020-08-19) (back then, it was added to
     -    `spawn.cc`, later it was moved to `fhandler/termios.cc` in 32d6a6cb5f
     +    `spawn.cc`, later it was moved to `fhandler/termios.cc` in 
32d6a6cb5f1e
          (Cygwin: pty, console: Encapsulate spawn.cc code related to
          pty/console., 2022-11-19)), it was implemented with strong assumptions
          that neither creating the file handle nor reading 1024 bytes from said
     @@ Commit message
      
          Let's add some error handling to that function.
      
     +    Fixed: bb4285206207 (Cygwin: pty: Implement new pseudo console 
support., 2020-08-19)
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       ## winsup/cygwin/fhandler/termios.cc ##
 2:  535cc52d4 ! 2:  8e9732407 Cygwin: is_console_app(): deal with the 
`.bat`/`.cmd` file extensions first
     @@ Commit message
          Let's honor the best practice to deal with easy conditions that allow
          early returns first.
      
     +    Fixed: bb4285206207 (Cygwin: pty: Implement new pseudo console 
suppot., 2020-08-19)
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       ## winsup/cygwin/fhandler/termios.cc ##
 3:  6ae42c5d1 ! 3:  e6101afa7 Cygwin: is_console_app(): handle app execution 
aliases
     @@ Metadata
       ## Commit message ##
          Cygwin: is_console_app(): handle app execution aliases
      
     -    In 0a9ee3ea23 (Allow executing Windows Store's "app execution 
aliases",
     +    In 2533912fc76c (Allow executing Windows Store's "app execution 
aliases",
          2021-03-12), I introduced support for calling Microsoft Store
          applications.
      
     @@ Commit message
          simply open the first few bytes of the `.exe` file to read the PE 
header
          in order to determine whether it is a console application or not.
      
     -    For app execution aliases, already creating a regular file handle for
     -    reading will fail. Let's introduce some special handling for the exact
     -    error code returned in those instances, and try to read the symlink
     -    target instead (taking advantage of the code I introduced in 
0631c6644e
     -    (Cygwin: Treat Windows Store's "app execution aliases" as symbolic
     -    links, 2021-03-22) to treat app execution aliases like symbolic 
links).
     +    For app execution aliases, already creating a regular file handle
     +    for reading will fail. Let's introduce some special handling for the
     +    exact error code returned in those instances, and try to read the
     +    symlink target instead (taking advantage of the code I introduced in
     +    0631c6644e63 (Cygwin: Treat Windows Store's "app execution aliases" as
     +    symbolic links, 2021-03-22) to treat app execution aliases like 
symbolic
     +    links).
      
     +    Fixes: 2533912fc76c (Allow executing Windows Store's "app execution 
aliases", 2021-03-12)
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       ## winsup/cygwin/fhandler/termios.cc ##
     @@ winsup/cygwin/fhandler/termios.cc: is_console_app (const WCHAR 
*filename)
         HANDLE h;
         h = CreateFileW (filename, GENERIC_READ, FILE_SHARE_READ,
                   NULL, OPEN_EXISTING, 0, NULL);
     ++  /* The "app execution aliases", i.e. the reparse points installed into
     ++     `%LOCALAPPDATA%\Microsoft\WindowsApps` for Microsoft Store apps 
cannot be
     ++     opened for reading via `CreateFile(..,. GENERIC_READ, ...)`, 
failing with
     ++     ERROR_CANT_ACCESS_FILE. Therefore, whenever that error is 
encountered,
     ++     let's see whether it is a reparse point and if it is, open the 
target
     ++     file instead. */
      +  if (h == INVALID_HANDLE_VALUE && GetLastError () == 
ERROR_CANT_ACCESS_FILE)
      +    {
      +      UNICODE_STRING ustr;

-- 
cygwingitgadget

Reply via email to