diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 0a73526e35..1444708120 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -32,28 +32,25 @@ jobs: $p = Get-ChildItem -Recurse "${env:RUNNER_TEMP}\artifacts" | where {$_.Name -eq "msys-2.0.dll"} | Select -ExpandProperty VersionInfo | Select -First 1 -ExpandProperty FileName cp $p "c:/Program Files/Git/usr/bin/msys-2.0.dll" + - uses: actions/checkout@v6 + with: + sparse-checkout: | + ui-tests + - uses: actions/cache/restore@v5 id: restore-wt with: key: wt-${{ env.WT_VERSION }} path: ${{ runner.temp }}/wt.zip - - name: Download Windows Terminal - if: steps.restore-wt.outputs.cache-hit != 'true' - shell: bash + - name: Install and configure portable Windows Terminal + working-directory: ui-tests run: | - curl -fLo "$RUNNER_TEMP/wt.zip" \ - https://github.com/microsoft/terminal/releases/download/v$WT_VERSION/Microsoft.WindowsTerminal_${WT_VERSION}_x64.zip + powershell -File setup-portable-wt.ps1 -WtVersion $env:WT_VERSION -DestDir $env:RUNNER_TEMP - uses: actions/cache/save@v5 if: steps.restore-wt.outputs.cache-hit != 'true' with: key: wt-${{ env.WT_VERSION }} path: ${{ runner.temp }}/wt.zip - - name: Install Windows Terminal - shell: bash - working-directory: ${{ runner.temp }} - run: | - "$WINDIR/system32/tar.exe" -xf "$RUNNER_TEMP/wt.zip" && - cygpath -aw terminal-$WT_VERSION >>$GITHUB_PATH - uses: actions/cache/restore@v5 id: restore-ahk with: @@ -99,10 +96,6 @@ jobs: "$WINDIR/system32/tar.exe" -C "$RUNNER_TEMP" -xvf "$RUNNER_TEMP/win32-openssh.zip" && echo "OPENSSH_FOR_WINDOWS_DIRECTORY=$(cygpath -aw "$RUNNER_TEMP/OpenSSH-Win64")" >>$GITHUB_ENV - - uses: actions/checkout@v6 - with: - sparse-checkout: | - ui-tests - name: Minimize existing Log window working-directory: ui-tests run: | @@ -124,6 +117,9 @@ jobs: & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force ctrl-c.ahk "$PWD\ctrl-c" 2>&1 | Out-Default if (!$?) { $exitCode = 1; echo "::error::Ctrl+C Test failed!" } else { echo "::notice::Ctrl+C Test log" } type ctrl-c.log + & "${env:RUNNER_TEMP}\ahk\AutoHotKey64.exe" /ErrorStdOut /force keystroke-order.ahk "$PWD\keystroke-order" 2>&1 | Out-Default + if (!$?) { $exitCode = 1; echo "::error::Keystroke-order Test failed!" } else { echo "::notice::Keystroke-order Test log" } + type keystroke-order.log exit $exitCode - name: Show logs if: always() @@ -131,6 +127,7 @@ jobs: run: | type bg-hook.log type ctrl-c.log + type keystroke-order.log - name: Take screenshot, if canceled id: take-screenshot if: cancelled() || failure() diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000..67b39459dd --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,319 @@ +# Guidelines for AI Agents Working on This Codebase + +## Project Overview + +This repository is the **Git for Windows fork** of the **MSYS2 runtime**, which is itself a fork of the **Cygwin runtime**. The runtime provides a POSIX emulation layer on Windows, producing `msys-2.0.dll` (analogous to Cygwin's `cygwin1.dll`). It is the foundational component that allows Unix-style programs (bash, coreutils, etc.) to run on Windows within the MSYS2 and Git for Windows ecosystems. + +### The Layered Fork Structure + +There are three layers of this project, each building on the one below: + +1. **Cygwin** (`git://sourceware.org/git/newlib-cygwin.git`, releases at https://cygwin.com): The upstream project. Cygwin is a POSIX-compatible environment for Windows consisting of a DLL (`cygwin1.dll`) that provides substantial POSIX API functionality, plus a collection of GNU and Open Source tools. The Cygwin project releases versioned tags (e.g., `cygwin-3.6.6`) from the `cygwin/cygwin` GitHub mirror. + +2. **MSYS2** (`https://github.com/msys2/msys2-runtime`): The MSYS2 project rebases its own patches on top of each Cygwin release. MSYS2 maintains branches named `msys2-X.Y.Z` (e.g., `msys2-3.6.6`) where the Cygwin code is the base and MSYS2-specific patches are applied on top. These patches implement features like POSIX-to-Windows path conversion (`msys2_path_conv.cc`), the `MSYS` environment variable for controlling runtime behavior, pseudo-console support toggling, and adaptations needed for MSYS2's focus on building native Windows software (as opposed to Cygwin's focus on running Unix software on Windows as-is). + +3. **Git for Windows** (`https://github.com/git-for-windows/msys2-runtime`, this repository): Git for Windows maintains a "merging rebase" on top of the MSYS2 patches. The `main` branch uses a special strategy where it always fast-forwards. Each rebase to a new upstream version starts with a "fake merge" commit (message: `Start the merging-rebase to cygwin-X.Y.Z`) that merges previous `main` using the `-s ours` strategy. This ensures the branch always fast-forwards despite being rebased. Git for Windows' own patches (on top of MSYS2's patches) address issues specific to Git's usage patterns, such as Ctrl+C signal handling, SSH hang fixes, and console output correctness. + +### Key Relationships + +- **Cygwin → MSYS2**: MSYS2 rebases onto each Cygwin release. When Cygwin releases version X.Y.Z, an `msys2-X.Y.Z` branch is created with MSYS2 patches rebased on top. +- **MSYS2 → Git for Windows**: Git for Windows performs a merging rebase that first merges in the MSYS2 patches, then rebases its own patches on top. +- The `main` branch in this repository (git-for-windows/msys2-runtime) is the Git for Windows branch, not Cygwin's or MSYS2's. + +## Repository Structure + +### Key Directories + +- **`winsup/cygwin/`**: The core of the Cygwin/MSYS2 runtime. This is where `msys-2.0.dll` (the POSIX emulation DLL) is built. Most development work happens here. Key files include: + - `dcrt0.cc`: Runtime initialization + - `spawn.cc`: Process spawning + - `path.cc`: Path handling + - `fork.cc`: fork() implementation + - `exceptions.cc`: Signal handling + - `msys2_path_conv.cc` / `msys2_path_conv.h`: MSYS2-specific POSIX-to-Windows path conversion (CC0-licensed) + - `environ.cc`: Environment variable handling, including the `MSYS` environment variable + - `fhandler/`: File handler implementations for various device types + - `local_includes/`: Internal headers + - `release/`: Version history files (one per Cygwin release version) +- **`winsup/utils/`**: Cygwin/MSYS2 utility programs (mount, cygpath, etc.) +- **`newlib/`**: The C library (newlib) used by the runtime +- **`ui-tests/`**: AutoHotKey-based integration tests that test the runtime in real terminal scenarios +- **`.github/workflows/`**: CI configuration + +## Build System + +### The Chicken-and-Egg Problem + +The MSYS2 runtime (`msys-2.0.dll`) is itself the POSIX emulation layer that the MSYS2 toolchain (GCC, binutils, etc.) depends on. The MSYS2 environment's own GCC links against `msys-2.0.dll` to provide POSIX semantics. This means you need a working MSYS2 runtime to compile a new MSYS2 runtime — a classic bootstrap problem. + +In practice, this is resolved by using an existing MSYS2 installation to build the new version. The CI workflow (`.github/workflows/build.yaml`) installs MSYS2 via the `msys2/setup-msys2` action, then builds the new runtime within that environment. + +### Build Dependencies + +Building requires MSYS2 packages: `msys2-devel`, `base-devel`, `autotools`, `cocom`, `gcc`, `gettext-devel`, `libiconv-devel`, `make`, `mingw-w64-cross-crt`, `mingw-w64-cross-gcc`, `mingw-w64-cross-zlib`, `perl`, `zlib-devel`. These are all **msys** packages (they link against `msys-2.0.dll`), not native MinGW packages. + +### Building in the Git for Windows SDK + +The Git for Windows SDK provides a complete MSYS2 environment with all necessary build dependencies pre-installed. The source tree is typically located at `/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime` inside the SDK. + +**Critical: PATH ordering.** The build must use the MSYS2 toolchain, not any MinGW toolchain that might be on the PATH. Before building, ensure: + +```bash +export PATH=/usr/bin:/mingw64/bin:/mingw32/bin:$PATH +``` + +If MinGW's GCC is found first, the build will fail because MinGW tools do not link against `msys-2.0.dll` and cannot produce the runtime DLL. + +### Build Commands + +```bash +# Generate autotools files +(cd winsup && ./autogen.sh) + +# Configure (the --with-msys2-runtime-commit flag embeds the commit hash) +./configure --disable-dependency-tracking --with-msys2-runtime-commit="$(git rev-parse HEAD)" + +# Build +make -j8 +``` + +For quick rebuilds of just the DLL during development: +```bash +# Rebuild only msys-2.0.dll +make -C ../build-x86_64-pc-msys/x*/winsup/cygwin -j15 new-msys-2.0.dll +``` + +The build output is `new-msys-2.0.dll` in the build directory. This is a staging name to avoid overwriting the running DLL. + +### Testing a Locally-Built DLL + +You cannot replace the SDK's own `msys-2.0.dll` while running inside the SDK — the DLL is loaded by every MSYS2 process including your shell. Instead, copy the built DLL into a separate installation such as a Portable Git: + +```bash +cp new-msys-2.0.dll /path/to/PortableGit/usr/bin/msys-2.0.dll +``` + +Then run tests using that Portable Git's mintty/bash. Back up the original DLL first. + +The `build-and-copy.sh` helper script in the repository root can reconfigure, rebuild, and copy `msys-2.0.dll` to a target location. + +### Internal API Constraints + +Code inside `msys-2.0.dll` cannot use the full C runtime or C++ standard library freely. Key limitations: + +- **`__small_sprintf`** is used instead of `sprintf`. It does NOT support `%lld` (64-bit integers) or floating-point format specifiers. For 64-bit values, split into high/low 32-bit halves and print as two `%u` values. +- **Memory allocation** in low-level code (e.g., DLL initialization, atexit handlers) should use `HeapAlloc(GetProcessHeap(), ...)` to avoid circular dependencies with the Cygwin malloc. + +### CI Pipeline + +The CI (`.github/workflows/build.yaml`) does the following: +1. **Build**: Compiles the runtime on `windows-latest` using MSYS2 +2. **Minimal SDK artifact**: Creates a minimal Git for Windows SDK with the just-built runtime, used for testing Git itself +3. **Test minimal SDK**: Runs Git's test suite against the new runtime +4. **UI tests**: AutoHotKey-based integration tests for terminal behavior (Ctrl+C interrupts, SSH operations, etc.) +5. **MSYS2 tests**: Runs the MSYS2 project's own test suite across multiple environments and compilers + +## Git Branch and Rebase Workflow + +### The Merging Rebase Strategy + +Git for Windows uses a "merging rebase" to maintain a fast-forwarding `main` branch. The key insight is a "fake merge" commit that: + +1. Starts from the new upstream commit (Cygwin tag) +2. Merges in the previous `main` using `-s ours` (takes NO changes from previous main, only the tree from upstream) +3. This makes `main` a parent of the new commit, so the result is a fast-forward from previous `main` +4. Patches are then rebased on top of this fake merge + +The commit message follows a strict format: `Start the merging-rebase to cygwin-X.Y.Z`. This is machine-parseable — `git rev-parse 'main^{/^Start.the.merging-rebase}'` finds the most recent such commit. + +### History of Merging Rebases + +The repository has been continuously rebased through Cygwin versions from 3.3.x through the current 3.6.6. Each rebase is visible as a `Start the merging-rebase to cygwin-X.Y.Z` commit on `main`. + +### Key Branches + +- `main`: Git for Windows' branch (fast-forwarding, contains merging-rebase commits) +- `cygwin-X_Y-branch` (e.g., `cygwin-3_6-branch`): Tracking branches for upstream Cygwin +- `cygwin/main`: Upstream Cygwin's main branch +- Various feature branches for specific fixes (e.g., `fix-ctrl+c-again`, `fix-ssh-hangs-reloaded`) + +### Key Remotes + +- `cygwin`: The upstream Cygwin repository (`git://sourceware.org/git/newlib-cygwin.git`) +- `msys2`: The MSYS2 fork (`https://github.com/msys2/msys2-runtime`) +- `git-for-windows`: This repository (`https://github.com/git-for-windows/msys2-runtime`) +- `dscho`: Johannes Schindelin's fork (primary maintainer) + +## Development Guidelines + +### Language and Style + +The runtime is written in **C++** (with some C). The code uses Cygwin's existing coding conventions. When modifying files under `winsup/cygwin/`: +- Follow the existing indentation and brace style of each file +- Cygwin code uses 8-space tabs in many files +- MSYS2-specific additions (like `msys2_path_conv.cc`) may use different conventions + +### Making Changes + +Most changes for Git for Windows purposes are in `winsup/cygwin/`. Common areas of modification: +- Signal handling (`exceptions.cc`, `sigproc.cc`) +- Process spawning (`spawn.cc`) +- PTY/console handling (`fhandler/` directory, `termios.cc`) +- Path conversion (`msys2_path_conv.cc`, `path.cc`) +- Environment handling (`environ.cc`) + +### Testing + +- The CI builds the runtime and runs Git's entire test suite against it +- UI tests in `ui-tests/` test real terminal scenarios using AutoHotKey +- MSYS2's own test suite is run across multiple compiler/environment combinations +- For local testing, build the DLL and copy it to replace `msys-2.0.dll` in an MSYS2 installation + +### Commit Discipline + +- One logical change per commit +- Commit messages should explain context, intent, and justification in prose (not bullet points) +- For the rebase workflow, commit messages follow specific patterns (e.g., `Start the merging-rebase to ...`) that tooling depends on — do not alter these patterns + +### Cygwin Commit Message Format + +Commits that modify code under `winsup/cygwin/` should follow the Cygwin project's commit message conventions, as established by the upstream maintainers (Corinna Vinschen, Takashi Yano, et al.): + +- **Subject prefix**: `Cygwin: : `, where `` is the subsystem (e.g. `pty`, `flock`, `termios`, `uinfo`, `path`, `spawn`). Example: `Cygwin: pty: Fix jumbled keystrokes by removing the per-keystroke pipe transfer`. Both upper-case and lower-case after the prefix are used upstream; there is no strict rule. +- **`Fixes:` trailer**: When a commit fixes a bug introduced by a specific earlier commit, reference it with `Fixes: <12-char-hash> ("")`. Example: `Fixes: acc44e09d1d0 ("Cygwin: pty: Add missing input transfer when switch_to_pcon_in state.")` +- **`Addresses:` trailer**: Reference the user-visible bug report URL. Example: `Addresses: https://github.com/git-for-windows/git/issues/5632` +- **Trailer ordering**: `Addresses:`, then `Fixes:`, then `Assisted-by:` / `Reviewed-by:` / `Reported-by:`, then `Signed-off-by:` last — following the pattern seen in upstream Cygwin commits. + +## PTY Architecture — Pipes, State Machine, and Input Routing + +This section documents the internal architecture of the pseudo-terminal (PTY) implementation in `winsup/cygwin/fhandler/pty.cc`. Understanding this is essential for debugging any issue involving terminal input/output, keystroke handling, signal delivery, and process foreground/background transitions. + +### Background: Why This Matters + +The pseudo console support in the Cygwin runtime is one of the most intricate subsystems in this codebase. It bridges two fundamentally different models of terminal I/O — POSIX and Win32 console — across multiple processes that share state through shared memory. The implementation is ambitious and evolving; the complexity of the interactions between pipe switching, pseudo console lifecycle, cross-process mutexes, and foreground process detection means that changes in one area can have subtle, hard-to-diagnose effects elsewhere. Historically, bug fixes in this area have occasionally introduced new regressions, which is simply a reflection of how difficult the problem space is. Any AI agent working on PTY-related issues should take the time to understand the full picture before proposing changes, and should be especially careful about mutex acquisition order, state transitions that span process boundaries, and the distinction between the two pipe pairs described below. + +### The Two Pipe Pairs + +Each PTY has **two independent pipe pairs** for input, serving different consumers: + +1. **Cygwin (cyg) pipe**: `to_slave` / `from_master` + - Used when a **Cygwin/MSYS2 process** (e.g., bash) is in the foreground. + - Input goes through `line_edit()` (in `termios.cc`) which handles line discipline (echo, canonical mode, special characters) before being written via `accept_input()`. + - The slave reads from `from_master` (aliased as `get_handle()` on the slave side). + +2. **Native (nat) pipe**: `to_slave_nat` / `from_master_nat` + - Used when a **non-Cygwin (native Windows) process** (e.g., `powershell.exe`, `cmd.exe`, a MinGW program) is in the foreground. + - When the pseudo console (pcon) is active, `CreatePseudoConsole()` wraps this pipe pair. The Windows `conhost.exe` process reads from `from_master_nat` and provides console input semantics to the native app. + - The master writes directly to `to_slave_nat` via `WriteFile()`, bypassing `line_edit()`. + +For **output**, there is a corresponding pair (`to_master` / `to_master_nat`) plus a forwarding thread (`master_fwd_thread`) that copies output from the nat pipe's slave side (`from_slave_nat`) to the cyg pipe's master side (`to_master`), so the terminal emulator (mintty) always reads from one place. + +### The Pseudo Console (pcon) + +When `MSYS=disable_pcon` is NOT set (the default), the runtime uses Windows' `CreatePseudoConsole()` API to give native console applications a real console to talk to. The pseudo console is created on demand when a non-Cygwin process becomes the foreground process, and torn down when it exits. This is what allows programs like `cmd.exe`, `powershell.exe`, or any MinGW-built program to work correctly inside a mintty terminal, which has no native Win32 console of its own. + +The pcon lifecycle is managed across process boundaries: the slave process (running the non-Cygwin app) and the master process (the terminal emulator) both participate. This cross-process coordination is the source of much of the complexity. + +Key state fields in the `tty` structure (shared memory, in `tty.h`): + +- **`pcon_activated`** (`bool`): True when a pseudo console is currently active. +- **`pcon_start`** (`bool`): True during pseudo console initialization. +- **`pcon_start_pid`** (`pid_t`): PID of the process that initiated pcon setup. + +### The Input State Machine + +The field **`pty_input_state`** (type `xfer_dir`, in `tty.h:137`) tracks which pipe pair currently "owns" the input. It has two values: + +- **`to_cyg`**: Input is flowing to the Cygwin pipe. The master's `write()` uses the `line_edit()` → `accept_input()` path, which writes to `to_slave` (cyg pipe). +- **`to_nat`**: Input is flowing to the native pipe. The master's `write()` writes directly to `to_slave_nat` (nat pipe), or through the pseudo console. + +The state transitions happen via **`transfer_input()`** (pty.cc, around line 3905), which: +1. Reads all pending data from the "source" pipe (the one being abandoned). +2. Writes that data into the "destination" pipe (the one being switched to). +3. Sets `pty_input_state` to the new direction. + +This ensures data already buffered in one pipe is not lost when switching. **However, `transfer_input()` is only correct at process-group boundaries** — specifically in `setpgid_aux()` (when the foreground changes) and `cleanup_for_non_cygwin_app()` (when a native session ends). Calling `transfer_input()` on every keystroke in `master::write()` was historically a source of bugs: during pseudo console oscillation (see below), per-keystroke transfers would steal readline's buffered data from the cyg pipe and push it to the nat pipe, causing character reordering. The correct approach is to let `setpgid_aux()` handle the transfer at the moment of the actual process-group change, not to anticipate it in the master. + +### Related State Fields + +- **`switch_to_nat_pipe`** (`bool`): Set to true when a non-Cygwin process is detected in the foreground. This is a prerequisite for `to_be_read_from_nat_pipe()` returning true. +- **`nat_pipe_owner_pid`** (`DWORD`): PID of the process that "owns" the nat pipe setup. Used to detect when the owner has exited (for cleanup). + +### The `to_be_read_from_nat_pipe()` Function + +This function (pty.cc, around line 1288) determines whether the current foreground process is a native (non-Cygwin) app. It checks: + +1. `switch_to_nat_pipe` must be true. +2. A named event `TTY_SLAVE_READING` must NOT exist (its existence means a Cygwin process is actively reading from the slave, indicating a Cygwin foreground). +3. `nat_fg(pgid)` returns true (the foreground process group contains a native process). + +**This function reads shared state without holding any mutex.** Its return value can therefore change between consecutive calls within the same function, which is an important consideration for callers that make multiple decisions based on the foreground state. + +### Mutexes and Synchronization + +Two cross-process named mutexes protect different aspects of the PTY state. Understanding which mutex protects what — and the fact that they are independent — is essential for diagnosing race conditions. + +- **`input_mutex`**: Protects the input data path. Held by `master::write()` while routing input to a pipe, by `transfer_input()` while moving data between pipes, and by `line_edit()` / `accept_input()`. +- **`pipe_sw_mutex`**: Protects pipe switching state — creation/destruction of the pseudo console, changes to `switch_to_nat_pipe`, `nat_pipe_owner_pid`. This is a DIFFERENT mutex from `input_mutex`. + +Because these are separate mutexes, it is possible for one process to modify the pipe switching state (under `pipe_sw_mutex`) while another process is in the middle of writing input (under `input_mutex`). Any code that modifies `pty_input_state` or `pcon_activated` must carefully consider whether it also needs `input_mutex` to avoid creating a window where the master's write path makes inconsistent decisions. + +Additionally, because these are **cross-process** named mutexes, they are shared via the kernel between the master (terminal emulator) and slave (bash and its children) processes. Operations that look local in the source code actually have system-wide synchronization effects. + +### The `master::write()` Input Routing (pty.cc, around line 2240) + +When the terminal emulator (mintty) sends a keystroke, it calls `master::write()`. After acquiring `input_mutex`, the function decides which code path to take: + +1. **Code path 1 — pcon+nat fast path** (line ~2237): If `to_be_read_from_nat_pipe()` AND `pcon_activated` AND `pty_input_state == to_nat` → flush any stale readahead via `accept_input()`, then write directly to `to_slave_nat` via `WriteFile()`. This is the fast path for native apps with pcon active. The readahead flush is necessary because a prior `master::write()` call may have gone through `line_edit()` during a brief oscillation gap, leaving data in the readahead buffer that would otherwise be emitted out of order. + +2. **Code path 2 — line_edit** (line ~2275): The default/fallthrough path. Calls `line_edit()` which processes the input through terminal line discipline and then calls `accept_input()`, which writes to either the cyg or nat pipe based on the current `pty_input_state`. The `accept_input()` routing includes a `!pcon_activated` guard: it only routes to the nat pipe when pcon is NOT active, matching the documented invariant that direct nat pipe writes are for when "pseudo console is not enabled." + +The conditions checked at each step involve multiple shared-memory fields (`to_be_read_from_nat_pipe()`, `pcon_activated`, `pty_input_state`). If any of these fields changes between consecutive calls to `master::write()` — or worse, between the check and the write within a single call — input can end up in the wrong pipe. + +### Pseudo Console Oscillation + +When a native process spawns short-lived Cygwin children (e.g. `git.exe` calling `cygpath` via `--format`), the pseudo console activates and deactivates in rapid succession: + +1. Native process in foreground: `pcon_activated=true`, `pty_input_state=to_nat` +2. Cygwin child starts: `setpgid_aux()` fires, transfers data to cyg pipe, `pcon_activated=false`, `pty_input_state=to_cyg` +3. Cygwin child exits (milliseconds later): native process regains foreground, pcon reactivates + +A single command can cause dozens of such cycles per second. This "oscillation" is the root cause of the character reordering bug fixed on the `fix-jumbled-character-order` branch (see git-for-windows/git#5632). During each gap (step 2), `master::write()` must correctly route keystrokes without stealing data from readline's buffer in the cyg pipe. + +The key insight: during the oscillation gap, `switch_to_nat_pipe` remains true (the native process is still alive) even though `pcon_activated` is false. This means `to_be_read_from_nat_pipe()` returns true, which historically caused several code paths to prematurely transfer data from the cyg pipe to the nat pipe. Those transfer code paths have been removed — `setpgid_aux()` in the slave is now the sole authority for pipe transfers at process-group boundaries. + +### Key Functions for State Transitions + +- **`setup_for_non_cygwin_app()`** (~line 4150): Called when a non-Cygwin process becomes foreground. Sets up the pseudo console and switches input to nat pipe. +- **`cleanup_for_non_cygwin_app()`** (~line 4184): Called when the non-Cygwin process exits. Tears down pcon, transfers input back to cyg pipe. +- **`reset_switch_to_nat_pipe()`** (~line 1091): Cleanup function called from various slave-side operations (e.g., `bg_check()`, `setpgid_aux()`). Detects when the nat pipe owner has exited and resets state. This function is particularly subtle because it runs in the slave process and modifies shared state that the master relies on. Note: the guard logic checks `process_alive()` first, then handles two sub-cases — when another process owns the nat pipe (return early), and when bash itself is the owner (return early if `pcon_activated` or `switch_to_nat_pipe` is still set, indicating the native session is ongoing). Without this two-level guard, `bg_check()` can tear down active pcon sessions, amplifying oscillation. +- **`mask_switch_to_nat_pipe()`** (~line 1249): Temporarily masks/unmasks the nat pipe switching. Used when a Cygwin process starts/stops reading from the slave. +- **`setpgid_aux()`** (~line 4214): Called when the foreground process group changes. May trigger pipe switching. + +### Debugging Tips + +When investigating PTY-related bugs, keep these patterns in mind: + +- **Data in two pipes**: If characters are lost, duplicated, or reordered, check whether data ended up split across the cyg and nat pipes due to a state transition during input. +- **Cross-process state changes**: The master and slave processes share state through the `tty` structure in shared memory. A state change in the slave (e.g., `reset_switch_to_nat_pipe()`) is immediately visible to the master, without any notification. Look for races where the master reads state, acts on it, but the state changed between the read and the action. +- **Mutex coverage gaps**: Check whether every modification of `pty_input_state`, `pcon_activated`, and `switch_to_nat_pipe` is protected by the appropriate mutex. The existence of two separate mutexes (`input_mutex` and `pipe_sw_mutex`) means that holding one does not protect against changes guarded by the other. +- **`transfer_input()` is correct only at process-group boundaries**: The proper places for `transfer_input()` are `setpgid_aux()` (foreground change) and `cleanup_for_non_cygwin_app()` (session end). Per-keystroke transfers in `master::write()` were historically a source of character reordering — they would steal readline's buffered data from the cyg pipe during pseudo console oscillation gaps. If you see a `transfer_input()` call in `master::write()`, question whether it is genuinely needed or whether `setpgid_aux()` already handles the case. +- **Pseudo console oscillation**: When characters are lost or reordered and the scenario involves a native process spawning Cygwin children, suspect pcon oscillation. The oscillation happens because each Cygwin child start/exit triggers a pcon teardown/setup cycle, and shared-memory flags (`pcon_activated`, `switch_to_nat_pipe`, `pty_input_state`) change rapidly without synchronization with the master's `input_mutex`. Tracing the state transitions across processes is essential for diagnosis. +- **Tracing**: For timing-sensitive bugs, in-process tracing with lock-free per-thread buffers (using Windows TLS and `QueryPerformanceCounter`) is effective. Avoid file I/O during reproduction — accumulate in memory and dump at process exit. See the `ui-tests/` directory for AutoHotKey-based reproducers that can drive mintty programmatically. + +## Packaging + +The MSYS2 runtime is packaged as an **msys** package (`msys2-runtime`) using `makepkg` with a `PKGBUILD` recipe in the `msys2/MSYS2-packages` repository. The package definition lives at `msys2-runtime/PKGBUILD` in that repository. + +## External Resources + +- **Cygwin project**: https://cygwin.com — upstream source, FAQ, user's guide +- **Cygwin source**: https://github.com/cygwin/cygwin (mirror of `sourceware.org/git/newlib-cygwin.git`) +- **Cygwin announcements**: https://inbox.sourceware.org/cygwin-announce — release announcements +- **Cygwin mailing lists**: https://inbox.sourceware.org/cygwin/ (general), https://inbox.sourceware.org/cygwin-patches/ (patches), https://inbox.sourceware.org/cygwin-developers/ (internals) — essential for understanding why specific code was added; commit messages often reference these discussions +- **MSYS2 project**: https://www.msys2.org — documentation, package management +- **MSYS2 runtime source**: https://github.com/msys2/msys2-runtime +- **MSYS2 packages**: https://github.com/msys2/MSYS2-packages — package recipes including `msys2-runtime` +- **Git for Windows**: https://gitforwindows.org +- **Git for Windows runtime**: https://github.com/git-for-windows/msys2-runtime (this repository) +- **MSYS2 environments**: https://www.msys2.org/docs/environments/ — explains MSYS vs UCRT64 vs CLANG64 etc. diff --git a/ui-tests/.gitattributes b/ui-tests/.gitattributes index 4dd1b9375b..7d5ccef0ca 100644 --- a/ui-tests/.gitattributes +++ b/ui-tests/.gitattributes @@ -1 +1,2 @@ *.ahk eol=lf +*.ps1 eol=lf diff --git a/ui-tests/background-hook.ahk b/ui-tests/background-hook.ahk index af7c27d313..76d04708d2 100755 --- a/ui-tests/background-hook.ahk +++ b/ui-tests/background-hook.ahk @@ -43,7 +43,7 @@ WaitForRegExInWindowsTerminal('`n49$', 'Timed out waiting for commit to finish', ; Verify that CursorUp shows the previous command Send('{Up}') Sleep 150 -Text := CaptureTextFromWindowsTerminal() +Text := CaptureBufferFromWindowsTerminal() if not RegExMatch(Text, 'git commit --allow-empty -m zOMG *$') ExitWithError 'Cursor Up did not work: ' Text Info('Match!') diff --git a/ui-tests/cpu-stress.ps1 b/ui-tests/cpu-stress.ps1 new file mode 100644 index 0000000000..4f1dbff8c0 --- /dev/null +++ b/ui-tests/cpu-stress.ps1 @@ -0,0 +1,6 @@ +$sleepExe = & cygpath.exe -aw /usr/bin/sleep.exe +$procs = 1..[Environment]::ProcessorCount | ForEach-Object { + Start-Process -NoNewWindow -PassThru cmd.exe -ArgumentList '/c','for /L %i in (1,1,999999) do @echo . >NUL' +} +& $sleepExe 1 +$procs | Stop-Process -Force -ErrorAction SilentlyContinue diff --git a/ui-tests/ctrl-c.ahk b/ui-tests/ctrl-c.ahk index 3ca8873de7..432c0b58c0 100644 --- a/ui-tests/ctrl-c.ahk +++ b/ui-tests/ctrl-c.ahk @@ -151,11 +151,27 @@ if (openSSHPath != '' and FileExist(openSSHPath . '\sshd.exe')) { Info('Started SSH server: ' sshdPID) Info('Starting clone') - Send('git -c core.sshCommand="ssh ' . sshOptions . '" clone ' . cloneOptions . '{Enter}') - Sleep 500 - Info('Waiting for clone to finish') - WinActivate('ahk_id ' . hwnd) - WaitForRegExInWindowsTerminal('Receiving objects: .*, done\.`r?`nPS .*>[ `n`r]*$', 'Timed out waiting for clone to finish', 'Clone finished', 15000, 'ahk_id ' . hwnd) + retries := 5 + Loop retries { + Send('git -c core.sshCommand="ssh ' . sshOptions . '" clone ' . cloneOptions . '{Enter}') + Sleep 500 + Info('Waiting for clone to finish (attempt ' . A_Index . '/' . retries . ')') + WinActivate('ahk_id ' . hwnd) + matchObj := WaitForRegExInWindowsTerminal('(Receiving objects: .*, done\.|fatal: early EOF)`r?`nPS .*>[ `n`r]*$', 'Timed out waiting for clone to finish', 'Clone command completed', 15000, 'ahk_id ' . hwnd) + + if InStr(matchObj[1], 'done.') + break + if A_Index == retries + ExitWithError('Clone failed after ' . retries . ' attempts (early EOF)') + Info('Clone failed (early EOF), restarting SSH server and retrying...') + if DirExist(largeGitClonePath) + DirDelete(largeGitClonePath, true) + ; Restart sshd for the next attempt (it may have exited after the failed connection) + Run(openSSHPath . '\sshd.exe ' . sshdOptions, '', 'Hide', &sshdPID) + if A_LastError + ExitWithError 'Error restarting SSH server: ' A_LastError + Info('Restarted SSH server: ' sshdPID) + } if not DirExist(largeGitClonePath) ExitWithError('`large-clone` did not work?!?') diff --git a/ui-tests/keystroke-order.ahk b/ui-tests/keystroke-order.ahk new file mode 100644 index 0000000000..e6161a782c --- /dev/null +++ b/ui-tests/keystroke-order.ahk @@ -0,0 +1,165 @@ +#Requires AutoHotkey v2.0 +#Include ui-test-library.ahk + +; Reproducer for https://github.com/git-for-windows/git/issues/5632 +; +; Keystroke reordering: when a non-MSYS2 process runs in the foreground +; of a PTY, keystrokes typed into bash arrive out of order because the +; MSYS2 runtime's transfer_input() can reorder bytes across pipe buffers. +; +; The test types characters interleaved with backspaces while a non-MSYS +; foreground process (powershell launching MSYS sleep) runs under CPU +; stress. If backspace bytes get reordered relative to the characters +; they should delete, readline produces wrong output. +; +; The test runs in two phases: +; Phase 1 (pcon enabled): the default mode, exercises the pseudo +; console oscillation code paths in master::write(). +; Phase 2 (disable_pcon): sets MSYS=disable_pcon so that pseudo +; console is never created, exercising the non-pcon input routing +; and verifying that typeahead is preserved correctly. + +SetWorkTree('git-test-keystroke-order') + +testString := 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789' + +hwnd := LaunchMintty() +winId := 'ahk_id ' hwnd + +; Wait for bash prompt via HTML export (Ctrl+F5). +deadline := A_TickCount + 60000 +while A_TickCount < deadline +{ + capture := CaptureBufferFromMintty(winId) + if InStr(capture, '$ ') + break + Sleep 500 +} +if !InStr(capture, '$ ') + ExitWithError 'Timed out waiting for bash prompt' +Info 'Bash prompt appeared' + +stressCmd := 'powershell.exe -File ' StrReplace(A_ScriptDir, '\', '/') '/cpu-stress.ps1' +Info 'Foreground command: ' stressCmd + +; === Phase 1: pcon enabled (default) === +Info '=== Phase 1: pcon enabled ===' +mismatch := RunKeystrokeTest(winId, stressCmd, testString, 20) + +if !mismatch +{ + ; === Phase 2: disable_pcon === + Info '=== Phase 2: disable_pcon ===' + WinActivate(winId) + SetKeyDelay 20, 20 + SendEvent('{Text}export MSYS=disable_pcon') + SendEvent('{Enter}') + Sleep 500 + + mismatch := RunKeystrokeTest(winId, stressCmd, testString, 5) +} + +WinActivate(winId) +SetKeyDelay 20, 20 +Send '{Ctrl down}c{Ctrl up}' +Sleep 500 +SendEvent('{Text}exit') +SendEvent('{Enter}') +Sleep 1000 +ExitApp mismatch ? 1 : 0 + +; Run the keystroke reordering test for a given number of iterations. +; Returns true if a mismatch was detected, false if all iterations passed. +RunKeystrokeTest(winId, stressCmd, testString, maxIterations) { + mismatch := false + chunkSize := 2 + + Loop maxIterations + { + iteration := A_Index + Info 'Iteration ' iteration ' of ' maxIterations + + WinActivate(winId) + + ; 1. Launch foreground stress process + SetKeyDelay 20, 20 + SendEvent('{Text}' stressCmd) + SendEvent('{Enter}') + + ; 2. Type with backspaces: send chunkSize chars + "XY" + BS*2 at a time. + SetKeyDelay 1, 1 + Sleep 500 + offset := 1 + while offset <= StrLen(testString) + { + chunk := SubStr(testString, offset, chunkSize) + SendEvent('{Text}' chunk 'XY') + SendEvent('{Backspace}{Backspace}') + offset += chunkSize + } + + ; 3. Poll the HTML export for what readline rendered after "$ ". + ; The HTML shows the final screen state (backspaces already applied). + Sleep 2000 + deadline := A_TickCount + 30000 + while A_TickCount < deadline + { + text := CaptureBufferFromMintty(winId) + + ; Find the last "$ " and extract the text after it + lastPrompt := 0 + pos := 1 + while pos := InStr(text, '$ ', , pos) + { + lastPrompt := pos + pos += 2 + } + if lastPrompt > 0 + { + after := Trim(SubStr(text, lastPrompt + 2)) + ; Take first "word" (up to whitespace or end) + spPos := InStr(after, ' ') + if spPos > 0 + after := SubStr(after, 1, spPos - 1) + + if after = testString + { + Info 'Iteration ' iteration ': OK' + break + } + if InStr(after, 'powershell') || InStr(after, 'sleep') || after = '' + { + ; Stress command or bare prompt -- keep waiting + } + else if SubStr(testString, 1, StrLen(after)) != after + { + Info 'MISMATCH in iteration ' iteration '!' + Info 'Expected: ' testString + Info 'Got: ' after + mismatch := true + break + } + } + Sleep 500 + } + + if A_TickCount >= deadline + { + Info 'TIMEOUT in iteration ' iteration + mismatch := true + break + } + if mismatch + break + + ; Clear readline buffer for next iteration + SetKeyDelay 20, 20 + Send '{Ctrl down}u{Ctrl up}' + Sleep 300 + } + + if !mismatch + Info 'All ' maxIterations ' iterations passed' + + return mismatch +} diff --git a/ui-tests/setup-portable-wt.ps1 b/ui-tests/setup-portable-wt.ps1 new file mode 100644 index 0000000000..572e6bc119 --- /dev/null +++ b/ui-tests/setup-portable-wt.ps1 @@ -0,0 +1,96 @@ +# Configures a portable Windows Terminal for the UI tests. +# +# Downloads WT if needed, then creates .portable marker and settings.json +# with exportBuffer bound to Ctrl+Shift+F12. The export file lands in the +# script's own directory (ui-tests/) so it gets uploaded as build artifact. +# +# The portable WT uses its own settings directory (next to the executable) +# so it never touches the user's installed Windows Terminal configuration. + +param( + [string]$WtVersion = $env:WT_VERSION, + [string]$DestDir = $env:TEMP +) + +if (-not $WtVersion) { $WtVersion = '1.22.11141.0' } + +$wtDir = "$DestDir\terminal-$WtVersion" +$wtExe = "$wtDir\wt.exe" + +# Download if the directory doesn't contain wt.exe yet +if (-not (Test-Path $wtExe)) { + $wtZip = "$DestDir\wt.zip" + if (-not (Test-Path $wtZip)) { + $url = "https://github.com/microsoft/terminal/releases/download/v$WtVersion/Microsoft.WindowsTerminal_${WtVersion}_x64.zip" + Write-Host "Downloading Windows Terminal $WtVersion ..." + curl.exe -fLo $wtZip $url + if ($LASTEXITCODE -ne 0) { throw "Download failed" } + } + Write-Host "Extracting ..." + & "$env:WINDIR\system32\tar.exe" -C $DestDir -xf $wtZip + if ($LASTEXITCODE -ne 0) { throw "Extract failed" } +} + +# Create .portable marker so WT reads settings from settings\ next to wt.exe +$portableMarker = "$wtDir\.portable" +if (-not (Test-Path $portableMarker)) { + Set-Content -Path $portableMarker -Value "" +} + +# Write settings.json with exportBuffer action +$settingsDir = "$wtDir\settings" +if (-not (Test-Path $settingsDir)) { New-Item -ItemType Directory -Path $settingsDir -Force | Out-Null } + +$bufferExportPath = ($PSScriptRoot + '\wt-buffer-export.txt') -replace '\\', '/' + +$settings = @" +{ + "`$schema": "https://aka.ms/terminal-profiles-schema", + "actions": [ + { + "command": { + "action": "exportBuffer", + "path": "$bufferExportPath" + }, + "id": "User.TestExportBuffer" + }, + { + "command": { "action": "copy", "singleLine": false }, + "id": "User.copy" + }, + { "command": "paste", "id": "User.paste" } + ], + "copyFormatting": "none", + "copyOnSelect": false, + "defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", + "keybindings": [ + { "id": "User.TestExportBuffer", "keys": "ctrl+shift+f12" }, + { "id": null, "keys": "ctrl+v" }, + { "id": null, "keys": "ctrl+c" } + ], + "profiles": { + "defaults": {}, + "list": [ + { + "commandline": "%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", + "guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", + "hidden": false, + "name": "Windows PowerShell" + } + ] + }, + "schemes": [], + "themes": [] +} +"@ + +Set-Content -Path "$settingsDir\settings.json" -Value $settings + +# Add WT to PATH if running in GitHub Actions +if ($env:GITHUB_PATH) { + $wtDir | Out-File -Append -FilePath $env:GITHUB_PATH +} + +Write-Host "Portable WT ready at: $wtDir" +Write-Host " exportBuffer path: $bufferExportPath" +Write-Host " exportBuffer key: Ctrl+Shift+F12" diff --git a/ui-tests/ui-test-library.ahk b/ui-tests/ui-test-library.ahk index 120e85ed76..a559be56db 100644 --- a/ui-tests/ui-test-library.ahk +++ b/ui-tests/ui-test-library.ahk @@ -67,41 +67,24 @@ RunWaitOne(command) { return Result } -; This function is quite the hack. It assumes that the Windows Terminal is the active window, -; then drags the mouse diagonally across the window to select all text and then copies it. -; -; This is fragile! If any other window becomes active, or if the mouse is moved, -; the function will not work as intended. -; -; An alternative would be to use `ControlSend`, e.g. -; `ControlSend '+^a', 'Windows.UI.Input.InputSite.WindowClass1', 'ahk_id ' . hwnd -; This _kinda_ works, the text is selected (all text, in fact), but the PowerShell itself -; _also_ processes the keyboard events and therefore they leave ugly and unintended -; `^Ac` characters in the prompt. So that alternative is not really usable. -CaptureTextFromWindowsTerminal(winTitle := '') { +; Capture the Windows Terminal buffer via the exportBuffer action (Ctrl+Shift+F12). +; Requires a portable WT with settings.json that maps Ctrl+Shift+F12 to exportBuffer +; writing to /wt-buffer-export.txt. +CaptureBufferFromWindowsTerminal(winTitle := '') { + static exportFile := A_ScriptDir . '\wt-buffer-export.txt' + if FileExist(exportFile) + FileDelete exportFile if winTitle != '' WinActivate winTitle - ControlGetPos &cx, &cy, &cw, &ch, 'Windows.UI.Composition.DesktopWindowContentBridge1', "A" - titleBarHeight := 54 - scrollBarWidth := 28 - pad := 8 - - SavedClipboard := ClipboardAll - A_Clipboard := '' - SendMode('Event') - if winTitle != '' - WinActivate winTitle - MouseMove cx + pad, cy + titleBarHeight + pad - if winTitle != '' - WinActivate winTitle - MouseClickDrag 'Left', , , cx + cw - scrollBarWidth, cy + ch - pad, , '' - if winTitle != '' - WinActivate winTitle - MouseClick 'Right' - ClipWait() - Result := A_Clipboard - Clipboard := SavedClipboard - return Result + Sleep 200 + Send '^+{F12}' + deadline := A_TickCount + 3000 + while !FileExist(exportFile) && A_TickCount < deadline + Sleep 50 + if !FileExist(exportFile) + return '' + Sleep 100 + return FileRead(exportFile) } WaitForRegExInWindowsTerminal(regex, errorMessage, successMessage, timeout := 5000, winTitle := '') { @@ -109,17 +92,82 @@ WaitForRegExInWindowsTerminal(regex, errorMessage, successMessage, timeout := 50 ; Wait for the regex to match in the terminal output while true { - capturedText := CaptureTextFromWindowsTerminal(winTitle) - if RegExMatch(capturedText, regex) - break + capturedText := CaptureBufferFromWindowsTerminal(winTitle) + if RegExMatch(capturedText, regex, &matchObj) + { + Info(successMessage) + return matchObj + } Sleep 100 if A_TickCount > timeout { Info('Captured text:`n' . capturedText) ExitWithError errorMessage } - if winTitle != '' - WinActivate winTitle - MouseClick 'WheelDown', , , 20 } - Info(successMessage) +} + +; Launch mintty with HTML export support. Returns the window handle. +; Ctrl+F5 is bound to export-html; the file is written to /mintty-export.html. +LaunchMintty(extraArgs := '') { + exportFile := A_ScriptDir . '\mintty-export.html' + savePattern := StrReplace(A_ScriptDir, '\', '/') '/mintty-export' + minttyClass := 'ahk_class mintty' + existing := Map() + for h in WinGetList(minttyClass) + existing[h] := true + + cmd := 'mintty.exe -o "KeyFunctions=C+F5:export-html" -o "SaveFilename=' savePattern '"' + if extraArgs != '' + cmd .= ' ' extraArgs + cmd .= ' -' + Run cmd, , , &childPid + Info 'Launched mintty, PID: ' childPid + + hwnd := 0 + deadline := A_TickCount + 10000 + while A_TickCount < deadline + { + for h in WinGetList(minttyClass) + { + if !existing.Has(h) + { + hwnd := h + break 2 + } + } + Sleep 100 + } + if !hwnd + ExitWithError 'New mintty window did not appear' + WinActivate('ahk_id ' hwnd) + Info 'Found new mintty: ' hwnd + return hwnd +} + +; Trigger Ctrl+F5 to export mintty's screen as HTML, read it, strip tags, +; and return the plain text. +CaptureBufferFromMintty(winTitle := '') { + static exportFile := A_ScriptDir . '\mintty-export.html' + if FileExist(exportFile) + FileDelete exportFile + if winTitle != '' + WinActivate winTitle + Send '^{F5}' + deadline := A_TickCount + 3000 + while !FileExist(exportFile) && A_TickCount < deadline + Sleep 50 + if !FileExist(exportFile) + return '' + Sleep 100 + html := FileRead(exportFile) + ; Extract body content only (skip CSS in