fix(builder): Linux Play leaves the game its hookshot_log.txt again #2279

Merged
mooki000 merged 1 commit from builder/play-keeps-log into main 2026-11-11 16:59:11 +00:00
mooki000 commented 2026-11-11 13:20:36 +00:00 (Migrated from github.com)

Since #2024, a Linux player who starts the game with Play (in the Builder after a build, or in the launcher that the Hookshot menu entry and hookshot in the game folder open) gets no new hookshot_log.txt: the game writes its log into #2024's capture instead, which keeps the first 64 KiB in hookshot_startup.log once the game has closed and throws the rest away. The log every issue asks for is missing, or an older run's log is still there and gets sent in its place.

Fixes #2330.

Why it happened

play-diagnostics.sh runs the game as "$1" 2>&1 >/dev/null | head -c 65536 ..., so the game's stderr is a pipe. The game opens hookshot_log.txt only when its stderr is nowhere to log (host/hos_main.c: GetFileType(err) == FILE_TYPE_UNKNOWN), and the POSIX shim answers FILE_TYPE_PIPE for a pipe (host/posix/win32_posix.c GetFileType): to the game, a pipe means someone reads its output, as with ./bin/hookshot | tee. Before #2024, Play gave the game /dev/null, which counts as nowhere, so it wrote its log.

What changed

  • builder/linux/hookshot_builder.py play(): the game's stderr is one end of a socket.socketpair(), and the script reads the other end on its stdin. The shim counts a socket as nowhere to log (it says so for the journal's socket a desktop launcher gives), so the game writes hookshot_log.txt and keeps hookshot_log.1.txt to .4.txt, as before #2024.
  • builder/linux/play-diagnostics.sh: reads the socket instead of a pipe. What reaches it is what the game prints before its log opens. On Linux the display start comes first, so display: SDL_Init: ...: no window and the loader's errors land there, along with the udmabuf warning the game already mirrors there. hookshot_startup.log keeps that, the 64 KiB cap and the exit status, as #2024 made it. The game's stdin is now /dev/null instead of the Builder's. If the script cannot make its temporary folder, the game gets /dev/null for stderr, as before #2024.
  • Nothing changes in the game, so this works with game folders already built. Hookshot.AppImage --play (hookshot-launch) never went through the script and is unchanged.

Why a socket and not a change in the game (a marker that makes it log even with a launcher's pipe): every game folder would need a new build, and a folder built before the change, started from a newer Builder, would still have no log. The script also cannot know in advance whether the game will fail early, so it cannot pick between the two set-ups.

How it was tested

  • tests/test_linux_play_diagnostics.py keeps #2024's checks and adds one. Play starts a small game compiled with the shim's own GetFileType (cut from host/posix/win32_posix.c). It prints a line, opens hookshot_log.txt only when GetFileType(stderr) is FILE_TYPE_UNKNOWN as host/hos_main.c does (the test checks that this condition is still there), and logs 16,384 lines, more than 64 KiB. On main the check fails (no hookshot_log.txt). With this change the whole log is in hookshot_log.txt, and hookshot_startup.log holds the first line and the exit status. It was also run with dash 0.5.12 as the shell (Debian's and Ubuntu's /bin/sh): it passes, and fails on main the same way.

  • python -I -B tests/run_all.py on this branch as opened (77a91124, on main 49f75dc8): 362 passed, 2 failed, 10 skipped. The 2 failures also fail on main on this machine, for reasons outside this change: tests/test_updater_asset_digest.py (its fixture needs the .NET 8 SDK, and only .NET 10 is installed) and tests/test_wayland_libdecor_hint.py (a clang 23 warning under -Werror).

  • The real game: a build of main (bdfe09c, with the same log set-up as main today) on an RTX 4090 with NVIDIA 615.78.08 on Arch Linux. The folder was laid out as the Builder lays it out (bin/hookshot), and each start went through the Builder's own play(), imported from hookshot_builder.py of each revision. The runs were headless and muted, and the game was ended with SIGTERM after 30 s (as closing it would):

    Play of game's stderr hookshot_log.txt hookshot_startup.log
    before #2024 (2009c5f) /dev/null written: 71,757 bytes, 643 lines (none)
    main a pipe not written: the 71,757-byte log of the run before stays 65,569 bytes: the log's first 65,536 bytes (from the version line, cut mid-line after the vk: 600 presents report) and Hookshot exited with status 143
    this branch a socket, then the game's log file written: 77,184 bytes, 682 lines, to vi: 1037 frames presented; the run before's log moved to hookshot_log.1.txt 33 bytes: Hookshot exited with status 143
  • A start that fails before the log opens (no display reachable, so SDL's video start fails in main before the log is set up): main and this branch leave the same 175-byte hookshot_startup.log (display: SDL_Init: wayland,x11 not available: no window and Hookshot exited with status 1), and no hookshot_log.txt. What #2024 added for this case is kept.

  • A start that fails after the log has opened: an empty image.bin, which play() still starts. The game logs its first lines, then stops with image.bin: 0 bytes, the lifted code expects 0x6020000: lift and image do not match, exit 1. With main's Play these three lines and the exit status are in hookshot_startup.log, and no hookshot_log.txt is written. With this change they are in hookshot_log.txt (274 bytes, the message is its last line), and hookshot_startup.log holds only Hookshot exited with status 1.

Not tested

  • The Builder's window itself (a click on Play). The test and the runs call the same play() the button calls.
  • The AppImage. The script is carried as before (no new file), and socket is part of every CPython, including the AppImage's python-build-standalone 3.12 (the version these runs used).
  • A start with a window on a desktop session: the runs were headless. The failure run covers the part before the window.
  • Distributions other than Arch. dash was only a stand-in shell for the test.

#1958 (profiles) changes two lines of the same test file. Whichever merges second needs a small rebase there.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 [email protected]

Since #2024, a Linux player who starts the game with **Play** (in the Builder after a build, or in the launcher that the Hookshot menu entry and `hookshot` in the game folder open) gets no new `hookshot_log.txt`: the game writes its log into #2024's capture instead, which keeps the first 64 KiB in `hookshot_startup.log` once the game has closed and throws the rest away. The log every issue asks for is missing, or an older run's log is still there and gets sent in its place. Fixes #2330. ## Why it happened `play-diagnostics.sh` runs the game as `"$1" 2>&1 >/dev/null | head -c 65536 ...`, so the game's stderr is a pipe. The game opens `hookshot_log.txt` only when its stderr is nowhere to log (`host/hos_main.c`: `GetFileType(err) == FILE_TYPE_UNKNOWN`), and the POSIX shim answers `FILE_TYPE_PIPE` for a pipe (`host/posix/win32_posix.c` `GetFileType`): to the game, a pipe means someone reads its output, as with `./bin/hookshot | tee`. Before #2024, Play gave the game `/dev/null`, which counts as nowhere, so it wrote its log. ## What changed - `builder/linux/hookshot_builder.py` `play()`: the game's stderr is one end of a `socket.socketpair()`, and the script reads the other end on its stdin. The shim counts a socket as nowhere to log (it says so for the journal's socket a desktop launcher gives), so the game writes `hookshot_log.txt` and keeps `hookshot_log.1.txt` to `.4.txt`, as before #2024. - `builder/linux/play-diagnostics.sh`: reads the socket instead of a pipe. What reaches it is what the game prints before its log opens. On Linux the display start comes first, so `display: SDL_Init: ...: no window` and the loader's errors land there, along with the udmabuf warning the game already mirrors there. `hookshot_startup.log` keeps that, the 64 KiB cap and the exit status, as #2024 made it. The game's stdin is now `/dev/null` instead of the Builder's. If the script cannot make its temporary folder, the game gets `/dev/null` for stderr, as before #2024. - Nothing changes in the game, so this works with game folders already built. `Hookshot.AppImage --play` (`hookshot-launch`) never went through the script and is unchanged. Why a socket and not a change in the game (a marker that makes it log even with a launcher's pipe): every game folder would need a new build, and a folder built before the change, started from a newer Builder, would still have no log. The script also cannot know in advance whether the game will fail early, so it cannot pick between the two set-ups. ## How it was tested - `tests/test_linux_play_diagnostics.py` keeps #2024's checks and adds one. Play starts a small game compiled with the shim's own `GetFileType` (cut from `host/posix/win32_posix.c`). It prints a line, opens `hookshot_log.txt` only when `GetFileType(stderr)` is `FILE_TYPE_UNKNOWN` as `host/hos_main.c` does (the test checks that this condition is still there), and logs 16,384 lines, more than 64 KiB. On `main` the check fails (`no hookshot_log.txt`). With this change the whole log is in `hookshot_log.txt`, and `hookshot_startup.log` holds the first line and the exit status. It was also run with dash 0.5.12 as the shell (Debian's and Ubuntu's `/bin/sh`): it passes, and fails on `main` the same way. - `python -I -B tests/run_all.py` on this branch as opened (77a91124, on `main` 49f75dc8): 362 passed, 2 failed, 10 skipped. The 2 failures also fail on `main` on this machine, for reasons outside this change: `tests/test_updater_asset_digest.py` (its fixture needs the .NET 8 SDK, and only .NET 10 is installed) and `tests/test_wayland_libdecor_hint.py` (a clang 23 warning under `-Werror`). - The real game: a build of `main` (bdfe09c, with the same log set-up as `main` today) on an RTX 4090 with NVIDIA 615.78.08 on Arch Linux. The folder was laid out as the Builder lays it out (`bin/hookshot`), and each start went through the Builder's own `play()`, imported from `hookshot_builder.py` of each revision. The runs were headless and muted, and the game was ended with SIGTERM after 30 s (as closing it would): | Play of | game's stderr | `hookshot_log.txt` | `hookshot_startup.log` | |---|---|---|---| | before #2024 (2009c5f) | `/dev/null` | written: 71,757 bytes, 643 lines | (none) | | `main` | a pipe | **not written**: the 71,757-byte log of the run before stays | 65,569 bytes: the log's first 65,536 bytes (from the version line, cut mid-line after the `vk: 600 presents` report) and `Hookshot exited with status 143` | | this branch | a socket, then the game's log file | written: 77,184 bytes, 682 lines, to `vi: 1037 frames presented`; the run before's log moved to `hookshot_log.1.txt` | 33 bytes: `Hookshot exited with status 143` | - A start that fails before the log opens (no display reachable, so SDL's video start fails in `main` before the log is set up): `main` and this branch leave the same 175-byte `hookshot_startup.log` (`display: SDL_Init: wayland,x11 not available: no window` and `Hookshot exited with status 1`), and no `hookshot_log.txt`. What #2024 added for this case is kept. - A start that fails after the log has opened: an empty `image.bin`, which `play()` still starts. The game logs its first lines, then stops with `image.bin: 0 bytes, the lifted code expects 0x6020000: lift and image do not match`, exit 1. With `main`'s Play these three lines and the exit status are in `hookshot_startup.log`, and no `hookshot_log.txt` is written. With this change they are in `hookshot_log.txt` (274 bytes, the message is its last line), and `hookshot_startup.log` holds only `Hookshot exited with status 1`. ## Not tested - The Builder's window itself (a click on Play). The test and the runs call the same `play()` the button calls. - The AppImage. The script is carried as before (no new file), and `socket` is part of every CPython, including the AppImage's python-build-standalone 3.12 (the version these runs used). - A start with a window on a desktop session: the runs were headless. The failure run covers the part before the window. - Distributions other than Arch. dash was only a stand-in shell for the test. #1958 (profiles) changes two lines of the same test file. Whichever merges second needs a small rebase there. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 16:49:32 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. I read the Linux Play path and the script's fd handling:

  • play() hands the script a socketpair (stdin = reading end, stderr = writing end). The script moves stdin to fd 3 for a background head | cat reader, closes its own copy, and runs the game with stdout to /dev/null, stdin /dev/null and stderr the socket. GetFileType on main returns FILE_TYPE_UNKNOWN for a socket (the case its comment already names for the journal's socket), so the game opens hookshot_log.txt as before #2024.
  • EOF for the reader is right: the game's write end and the script's fd 2 are the only writers, and the script does exec 2>/dev/null after the game exits before waiting. The Python side closes both ends after Popen, and the mktemp-failure fallback gives the game /dev/null as before.
  • Only Linux builder files and a Linux-only test change (the test is skipped on windows), so the Windows builder and game are untouched.
  • The new test cuts GetFileType out of the shim and checks that hos_main.c still has the condition, so it will notice if either side drifts. {cxx} is a supported RUN_ALL_ARGS placeholder.

The PR text claims match what I see in the diff. Real-game numbers are the author's own run on Arch, so I did not rerun them. The Linux AppImage, Windows builder and Game-free source regression workflows show action_required (first-time contributor approval), so they have not run yet. Someone with rights needs to approve them before merging. Only the title check has run, and it passes.

Looks good. I read the Linux Play path and the script's fd handling: - `play()` hands the script a socketpair (stdin = reading end, stderr = writing end). The script moves stdin to fd 3 for a background `head | cat` reader, closes its own copy, and runs the game with stdout to /dev/null, stdin /dev/null and stderr the socket. `GetFileType` on main returns FILE_TYPE_UNKNOWN for a socket (the case its comment already names for the journal's socket), so the game opens `hookshot_log.txt` as before #2024. - EOF for the reader is right: the game's write end and the script's fd 2 are the only writers, and the script does `exec 2>/dev/null` after the game exits before waiting. The Python side closes both ends after Popen, and the mktemp-failure fallback gives the game /dev/null as before. - Only Linux builder files and a Linux-only test change (the test is skipped on windows), so the Windows builder and game are untouched. - The new test cuts `GetFileType` out of the shim and checks that hos_main.c still has the condition, so it will notice if either side drifts. `{cxx}` is a supported RUN_ALL_ARGS placeholder. The PR text claims match what I see in the diff. Real-game numbers are the author's own run on Arch, so I did not rerun them. The Linux AppImage, Windows builder and Game-free source regression workflows show `action_required` (first-time contributor approval), so they have not run yet. Someone with rights needs to approve them before merging. Only the title check has run, and it passes.
mooki000 commented 2026-11-11 16:54:51 +00:00 (Migrated from github.com)

Thanks for the careful read. About the workflows: GitHub now starts them for my pushes without an approval (it did for #2380). Main's game-free tests fail until #2380 is in, so I'll rebase this onto main right after that merge and push; the Linux AppImage, Windows builder and game-free runs should then show here on their own.

Thanks for the careful read. About the workflows: GitHub now starts them for my pushes without an approval (it did for #2380). Main's game-free tests fail until #2380 is in, so I'll rebase this onto main right after that merge and push; the Linux AppImage, Windows builder and game-free runs should then show here on their own.
Commenting is not possible because the repository is archived.
No description provided.