You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(tools): prevent configure seeds from overwriting their cache - #8085
Passing the input CMakeCache.txt as the seed output currently replaces the full cache with generated statements and exits successfully. Reject identical paths, normalized aliases, symbolic links and hard links before reading, filtering or writing, with an argparse error that preserves the cache. Distinct output files retain their existing behavior, including include/exclude filtering.
Rebased onto main at 1ed63f63c26f56a007238f1696c94f48ae4d6be7, which includes the UTF-8 BOM parsing fix from #7997. Split the compiler-backed tests into groups to respect the 30-second per-test limit. Register all four concrete test classes: 3 integration checks, 7 path checks, 3 round-trip checks and 1 BOM parsing check. The BOM check has its own configure_seed_parsing CTest entry, addressing the review finding that class-based selection would otherwise leave it unexecuted.
Tested
Ubuntu 26.04.1 x86-64 in a temporary WSL2 environment, GCC 15.2.0, CMake 4.2.3, Ninja 1.13.2 and Python 3.14.4. Configured the real project with -DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Release and built all its default targets.
Negative control on the rebased branch before the two-line registration fix: unittest discovery finds 14 test methods, but CTest's three class selections register only 13. The coverage audit fails specifically for ChecksParsingTests.test_checks_reads_cache_with_utf8_bom.
Final commit d96a010710e3fad87a82d949a3b42b00d2571212: CTest's four class selections cover all 14 methods exactly once, with no missing or duplicate methods. All 14 methods were actually executed through the four CTest entries, zero failures and zero skips, including real symlinks/hard links and compiler-backed round trips. Longest entry: 21.500 s.
test_registration: 34 Python tests passed, zero skips; longest CTest entry 0.224 s. Both CTest runs used -V --timeout 30 --parallel 2; JUnit and verbose logs were checked for failures, skips and the expected test counts.
Tested SHA-256 values of CMakeLists.txt, configure_seed.py and test_configure_seed.py match the committed blobs. tools/check_test_registration.py passes against the built Ninja tree.
Fresh final review of the updated diff, test evidence and this description by Astra (gpt-6-astra, reasoning high) before pushing: no open findings.
Scope: local validation is a native Linux relinker-only build plus the focused tests above. The complete CTest suite, full system-library/application builds, Windows/macOS local tests, game/title and GPU execution were not run. GitHub Actions checks for this updated head are verified after the push; older head results are not presented as results for this one.
GitHub Actions on the updated head: relinker workflow passed Linux, Windows, macOS ARM64, macOS x86_64 and ASAN; conventions passed. All four configure_seed entries actually ran on each platform and stayed below 30 seconds:
CI platform
configure_seed
paths
round_trips
parsing
Linux
1.09 s
0.64 s
2.09 s
0.08 s
Windows
4.10 s
1.14 s
8.10 s
0.11 s
macOS ARM64
4.29 s
0.81 s
10.66 s
0.11 s
macOS x86_64
8.75 s
1.63 s
11.65 s
0.24 s
Checklist
Based on current main; no other open PR implements the same functions
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
@vittoriosalvadore thanks, the guard works (same path, normalized alias, symlink and hard link are all refused and the cache is left alone). One problem from the rebase base: since this branch was cut, main added ChecksParsingTests (test_checks_reads_cache_with_utf8_bom, from #7997) to tools/tests/test_configure_seed.py. The three new registrations select tests by class name, so after the merge that class is in none of them and CTest stops running it (the groups cover 13 of the file's 14 tests). Could you rebase on main and register ChecksParsingTests too, either as its own add_test or inside one of the three groups?
@oneandonlydean Thank you for the review. Rebased on main and registered ChecksParsingTests in #8085. All 48 focused local tests pass with zero skips; the relinker CI checks also pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Passing the input CMakeCache.txt as the seed output currently replaces the full cache with generated statements and exits successfully. Reject identical paths, normalized aliases, symbolic links and hard links before reading, filtering or writing, with an argparse error that preserves the cache. Distinct output files retain their existing behavior, including include/exclude filtering.
Rebased onto main at
1ed63f63c26f56a007238f1696c94f48ae4d6be7, which includes the UTF-8 BOM parsing fix from #7997. Split the compiler-backed tests into groups to respect the 30-second per-test limit. Register all four concrete test classes: 3 integration checks, 7 path checks, 3 round-trip checks and 1 BOM parsing check. The BOM check has its ownconfigure_seed_parsingCTest entry, addressing the review finding that class-based selection would otherwise leave it unexecuted.Tested
Ubuntu 26.04.1 x86-64 in a temporary WSL2 environment, GCC 15.2.0, CMake 4.2.3, Ninja 1.13.2 and Python 3.14.4. Configured the real project with
-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Releaseand built all its default targets.ChecksParsingTests.test_checks_reads_cache_with_utf8_bom.d96a010710e3fad87a82d949a3b42b00d2571212: CTest's four class selections cover all 14 methods exactly once, with no missing or duplicate methods. All 14 methods were actually executed through the four CTest entries, zero failures and zero skips, including real symlinks/hard links and compiler-backed round trips. Longest entry: 21.500 s.test_registration: 34 Python tests passed, zero skips; longest CTest entry 0.224 s. Both CTest runs used-V --timeout 30 --parallel 2; JUnit and verbose logs were checked for failures, skips and the expected test counts.tools/check_test_registration.pypasses against the built Ninja tree.python -B tools/check_conventions.py --base origin/mainandgit diff --check origin/main...HEAD: passed.python -B tools/pr_overlap.py --branch codex/fix-configure-seed-input-alias --jobs 4: the only confirmed match is fix(tools): prevent configure seeds from overwriting their cache #8085 itself, because its old remote head is no longer an ancestor after rebasing; this is an update to that PR. Eleven existing PRs conflict with main and share the root CMakeLists.txt, preventing automatic comparison: feat(input): add native SDL input configuration with ImGui #674, feat(aot): discuss native ARM64 compiler architecture and integration #1835, feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021, test: organize library tests and label CTest suites #6044, feat(wasm): run relinked titles in the browser with WebGPU #6879, perf(agc): draw finisher pipeline (APS5_DRAW_PIPELINE) #6946, perf(agc): GPU descriptor heap (APS5_GPU_HEAP) #6947, Macos port #7201, feat: macOS on Apple Silicon host support (Rosetta 2 and MoltenVK) #7332, fix(tools): fail when a test_*.py script is never run #7523 and feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642. Their complete CMake diffs were reviewed manually; none changes configure_seed or duplicates these class registrations. test: organize library tests and label CTest suites #6044 changes component labels for other tests and fix(tools): fail when a test_*.py script is never run #7523 adds a script-registration checker; neither implements this cache-alias guard or the BOM registration correction. Depends on: none.gpt-6-astra, reasoning high) before pushing: no open findings.Scope: local validation is a native Linux relinker-only build plus the focused tests above. The complete CTest suite, full system-library/application builds, Windows/macOS local tests, game/title and GPU execution were not run. GitHub Actions checks for this updated head are verified after the push; older head results are not presented as results for this one.
GitHub Actions on the updated head: relinker workflow passed Linux, Windows, macOS ARM64, macOS x86_64 and ASAN; conventions passed. All four configure_seed entries actually ran on each platform and stayed below 30 seconds:
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)