fix(builder): retain bounded Linux Play startup diagnostics #2024

Merged

Arkex-ai merged 1 commit from fix/linux-play-diagnostics into main

2026-11-11 02:45:59 +00:00

Arkex-ai commented

2026-11-10 23:59:13 +00:00

(Migrated from github.com)

Summary

When a game exits before its internal log explains why, Linux Builder's Play button discards the diagnostic that a direct terminal launch would show. Play now leaves a small local startup log after the process exits.

Closes #2022. Related to the diagnosis in #1984.

Technical changes

Start a detached shell that retains the first 64 KiB of stderr, drains the rest and records the child's exit status. Publish the result atomically as hookshot_startup.log in the game folder.
Keep the direct terminal launch path and environment intact. The game still runs when the Builder cannot create the temporary diagnostics directory.

Validation

At 1cf737e on main c09e0ac, a WSL Ubuntu test launched two synthetic children concurrently. Both emitted 1 MiB on stderr and exited 73. The final private log contained the capped prefix and the exit status. A forced publication failure did not prevent the child from running.
Before the patch, the same Play path lost an authored stderr message that was visible when the child was started directly.
The exact-head AppImage contains both launcher files byte-for-byte. In WSL, the extracted launcher retained the synthetic child's stderr and status 73 after the caller exited. The graphical entry point and gameplay were not exercised.
The upstream source suite finished at 338 passed / 0 failed / 10 skipped on Linux and 325 / 0 / 23 on Windows. Linux ran the new regression.

Current-head check	Result
Source suite	Passed
Windows build	Passed
Linux AppImage	Passed

## Summary When a game exits before its internal log explains why, Linux Builder's Play button discards the diagnostic that a direct terminal launch would show. Play now leaves a small local startup log after the process exits. Closes #2022. Related to the diagnosis in #1984. ## Technical changes - Start a detached shell that retains the first 64 KiB of stderr, drains the rest and records the child's exit status. Publish the result atomically as `hookshot_startup.log` in the game folder. - Keep the direct terminal launch path and environment intact. The game still runs when the Builder cannot create the temporary diagnostics directory. ## Validation - At `1cf737e` on main `c09e0ac`, a WSL Ubuntu test launched two synthetic children concurrently. Both emitted 1 MiB on stderr and exited 73. The final private log contained the capped prefix and the exit status. A forced publication failure did not prevent the child from running. - Before the patch, the same Play path lost an authored stderr message that was visible when the child was started directly. - The exact-head AppImage contains both launcher files byte-for-byte. In WSL, the extracted launcher retained the synthetic child's stderr and status 73 after the caller exited. The graphical entry point and gameplay were not exercised. - The upstream source suite finished at **338 passed / 0 failed / 10 skipped on Linux** and **325 / 0 / 23 on Windows**. Linux ran the new regression.  | Current-head check | Result | |---|---| | [Source suite](https://github.com/NotRexed/Hookshot/actions/runs/42927745537) | Passed | | [Windows build](https://github.com/NotRexed/Hookshot/actions/runs/42927745703) | Passed | | [Linux AppImage](https://github.com/NotRexed/Hookshot/actions/runs/42927745635) | Passed |

hckrman101 (Migrated from github.com) approved these changes

2026-11-11 02:19:07 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good. I read the launcher script and the Python change; CI is green on the head (source suite on Linux and Windows, Windows build, AppImage), and I did not run it myself.

The design holds up: the script is passed as sh -c source, so it still works after the AppImage mount goes away. mktemp failure falls back to exec "$1", and an unreadable script falls back to the old direct Popen. The pipeline always drains stderr, so a noisy game is not blocked after the 64 KiB cap. The log is published with an atomic mv, and the temp dir is removed by the trap. hookshot-launch and the terminal path are untouched. The test is declared with RUN_ALL_SKIP for Windows and is discovered by run_all.py, and it covers the concurrent launches and the failed-publish case.

Nits, none blocking:

The log only appears after the game exits, so a hang shows nothing. The README line says "after the game exits", which is accurate.
Every run, including a clean exit 0, replaces the previous hookshot_startup.log, so an earlier crash's log is overwritten by the next good run. That seems fine for a "last run" log, but it is worth knowing.
mv -T is GNU-specific. This is fine on the glibc distros the AppImage targets, and a failure just means no log, not a failed launch.

Looks good. I read the launcher script and the Python change; CI is green on the head (source suite on Linux and Windows, Windows build, AppImage), and I did not run it myself. The design holds up: the script is passed as `sh -c` source, so it still works after the AppImage mount goes away. `mktemp` failure falls back to `exec "$1"`, and an unreadable script falls back to the old direct `Popen`. The pipeline always drains stderr, so a noisy game is not blocked after the 64 KiB cap. The log is published with an atomic `mv`, and the temp dir is removed by the trap. `hookshot-launch` and the terminal path are untouched. The test is declared with `RUN_ALL_SKIP` for Windows and is discovered by `run_all.py`, and it covers the concurrent launches and the failed-publish case. Nits, none blocking: - The log only appears after the game exits, so a hang shows nothing. The README line says "after the game exits", which is accurate. - Every run, including a clean exit 0, replaces the previous `hookshot_startup.log`, so an earlier crash's log is overwritten by the next good run. That seems fine for a "last run" log, but it is worth knowing. - `mv -T` is GNU-specific. This is fine on the glibc distros the AppImage targets, and a failure just means no log, not a failed launch.