fix(release): the setup, the Builder and the game update from the repository they were built in #1414

Open
notquiteog wants to merge 1 commit from notquiteog/pr/release-repository into main
notquiteog commented 2026-11-10 01:15:26 +00:00 (Migrated from github.com)

What

The setup, the Builder's updater, the Builder's "releases" link, the game's update check and the Linux updater now take the GitHub repository they install and update from from one setting, HOOKSHOT_REPOSITORY (owner/name), instead of naming NotRexed/Hookshot in six places. The default is NotRexed/Hookshot, so nothing changes for this repository's builds.

  • builder/setup/setup.c: a compile-time define; windows-builder.yml passes $GITHUB_REPOSITORY (the setup's compile line keeps --dependent-load-flag=0x800).
  • The Builder (C#): tools/repository.txt, read by a new Repository class (Updater.cs, MainWindow.xaml.cs); scripts/release_parts.py package --repository writes it, and the workflow passes $GITHUB_REPOSITORY.
  • host/hos_update.c: a define from nx2/version.py repository() (HOOKSHOT_REPOSITORY, else repository.txt beside the tools, else the default), given by nx2/build.py to that one file.
  • builder/linux/linux_update.py: repository.txt in the AppImage's tools, written by scripts/linux/appimage.sh from $GITHUB_REPOSITORY.

Why

A fork's release installed and offered this repository's builds, never its own: its HookshotBuilder.exe fetched hookshot-setup.txt from NotRexed/Hookshot's release, and its update checks asked NotRexed/Hookshot's API. Same kind of fix as #1312 (the AppImage's parts list), for the rest of the update path. Found on notquiteog/Hookshot, where it is in use since v2.0.12.

Testing

  • builder/setup/setup.c and host/hos_update.c compile for Windows (MinGW) and host/hos_update.c for Linux, with and without the define.
  • Preprocessed setup.c without the define names https://github.com/NotRexed/Hookshot/releases/... in its URLs, as before; with -DHOOKSHOT_REPOSITORY="notquiteog/Hookshot" all three URLs name the fork.
  • nx2/version.py and linux_update.py fall back to NotRexed/Hookshot without the file.
  • On the fork (since v2.0.12): the downloaded HookshotBuilder.exe contains the fork's setup-list URLs, and the app part carries tools/repository.txt = notquiteog/Hookshot; the Builder's C# compiled in that CI run.
  • tests/run_all.py on this branch: 203 passed, 0 failed, 5 skipped.

Not covered

  • The C# was compiled by CI only (no .NET where this was written).
  • An update offered and installed end to end from a fork's release has not been watched on a player's PC.

🤖 Generated with Claude Code

## What The setup, the Builder's updater, the Builder's "releases" link, the game's update check and the Linux updater now take the GitHub repository they install and update from from one setting, `HOOKSHOT_REPOSITORY` (`owner/name`), instead of naming `NotRexed/Hookshot` in six places. **The default is `NotRexed/Hookshot`, so nothing changes for this repository's builds.** - `builder/setup/setup.c`: a compile-time define; `windows-builder.yml` passes `$GITHUB_REPOSITORY` (the setup's compile line keeps `--dependent-load-flag=0x800`). - The Builder (C#): `tools/repository.txt`, read by a new `Repository` class (`Updater.cs`, `MainWindow.xaml.cs`); `scripts/release_parts.py package --repository` writes it, and the workflow passes `$GITHUB_REPOSITORY`. - `host/hos_update.c`: a define from `nx2/version.py repository()` (`HOOKSHOT_REPOSITORY`, else `repository.txt` beside the tools, else the default), given by `nx2/build.py` to that one file. - `builder/linux/linux_update.py`: `repository.txt` in the AppImage's tools, written by `scripts/linux/appimage.sh` from `$GITHUB_REPOSITORY`. ## Why A fork's release installed and offered this repository's builds, never its own: its `HookshotBuilder.exe` fetched `hookshot-setup.txt` from NotRexed/Hookshot's release, and its update checks asked NotRexed/Hookshot's API. Same kind of fix as #1312 (the AppImage's parts list), for the rest of the update path. Found on notquiteog/Hookshot, where it is in use since v2.0.12. ## Testing - `builder/setup/setup.c` and `host/hos_update.c` compile for Windows (MinGW) and `host/hos_update.c` for Linux, with and without the define. - Preprocessed `setup.c` without the define names `https://github.com/NotRexed/Hookshot/releases/...` in its URLs, as before; with `-DHOOKSHOT_REPOSITORY="notquiteog/Hookshot"` all three URLs name the fork. - `nx2/version.py` and `linux_update.py` fall back to `NotRexed/Hookshot` without the file. - On the fork (since v2.0.12): the downloaded `HookshotBuilder.exe` contains the fork's setup-list URLs, and the app part carries `tools/repository.txt` = `notquiteog/Hookshot`; the Builder's C# compiled in that CI run. - `tests/run_all.py` on this branch: 203 passed, 0 failed, 5 skipped. ## Not covered - The C# was compiled by CI only (no .NET where this was written). - An update offered and installed end to end from a fork's release has not been watched on a player's PC. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) reviewed 2026-11-10 04:18:46 +00:00
hckrman101 (Migrated from github.com) left a comment

Nice, small change, and the default keeps upstream builds identical. I read the Windows (setup, Builder, game) and Linux paths; the compile-time define, the C# and the L## widening all look right to me, and the define is part of the object cache key in nx2/build.py so a fork build won't reuse an upstream hos_update.o. CI is green.

One real gap on Linux, which is half of what this PR is for:

  • scripts/linux/appimage.sh still calls python3 scripts/linux/make_parts.py "$APPDIR" "$VERSION" "$B/parts" without --base, and make_parts.py defaults --base to https://github.com/NotRexed/Hookshot/releases/download/linux-parts. publish_parts.sh uploads the parts to $GITHUB_REPOSITORY's linux-parts, but the hookshot-linux.txt a fork publishes names upstream's linux-parts URLs. So with this PR a fork's AppImage reads the fork's list (good), but then downloads the parts from NotRexed (the Windows side already passes --base "https://github.com/$GITHUB_REPOSITORY/..."). Passing --base "https://github.com/${GITHUB_REPOSITORY:-NotRexed/Hookshot}/releases/download/linux-parts" in appimage.sh closes it. Worth fixing here, since otherwise the Linux update still pulls upstream binaries on a fork.

Smaller points:

  • Nit: no test covers any of this. nx2/version.py repository() / repository_define() and linux_update._repository() (file present, file missing, bad value falling back to the default) are cheap to unit test, e.g. in tests/test_linux_update.py (already registered in tests/run_all.py), so no new registration is needed.
  • Nit: the validation regex [A-Za-z0-9._-]+ for the name part also accepts . and ... Harmless since the value comes from CI or a file you ship, but owner/.. would build a path-traversing URL; (?!\.+$) or similar would rule it out.
  • Nit: linux_update._repository() ignores HOOKSHOT_REPOSITORY while nx2/version.py honors it, though the comment says it is the same rule. Fine either way, just make the comment say so.
  • Nit: import re inside the function in linux_update.py; could go at the top.

Trust/redirects: the repository only comes from the CI env at packaging time (or a build-time env var for source builds), is regex-checked before use, and ends up in fixed github.com / api.github.com URL templates, so I don't see a way to point updates at another host. GitHub follows renames/transfers, so a renamed fork keeps working. Note a fork's players trust the fork's releases, which is the intent here.

Overlaps: none with #1201 or #1452 (release.yml / package.json only). #1059 and #1062 both edit windows-builder.yml (setup compile / package steps, version step), linux-appimage.yml and scripts/linux/appimage.sh, so expect small textual conflicts in whichever lands second; no semantic conflict, since GITHUB_REPOSITORY is the same inside the workflow_call they add. The package --repository and the compile -D lines are one-line additions, so rebasing should be easy.

Option ids aren't touched.

Nice, small change, and the default keeps upstream builds identical. I read the Windows (setup, Builder, game) and Linux paths; the compile-time define, the C# and the `L##` widening all look right to me, and the define is part of the object cache key in `nx2/build.py` so a fork build won't reuse an upstream `hos_update.o`. CI is green. One real gap on Linux, which is half of what this PR is for: - `scripts/linux/appimage.sh` still calls `python3 scripts/linux/make_parts.py "$APPDIR" "$VERSION" "$B/parts"` without `--base`, and `make_parts.py` defaults `--base` to `https://github.com/NotRexed/Hookshot/releases/download/linux-parts`. `publish_parts.sh` uploads the parts to `$GITHUB_REPOSITORY`'s `linux-parts`, but the `hookshot-linux.txt` a fork publishes names upstream's `linux-parts` URLs. So with this PR a fork's AppImage reads the fork's list (good), but then downloads the parts from NotRexed (the Windows side already passes `--base "https://github.com/$GITHUB_REPOSITORY/..."`). Passing `--base "https://github.com/${GITHUB_REPOSITORY:-NotRexed/Hookshot}/releases/download/linux-parts"` in `appimage.sh` closes it. Worth fixing here, since otherwise the Linux update still pulls upstream binaries on a fork. Smaller points: - Nit: no test covers any of this. `nx2/version.py repository()` / `repository_define()` and `linux_update._repository()` (file present, file missing, bad value falling back to the default) are cheap to unit test, e.g. in `tests/test_linux_update.py` (already registered in `tests/run_all.py`), so no new registration is needed. - Nit: the validation regex `[A-Za-z0-9._-]+` for the name part also accepts `.` and `..`. Harmless since the value comes from CI or a file you ship, but `owner/..` would build a path-traversing URL; `(?!\.+$)` or similar would rule it out. - Nit: `linux_update._repository()` ignores `HOOKSHOT_REPOSITORY` while `nx2/version.py` honors it, though the comment says it is the same rule. Fine either way, just make the comment say so. - Nit: `import re` inside the function in `linux_update.py`; could go at the top. Trust/redirects: the repository only comes from the CI env at packaging time (or a build-time env var for source builds), is regex-checked before use, and ends up in fixed `github.com` / `api.github.com` URL templates, so I don't see a way to point updates at another host. GitHub follows renames/transfers, so a renamed fork keeps working. Note a fork's players trust the fork's releases, which is the intent here. Overlaps: none with #1201 or #1452 (release.yml / package.json only). #1059 and #1062 both edit `windows-builder.yml` (setup compile / package steps, version step), `linux-appimage.yml` and `scripts/linux/appimage.sh`, so expect small textual conflicts in whichever lands second; no semantic conflict, since `GITHUB_REPOSITORY` is the same inside the `workflow_call` they add. The package `--repository` and the compile `-D` lines are one-line additions, so rebasing should be easy. Option ids aren't touched.
hckrman101 commented 2026-11-10 17:49:15 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:45:53 +00:00
Joey-Magnus (Migrated from github.com) left a comment

Half of the earlier review is resolved: main fixed the Linux parts gap on its own. In 2228816, scripts/linux/appimage.sh now passes --base "https://github.com/${GITHUB_REPOSITORY:-NotRexed/Hookshot}/releases/download/linux-parts" and builds UPDATE_REPO the same way.

The branch conflicts in windows-builder.yml, linux_update.py, setup.c, hos_update.c, nx2/build.py and release_parts.py, though. Since this branch was cut, #2061 (update tracks) added new hard-coded NotRexed/Hookshot URLs that this PR doesn't cover yet:

  • the nightly hookshot-setup.txt and hookshot-linux.txt URLs in setup.c:636, hos_update.c:166/331 and Updater.cs:96
  • the releases?per_page=20 lists in hos_update.c:170/334, Updater.cs:41 and linux_update.py:28
  • release_of() in linux_update.py:133

Without those, a fork on the Release candidate or Nightly track still updates from upstream. builder/py/hookshot_builder/__init__.py also hard-codes REPO. The earlier nits are still open: no unit test for repository(), and the regex accepts owner/...

Half of the earlier review is resolved: main fixed the Linux parts gap on its own. In 2228816, scripts/linux/appimage.sh now passes `--base "https://github.com/${GITHUB_REPOSITORY:-NotRexed/Hookshot}/releases/download/linux-parts"` and builds `UPDATE_REPO` the same way. The branch conflicts in windows-builder.yml, linux_update.py, setup.c, hos_update.c, nx2/build.py and release_parts.py, though. Since this branch was cut, #2061 (update tracks) added new hard-coded NotRexed/Hookshot URLs that this PR doesn't cover yet: - the nightly `hookshot-setup.txt` and `hookshot-linux.txt` URLs in setup.c:636, hos_update.c:166/331 and Updater.cs:96 - the `releases?per_page=20` lists in hos_update.c:170/334, Updater.cs:41 and linux_update.py:28 - `release_of()` in linux_update.py:133 Without those, a fork on the Release candidate or Nightly track still updates from upstream. `builder/py/hookshot_builder/__init__.py` also hard-codes `REPO`. The earlier nits are still open: no unit test for `repository()`, and the regex accepts `owner/..`.
Commenting is not possible because the repository is archived.
No description provided.