fix(builder): the Python setup checks the setup list as setup.c does #2057

Merged
SimonKimDev merged 1 commit from fix/setup-list-validation into main 2026-11-11 03:10:50 +00:00
SimonKimDev commented 2026-11-11 01:26:26 +00:00 (Migrated from github.com)

The Python builder takes the setup list's names and checksums as they come, so a list with a .. or absolute tag makes install() remove that folder before any checksum is checked. setup.c already refuses such a list.

  • read_manifest checks the tag, folder names and checksums by setup.c's list_name_ok rules before anything is removed.
  • read_manifest turns a non-numeric part size into a SetupError, so the Builder shows its usual error.
  • reuse_part matches a checksum in parts.txt exactly, not as a substring, and skips a cached version name that fails the same rules.

Part of #2045 (item 3).

The Python builder takes the setup list's names and checksums as they come, so a list with a `..` or absolute tag makes `install()` remove that folder before any checksum is checked. `setup.c` already refuses such a list. - `read_manifest` checks the tag, folder names and checksums by `setup.c`'s `list_name_ok` rules before anything is removed. - `read_manifest` turns a non-numeric part size into a `SetupError`, so the Builder shows its usual error. - `reuse_part` matches a checksum in `parts.txt` exactly, not as a substring, and skips a cached version name that fails the same rules. Part of #2045 (item 3).
Arkex-ai commented 2026-11-11 01:38:10 +00:00 (Migrated from github.com)

Both setup-list tests passed on Windows and Ubuntu at 883d6e64. I closed my overlapping #2060. Two small follow-ups from that work may fit here: translate a nonnumeric part size into SetupError so the Builder can display the usual error, and validate the cached version name before using it for part reuse. The published stable, RC1 and Nightly lists each had eight parts in the compatibility check.

Both setup-list tests passed on Windows and Ubuntu at `883d6e64`. I closed my overlapping #2060. Two small follow-ups from that work may fit here: translate a nonnumeric part size into `SetupError` so the Builder can display the usual error, and validate the cached version name before using it for part reuse. The published stable, RC1 and Nightly lists each had eight parts in the compatibility check.
SimonKimDev commented 2026-11-11 01:41:22 +00:00 (Migrated from github.com)

@Arkex-ai sure sounds good, i will add both. Appreciate the review!

@Arkex-ai sure sounds good, i will add both. Appreciate the review!
SimonKimDev commented 2026-11-11 01:44:38 +00:00 (Migrated from github.com)

added

added
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 02:19:17 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. I compared list_name_ok / list_ok with setup.c and they match: same character set, no empty, . or .. part, no leading root, a . dest or - from/to allowed, 64 lowercase hex checksum, https only. Python does not use the run line, so skipping that check is fine. Both follow-ups @Arkex-ai asked for are in: a non-numeric part size becomes a SetupError, and the cached version name is checked before reuse_part uses it. The exact-checksum match via .split() is right, and tests/test_setup_list_names.py covers the refusals and the reuse case (it passes on Linux and Windows).

CI: the red Python source checks are test_make_game_ngx.py (Linux, a switching attribute on a SimpleNamespace) and test_pipeline_state_portable.py (Windows, fopen deprecation under -Werror). Neither touches this PR's files, so they are not caused by it.

Nit, no change needed: the new test file has no trailing newline.

Looks good. I compared `list_name_ok` / `list_ok` with `setup.c` and they match: same character set, no empty, `.` or `..` part, no leading root, a `.` dest or `-` from/to allowed, 64 lowercase hex checksum, https only. Python does not use the `run` line, so skipping that check is fine. Both follow-ups @Arkex-ai asked for are in: a non-numeric part size becomes a `SetupError`, and the cached version name is checked before `reuse_part` uses it. The exact-checksum match via `.split()` is right, and `tests/test_setup_list_names.py` covers the refusals and the reuse case (it passes on Linux and Windows). CI: the red Python source checks are `test_make_game_ngx.py` (Linux, a `switching` attribute on a SimpleNamespace) and `test_pipeline_state_portable.py` (Windows, `fopen` deprecation under -Werror). Neither touches this PR's files, so they are not caused by it. Nit, no change needed: the new test file has no trailing newline.
Commenting is not possible because the repository is archived.
No description provided.