Skip to content

Run CI on pull requests, on the vendored branch, and on Windows - #10

Open
dougchansan wants to merge 1 commit into
ExpansionPak:moderngekko-vendorfrom
dougchansan:ci-pr-and-windows
Open

Run CI on pull requests, on the vendored branch, and on Windows#10
dougchansan wants to merge 1 commit into
ExpansionPak:moderngekko-vendorfrom
dougchansan:ci-pr-and-windows

Conversation

@dougchansan

@dougchansan dougchansan commented Aug 6, 2026

Copy link
Copy Markdown

Three gaps in the current workflow, each of which let a break through.

Nothing ran on a pull request — only push to main and manual dispatch. So a change could reach a branch other people vendor before anything compiled it. The missing include that stopped core building (#7) got in exactly that way.

Only main was watched on push, but moderngekko-vendor is the branch ModernGekko actually vendors. The branch being consumed was the one not being checked.

Windows was not built at all. That is how the precompiled-header flags being applied to C sources — GXRuntime handed a C++ PCH, fatal error C1853 — reached the vendored branch and stopped the downstream launcher from compiling.

Also: the unit tests exist and nothing ran them. tests is EXCLUDE_FROM_ALL, so it has to be asked for by name. Linux and Windows now build it and run ctest.

Concurrency cancels superseded runs, since these builds are not cheap.

Verified

YAML parses; triggers, watched branches and jobs are what they should be. The tests target name and its EXCLUDE_FROM_ALL status are read from Source/UnitTests/CMakeLists.txt rather than assumed.

I have not been able to rehearse the runners themselves. The macOS and Linux jobs are unchanged apart from the test steps, so they should behave as before; the Windows job is new and may need a fixup on its first run.

Note

The Windows job will fail until #7 lands — that is the point of it. Merge #7 first.

How this fits

These land together as a set: savestates working end to end, the Windows build and test suite being usable at all, and CI so none of it regresses unnoticed. This PR is RC #10.

MG #18 Bump vendored RecompCore prerequisite - without it the launcher cannot compile on Windows (C sources get a C++ PCH)
RC #7 Interpreter.cpp include prerequisite - core does not compile without it
RC #8 In-game File/View menu and hotkeys introduces Core/SavestateLayout.h, the one definition of where savestates live, what they are called and how they are ordered
MG #21 Launcher savestate picker, --load-state consumes that header, so the launcher and the in-game menu cannot disagree
MG #22 Make the test suite pass on Windows land before CI, or the first run is red on day one
MG #23 Build and test on PRs, three platforms the reason the rest stayed broken unnoticed
RC #10 CI on PRs, the vendored branch, and Windows same gap, other repo; its Windows job fails until RC #7 lands
MG #19 --opt-level, default -O2 independent
MG #20 Cache-domain affinity independent

Suggested order: #18 and RC #7, then RC #8, then MG #21; MG #22 before MG #23. The rest are independent.

Verified on three platforms: Windows (MSVC + clang), Linux (g++ 15.2, x86_64) and macOS 26.1 (clang, arm64). The portable pieces - the savestate layout and its tests, frontend_config, dol_patch - build and pass on all three. Windows-only pieces are guarded and their tests registered behind if(WIN32).

Three gaps, each of which let a break through.

Nothing ran on a pull request, so a change reached a branch other people
vendor before anything compiled it. A missing include that stopped `core`
building got in exactly that way.

Only `main` was watched on push, but `moderngekko-vendor` is the branch
ModernGekko actually vendors, so the branch being consumed was the one
not being checked.

Windows was not built at all. That is how the precompiled-header flags
being applied to C sources -- GXRuntime handed a C++ PCH, fatal error
C1853 -- reached the vendored branch and stopped the downstream launcher
compiling.

The unit tests also existed without anything running them; `tests` is
EXCLUDE_FROM_ALL, so it has to be asked for by name. Linux and Windows
now build it and run ctest.

Concurrency cancels superseded runs, since these builds are not cheap.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant