Tty conversion - #126
Draft
graeme wants to merge 20 commits into
Draft
Conversation
Foundation.Process cannot call setsid() between fork and exec, which a controlling-terminal pty requires. Subprocess exposes it as PlatformOptions.createSession, alongside the file-descriptor hand-off and process-group teardown the pty path needs. Pinned exactly at 1.0.0 per CONVENTIONS.md; pulls in swift-system 1.8.0 transitively. Scoped to the BrewCLI target only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wraps openpty with the two things that are easy to get wrong: the local replica descriptor must be closed after spawn or reads never see EOF, and end-of-input arrives as either 0 or EIO depending on ordering. Clears OPOST on the replica so the kernel does not rewrite every \n as \r\n. Carriage returns the child writes itself are untouched, which is what keeps progress-bar redraws intact while leaving captured bytes verbatim. Sets a non-zero window size; openpty defaults to 0x0, which some tools read as "not a terminal" and use to suppress progress output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selects terminal-backed execution per run rather than globally, so runs whose output is parsed can stay on pipes where stdout and stderr remain distinct. Defaults to false; no caller opts in yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both execution paths now go through Subprocess rather than Foundation.Process. The pipe path is behaviour-preserving; the new terminal path hands the child a pty replica as stdout and stderr, so isatty holds and Homebrew emits colour and progress rendering without HOMEBREW_COLOR forcing it. Runs create their own session. That is what makes the pty a controlling terminal rather than just a terminal-shaped descriptor, and it puts the child and its descendants in one process group so teardown signals all of them — cancelling an install now stops the curl or git it was waiting on instead of orphaning it. Verified by test: two processes spawned, none left after cancel. The terminal drain stops on a quiet terminal once the child has exited, rather than waiting for end-of-input. End-of-input is driven by descriptors, so a grandchild holding the replica would otherwise stall the drain indefinitely. TERM is set explicitly because a GUI process launched from Finder inherits none, and tools suppress colour and progress without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Display work is only ever shown, so it takes the terminal path and gets Homebrew's colour and progress rendering natively. Capture work is parsed by its caller and stays on pipes, where stdout and stderr remain distinct — it keeps forcing colour, since a pipe strips it and brew doctor is both shown and parsed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Standard input stays on /dev/null even when stdout and stderr are a terminal. An interactive rc file that reads stdin would otherwise block forever, since nothing ever writes to the pty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Picks up swift-subprocess 1.0.0 and its swift-system dependency, matching the package-level Package.resolved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scanner only told callers whether a sequence was SGR. Reporting the final byte alongside the parameters lets a second caller act on erase-in-line and friends without duplicating the scanning. Pure refactor: parse still acts only on 'm'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies terminal overwrite semantics to a stream of output. A terminal is not an append-only transcript: a carriage return moves the cursor back to column zero and what follows overwrites what was there. curl redraws its progress bar hundreds of times that way, with no newline between redraws. Splitting on newlines alone turns a whole download into one enormous line holding every intermediate state. This keeps the line as cells with a cursor, so a redraw replaces earlier content instead of accumulating after it, and reports both committed lines and in-progress revisions. Styling is tracked per cell rather than left inline, because counting escape bytes as visible columns would land every later overwrite in the wrong place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A terminal-backed line is not final when it first appears: a progress bar revises the same line repeatedly before a newline settles it. Lines now carry whether they are complete, plus their resolved styled spans, and a job replaces its trailing incomplete row rather than appending. The replacement keeps the row's identity so anything keyed on it sees one row changing rather than a new row each redraw. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Terminal output no longer splits on newlines, because a terminal's redraws are separated by carriage returns — by the newline measure a whole download is one line. The assembler resolves the overwrites and reports the line while it is still being drawn, so a progress bar animates in place instead of leaving hundreds of rows behind. Reads land on arbitrary byte boundaries, so a multi-byte character split across two reads is held until the rest arrives rather than dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lines arrive with their styling already resolved, so the console renders those spans rather than re-parsing the raw text on every pass. For terminal-backed runs the spans also reflect the overwrites the assembler applied, which raw text alone cannot express. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spans the boundary the bug actually crossed: a real subprocess writing 600 carriage-return redraws, through the drain and assembler, into the console's job buffer. Asserts the four rows a terminal would show rather than the several hundred the console used to accumulate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Building the message allocates, and an allocation makes syscalls of its own that overwrite errno before strerror reads it. The failure reported as "Unknown error: -6"; captured immediately it is ENXIO, which is what actually points at pty exhaustion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pty pool is far smaller than kern.tty.ptmx_max advertises — measured at ~36 concurrent on a development machine. Swift Testing runs suites in parallel, so these tests were competing for it and failing with ENXIO in 3 of 8 runs. Allocation itself is sound: 700 sequential allocate-and-release cycles pass without a failure, so nothing leaks. Nothing in the app contends this way either, since the command center runs one subprocess at a time. Ten consecutive full-suite runs now pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Allocation is an environmental resource that genuinely runs out — the device pool is much smaller than kern.tty.ptmx_max advertises. Failing an install over it is a poor trade when the command runs perfectly well on pipes; what degrades is Homebrew's progress rendering, not the operation. The fallback forces colour back on, since the terminal was what supplied it, and keeps the caller's line observer so the console still streams. Only allocation failure degrades: a command that cannot run still fails. Reached through an injected factory, since the failure is otherwise only reachable by exhausting the machine's pty devices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drops restatements of what the code already says, keeping the notes that document something non-obvious: descriptor ownership, errno capture, OPOST, the drain's timeout, setsid, and the pty pool's size. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ESC[G, ESC[C and ESC[D address a column within the current line, so they belong to the line model rather than to the screen-buffer follow-up. Some progress renderers use ESC[G where curl uses \r; ignoring it let a redraw append instead of overwrite, which is the exact failure the assembler exists to prevent. Columns are clamped: a malformed ESC[999999999C would otherwise have the next write pad every cell up to it. Erase-to-start now blanks with the current style. A terminal erases with the active attribute, so carrying the old one over left a stale colour behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A terminal merges stdout and stderr onto one device, so a pty run returns an empty standardError. SerialBrewCommandCenter read exactly that field to build BrewCommandError.failed for display work, and display work is the only path that runs on a pty. Every failed install, upgrade and uninstall therefore reached the user as OperationFailure's generic fallback string instead of brew's own message. The merged transcript is the only account of the failure on that path, so CommandFailureDetail prefers stderr and falls back to the transcript's tail. Trimmed, because it feeds an error banner rather than a log pane. To make that fallback usable, the pty path now returns the assembled transcript rather than the verbatim bytes. Raw pty bytes are a cursor script in which a whole download is one line carrying every intermediate redraw; resolving the overwrites is what makes the text readable. Nothing parses display output, and the drain already had the assembled lines, so this also gives a purpose to bytes that were previously accumulated for the life of the run and never read. Decoding is no longer all-or-nothing. takeDecodableUTF8Prefix held back only trailing bytes, so an invalid byte anywhere else made every candidate fail and cost a *leading* byte per read instead — at one byte discarded per 4096 arriving, output that produced invalid bytes faster than that grew the buffer without bound and silenced the console for the rest of the run. UTF8StreamDecoder holds back only a genuinely incomplete trailing sequence and substitutes U+FFFD for the rest. The same reasoning replaces the failable String initialisers, where one bad byte discarded a whole run's output, or a whole line of it. BrewRunOptions carried the channel as two booleans that could contradict each other, with forceColor documented as redundant against the other. They are one three-state choice, so they are now one enum, and the merged-streams contract lives in the type rather than a doc comment. PseudoTerminal.read no longer reports a bad descriptor as a clean finish; a truncated run says so instead of passing for a complete one. Its descriptors are let-bound, since they never change after init, and the ordering rule the unlocked read actually depends on is documented on closePrimary rather than implied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Run brew under a pseudo-terminal
Summary
Homebrew commands shown in the console now run against a pseudo-terminal instead of pipes. Brew and the tools it shells out to see a real terminal, so they emit their own colour and progress rendering, and output arrives as it is produced rather than in block-buffered bursts. Commands whose output gets parsed stay on pipes, so no parser changes behaviour.
Changes
Why this split
Screen-level emulation, which the multi-line spinner artifacts need, is a different data model: a grid with a cursor rather than a transcript of rows. It would touch scrollback, selection, copy and export, so it belongs in its own PR.
Testing
PR checklist
Follow-ups