fix(cores): build mGBA with cmake, and unpin fceumm onto the fixed patch - #28
Merged
Merged
Conversation
Adopted from upstream LoveRetro#799 by Prashant Vaibhav, taken while upstream is still GPL-3.0 (see LoveRetro#765). Clears the last two boxes in #16. Both cores were pinned backwards to dodge upstream drift. That was a holding action; this is the real fix, so the pins move forwards instead. mgba: libretro/mgba deleted Makefile.libretro in 8940477 when its CI moved to cmake, and patches/mgba.patch existed only to add a platform block to that file. The patch is deleted and the same flags now go to cmake directly via mgba_MAKE, with mgba_CORE pointing at the build dir. Pinned to e31759b (2026-08-05), verified cmake-only, exposing BUILD_LIBRETRO / LIBMGBA_ONLY / LIBRETRO_STATIC and building target mgba_libretro. cmake is present in the toolchain image, which also ships an aarch64 cross toolchain file. fceumm: the rewritten patch targets the post-8ed0cb2 Makefile.libretro, so it cannot apply to the commit we pinned to avoid that change. Pinned to b5e3566, whose Makefile.libretro blob is the exact pre-image the patch was written against. Verified per #16's method - `git apply --check` at the new pin passes for both tg5040 and tg5050, and fails at the old pin, so patch and pin have to move together. Deviation from upstream: their mgba_MAKE ends `-- $(PROCS)`, but the shared rule in all/cores/makefile already appends $(PROCS), which expanded to `-- -j4 -j4`. Dropped ours, keeping the `--` so -j4 still reaches the native build tool rather than becoming a cmake flag. Not built for ARM here - no container runtime in this environment - so the cmake cross-compile itself is still unproven on our side. CI covers it. Co-Authored-By: Prashant Vaibhav <prashant@vaibhav.de>
lepht
pushed a commit
that referenced
this pull request
Aug 12, 2026
Brings in the three upstream adoptions (#26, #27, #28) now on main. One conflict, in the scroll-text block of nextui.c. Both sides edited adjacent lines for unrelated reasons: - this branch added SCREEN_CONTEXTMENU to the outer screen exclusion list - #27 added a total>0 guard to the inner condition Resolved as the union of both, which is the only correct answer here. Taking ours alone drops total>0 and reopens #6 two lines above the top->entries->items[top->selected] read; taking theirs alone drops the context menu exclusion and renders scroll text over it. Git auto-merged the other five guard sites #27 added. Verified after the merge: no assert(entry) remains, every read of entries->items[top->selected] sits behind a count or total>0 check, nextui.c is syntax-clean against the desktop platform headers, and make test passes.
lepht
pushed a commit
that referenced
this pull request
Aug 12, 2026
Carries the main merge (and with it #26, #27, #28) up the stack. Same single conflict as one level down, in the same two lines of nextui.c: this branch had added SCREEN_SEARCH to the outer screen exclusion list on top of SCREEN_CONTEXTMENU, while #27 added a total>0 guard to the inner condition. Resolved as the union again - all four screen exclusions kept, total>0 kept. Verified after the merge: no assert(entry) remains, every read of entries->items[top->selected] is guarded, nextui.c is syntax-clean against the desktop platform headers, and make test passes.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Clears the last two unchecked boxes in #16. Adopted from upstream LoveRetro/NextUI#799 by @pvaibhav.
The point
Both cores were pinned backwards to dodge upstream drift. That was a holding action, and #16 said so at the time. This is the actual fix, so the pins move forwards instead.
mgba — cmake
libretro/mgbadeletedMakefile.libretroin8940477when its CI moved to cmake, andpatches/mgba.patchexisted only to add a platform block to that file. So the patch is deleted outright and the same compiler flags now go to cmake directly viamgba_MAKE, withmgba_COREpointing into the build dir.Pinned to
e31759b(2026-08-05), verified: cmake-only, exposesBUILD_LIBRETRO/LIBMGBA_ONLY/LIBRETRO_STATIC, and builds targetmgba_libretrowithPREFIX ""and noLIBRARY_OUTPUT_DIRECTORY— so the.solands wheremgba_COREexpects it.This adds cmake as a build dependency for one core. I checked rather than assumed: cmake is installed in the
tg5040-toolchainDockerfile, which also ships an aarch64 cross-compile toolchain file. Core builds run inside that container, not on the runner.fceumm — forwards past the drift
The rewritten patch targets the post-
8ed0cb2Makefile.libretro(the commit that replaced bundled zlib), so it cannot apply to the commit we pinned to avoid it. Pinned tob5e3566, whoseMakefile.libretroblob is the exact pre-image the patch was written against.Verified with #16's own method — fetch the commit,
git apply --checkboth platform patches:becde7dc(old)b5e3566(new)One deviation from upstream
Their
mgba_MAKEends-- $(PROCS), but the shared rule inall/cores/makefilealready appends$(PROCS), expanding to-- -j4 -j4. Dropped ours and kept the--, so-j4still reaches the native build tool rather than becoming a cmake flag (which would add a CMake ≥3.12 requirement). Upstream has the same duplication and may want the same fix.Testing
Dry-ran the makefile expansion on both platforms — pin checkout, the now-skipped patch step, the cmake invocation, and the final
mvall resolve correctly.Not built for ARM here. There's no container runtime in this environment, so the cmake cross-compile itself is unproven on our side; CI is the real gate on that.
Why now
Upstream is mid-transition from GPL-3.0 to PolyForm Noncommercial (LoveRetro/NextUI#765). Their
LICENSEis still GPL-3.0 today, so this is adoptable now; code taken after the transition would not be. Original authorship is preserved in the commit trailer.