mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-04-06 07:38:21 +00:00
build: fix Windows build failures in helpgen and framegen (#11803)
> [!WARNING] > Review/approve this AFTER #11798, #11800 and #11801 (this PR stacks on top of rhem... ergo, it includes their commits) > Don't cheat! Start from the oldest one! 😄 I know these are almost one-liners but I am doing this mostly for documentation and karma points. BTW, Github needs to level up this wankflow like a lot... IMHO ## Summary - Use `writerStreaming()` instead of `writer()` for stdout in helpgen and main_build_data (`ftruncate` on pipes fails on Windows with `INVALID_PARAMETER` mapped to `FileTooBig`) - Replace POSIX `scandir` with `opendir`/`readdir` plus `qsort` in framegen since `scandir` is not available on Windows ## Context This fix was previously applied upstream by Mitchell (f4998c6ab) and reverted 15 minutes later (0fdddd5bc). The reason for the revert is not clear. Around the same time, a CI step was added to execute cmake examples on Windows, which was later removed (b723f2a43) with the note "hangs, so remove it entirely". Whether the revert is related to the hang or had a separate reason, we don't know. What we do know: - Both `helpgen` and `framegen` run during normal builds on Windows (via `SharedDeps`), not just during dist packaging. Claude had told me the opposite before but "don't trust and verify". - Without this fix, both tools fail: helpgen with `FileTooBig` (ftruncate on pipes), framegen with `scandir` undeclared - The fix does not regress Linux or macOS ## Stack Stacked on 012-windows/fix-glslang-msvc. ## Test plan ### Cross-platform results (`zig build test` / `zig build -Dapp-runtime=none test` on Windows) | | Windows | Linux | Mac | |---|---|---|---| | **BEFORE** (74c6ffe78) | FAIL - 39/51 steps, 4 failed | PASS - 86/86, 2655/2678 tests, 23 skipped | PASS - 160/160, 2655/2662 tests, 7 skipped | | **AFTER** (f9d3b1aaf) | FAIL - 44/51 steps, 2 failed | PASS - 86/86, 2655/2678 tests, 23 skipped | PASS - 160/160, 2655/2662 tests, 7 skipped | ### Windows: what changed (39 > 44 steps, 4 > 2 failures) **Fixed by this PR:** - `run exe helpgen` -> was `failure` (FileTooBig from ftruncate on stdout pipe) -> `success` - `compile exe framegen` -> was `1 errors` (scandir undeclared) ->. `success` **Remaining failures (pre-existing, fixed by later PRs in stack):** - `translate-c` -> 3 errors (`ssize_t` unknown in ghostty.h on MSVC) - `compile lib freetype` -> 2 errors (`unistd.h` not found) ### Linux/macOS: no regressions Identical pass counts and test results before and after. ## Discussion points ### "Grep wider" other `stdout().writer()` callsites There are 15+ other `stdout().writer(&buf)` callsites in the codebase. Build-time generators that capture stdout (webgen, mdgen, unicode generators) would have the same `ftruncate` issue if they ran on Windows. Currently they don't appear in the Windows build graph, but worth noting for future Windows work. ### `writerStreaming()` vs `writer()` `writer()` calls `ftruncate` on flush/end to set the file size, which fails on pipes (stdout captured by the build system). `writerStreaming()` skips the truncate since the output goes to a pipe, not a seekable file. This is the correct API for this use case on all platforms, not just Windows. ## What I Learnt - When upstream has applied and reverted something, state what you observe rather than speculating about their reasoning. Let the reviewer fill in context you don't have. - "Grep wider" (testing pattern): `stdout().writer()` appears in 17 files. Only 2 are fixed here because only 2 are in the current Windows build path. But the pattern exists more broadly. - I feel like I am training my replacements. I mean, I am a parent, it rhymes. - I feel like my replacements are training me. It rhymes as well.
This commit is contained in:
@@ -8,15 +8,16 @@
|
||||
|
||||
#define SEPARATOR '\x01'
|
||||
#define CHUNK_SIZE 16384
|
||||
#define MAX_FRAMES 1024
|
||||
#define PATH_SEP '/'
|
||||
|
||||
static int filter_frames(const struct dirent *entry) {
|
||||
const char *name = entry->d_name;
|
||||
static int is_frame_file(const char *name) {
|
||||
size_t len = strlen(name);
|
||||
return len > 4 && strcmp(name + len - 4, ".txt") == 0;
|
||||
}
|
||||
|
||||
static int compare_frames(const struct dirent **a, const struct dirent **b) {
|
||||
return strcmp((*a)->d_name, (*b)->d_name);
|
||||
static int compare_names(const void *a, const void *b) {
|
||||
return strcmp(*(const char **)a, *(const char **)b);
|
||||
}
|
||||
|
||||
static char *read_file(const char *path, size_t *out_size) {
|
||||
@@ -54,25 +55,47 @@ int main(int argc, char **argv) {
|
||||
const char *frames_dir = argv[1];
|
||||
const char *output_file = argv[2];
|
||||
|
||||
struct dirent **namelist;
|
||||
int n = scandir(frames_dir, &namelist, filter_frames, compare_frames);
|
||||
if (n < 0) {
|
||||
// Use opendir/readdir instead of scandir for Windows compatibility
|
||||
DIR *dir = opendir(frames_dir);
|
||||
if (!dir) {
|
||||
fprintf(stderr, "Failed to scan directory %s: %s\n", frames_dir, strerror(errno));
|
||||
return 1;
|
||||
}
|
||||
|
||||
char *names[MAX_FRAMES];
|
||||
int n = 0;
|
||||
struct dirent *entry;
|
||||
while ((entry = readdir(dir)) != NULL) {
|
||||
if (!is_frame_file(entry->d_name)) continue;
|
||||
if (n >= MAX_FRAMES) {
|
||||
fprintf(stderr, "Too many frame files (max %d)\n", MAX_FRAMES);
|
||||
closedir(dir);
|
||||
return 1;
|
||||
}
|
||||
names[n] = strdup(entry->d_name);
|
||||
if (!names[n]) {
|
||||
fprintf(stderr, "Failed to allocate memory\n");
|
||||
closedir(dir);
|
||||
return 1;
|
||||
}
|
||||
n++;
|
||||
}
|
||||
closedir(dir);
|
||||
|
||||
if (n == 0) {
|
||||
fprintf(stderr, "No frame files found in %s\n", frames_dir);
|
||||
return 1;
|
||||
}
|
||||
|
||||
qsort(names, n, sizeof(char *), compare_names);
|
||||
|
||||
size_t total_size = 0;
|
||||
char **frame_contents = calloc(n, sizeof(char*));
|
||||
size_t *frame_sizes = calloc(n, sizeof(size_t));
|
||||
|
||||
for (int i = 0; i < n; i++) {
|
||||
char path[4096];
|
||||
snprintf(path, sizeof(path), "%s/%s", frames_dir, namelist[i]->d_name);
|
||||
snprintf(path, sizeof(path), "%s%c%s", frames_dir, PATH_SEP, names[i]);
|
||||
|
||||
frame_contents[i] = read_file(path, &frame_sizes[i]);
|
||||
if (!frame_contents[i]) {
|
||||
|
||||
@@ -12,7 +12,7 @@ pub fn main() !void {
|
||||
const alloc = gpa.allocator();
|
||||
|
||||
var buf: [4096]u8 = undefined;
|
||||
var stdout = std.fs.File.stdout().writer(&buf);
|
||||
var stdout = std.fs.File.stdout().writerStreaming(&buf);
|
||||
const writer = &stdout.interface;
|
||||
try writer.writeAll(
|
||||
\\// THIS FILE IS AUTO GENERATED
|
||||
|
||||
@@ -34,7 +34,7 @@ pub fn main() !void {
|
||||
|
||||
// Our output always goes to stdout.
|
||||
var buffer: [1024]u8 = undefined;
|
||||
var stdout_writer = std.fs.File.stdout().writer(&buffer);
|
||||
var stdout_writer = std.fs.File.stdout().writerStreaming(&buffer);
|
||||
const writer = &stdout_writer.interface;
|
||||
switch (action) {
|
||||
.bash => try writer.writeAll(@import("extra/bash.zig").completions),
|
||||
|
||||
Reference in New Issue
Block a user