Repository navigation
ci: gate the preloader so a green run means the library is sound - #37
Merged
Merged
Conversation
There was no CI in this repository, so nothing checked that the tree
still builds, that the host suite still passes, or that the sources
listed in CMakeLists are the ones on disk.
Four jobs, each a real check rather than a smoke test:
host-tests the 57-test C++ suite, including the three
ThreadSanitizer tests, which must actually run
(setarch is installed so they cannot silently skip)
rust cargo fmt --check, clippy with the correctness group
as errors, and the 249-test workspace suite, on a
pinned toolchain so the gates cannot move underneath
the repository
native-build libsweetloader.so and the three Rust cdylibs
cross-compiled for arm64-v8a on the NDK the client
pins, then checked for its JNI and SDK exports and
for static-archive symbols leaking into the dynamic
table
repo-integrity no .cpp missing from SWEETLOADER_SOURCES, no host
test unregistered, every fetched dependency pinned
to a commit and attributed in NOTICE
The drift checks are scripts rather than inline shell so they can be
run by hand and tested. gen_rules.py had no test at all despite being
what turns a game update into a hook address: its test builds a
synthetic stripped-binary-shaped ELF so the .data.rel.ro relocation
step is exercised without a real APK, and disabling that step fails
three of the eight tests.
Three jobs failed on the first run, all of them in the CI rather than in the library. Each failure was a real defect in a gate: 1. native-build reported "missing exported symbol: GetSweetLoaderInput" for a symbol that is present. Under `set -o pipefail` a `nm ... | grep -q` pipeline reports the producer's SIGPIPE as the pipeline status, because grep -q exits on its first match and closes the pipe. The check passed locally only because I ran it without pipefail. The symbol tables are now dumped to files once and every check greps a file. 2. host-tests failed with "mkdir: cannot create directory '.../../../../../build': Permission denied". run_tests.sh cached its fetched nlohmann/json and fmt headers four levels above its own root, which is correct for the submodule inside the client and escapes the checkout entirely for a standalone clone. The cache now sits beside the checkout and honours SWEETLOADER_HOST_CACHE; CI points it at the runner's temp dir. 3. rust failed on `cargo fmt --check`, and it was right to: the tree had never been formatted, so 157 sites in 30 files differed. No rustfmt.toml gets it to zero (max_width 100/120 and the use_small_heuristics variants all leave >120 sites), so the tree is normalized instead, in this commit, with the tests and clippy re-run green afterwards. The gate is what keeps it that way. Also switched the cdylib existence checks to `test -f` for the same reason as (1), and set an explicit cache dir for the host job.
The previous commit put SWEETLOADER_HOST_CACHE in the job's env:, where the runner context is not in scope. GitHub rejected the whole workflow file, so the run reported failure with zero jobs and no annotation to read. actionlint names it precisely: ci.yml:35:35: context "runner" is not allowed here. available contexts are github, inputs, matrix, needs, secrets, strategy, vars The variable moves to the step that needs it, where runner.temp is in scope. actionlint is clean on the file now.
`ls -d /opt/android-sdk/ndk/*/... | head -1` is the same shape as the SIGPIPE bug just fixed in the workflow: when two NDK versions are installed, ls keeps writing after head has exited and the pipeline status is the SIGPIPE. It does not reproduce with one NDK, so it would have surfaced on the first machine with two. `find -print -quit` writes one line and stops on its own. shellcheck is clean on the script now.
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
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 changed
The preloader had no CI at all, so nothing checked that the tree still builds, that the host
suite still passes, or that the sources listed in
CMakeLists.txtare the ones on disk. Thisadds four jobs, each a real check rather than a smoke test. A green run is the claim that the
library a client loads is the library the sources describe.
host-testsrustcargo fmt --check, clippy with the correctness group as errors, and the 249-test workspace suite.native-buildlibsweetloader.soand the three Rust cdylibs cross-compile for arm64-v8a, and the built.soexports the JNI and SDK symbols with no static-archive leakage.repo-integrity.cppmissing fromSWEETLOADER_SOURCES, no host test unregistered, every fetched dependency pinned to a commit and attributed inNOTICE.The first run failed, and it was worth having
The first run went red on three of the four jobs. Every failure was a defect in the gate itself,
which is the failure mode a gate exists to prevent, so each is worth naming:
native-buildclaimed a present symbol was missing. The check wasllvm-nm -D --defined-only libsweetloader.so | grep -q " GetSweetLoaderInput$". Underset -o pipefail,grep -qexits on its first match and closes the pipe, the producer takesSIGPIPE, and the pipeline reports that instead of the match.
GetSweetLoaderInputis in thedynamic table; the check said it was not. Reproduced directly:
The symbol tables are now written to files once and every check greps a file.
host-testscould not create its cache.run_tests.shcached its fetchednlohmann/jsonand
fmtheaders four levels above its own root:That path is correct for this tree as the client's submodule and escapes the checkout entirely
for a standalone clone, which is what CI has. The cache now sits beside the checkout and honours
SWEETLOADER_HOST_CACHE, which the job points at the runner's temp dir.rustwas right: the tree was not formatted.cargo fmt --checkreported 157 sites across30 files, because the Rust tree had never been through rustfmt. No configuration rescues it
(
max_width100 or 120, with and withoutuse_small_heuristics = "Max", all leave more than 120sites), so the tree is normalized in this PR, with the test suite and clippy re-run green after.
A fourth fix was needed after the tree was normalized.
SWEETLOADER_HOST_CACHEwas firstplaced in the job's
env:, where therunnercontext is not in scope. GitHub rejected the wholefile, so the run reported failure with zero jobs and no annotation.
actionlintnames it:context "runner" is not allowed here. The variable moved to the step, where it is in scope.Why each remaining gate is shaped the way it is
The sanitizer tests must actually run.
run_tests.shskips a ThreadSanitizer test whensetarchis unavailable, and a skip reads as a pass. The runner has ASLR on and TSAN refuses tostart without
setarch -R, soutil-linuxis installed and the job fails if the output containsskipped: ThreadSanitizer.The Rust toolchain is pinned to
1.100.0, sorustfmtandclippycannot move the gatesunder the repository without a commit.
Only clippy's correctness group is an error. The workspace carries ~30 style lints, which are
advisory; the correctness group catches a real defect rather than a preference.
The drift checks are scripts, not inline shell
check_sources_listed.pycatches the two failures that are otherwise silent: a new.cppneveradded to
SWEETLOADER_SOURCESis simply not compiled, and a new host test never registered neverruns.
gen_rules.pyhad no test at allIt is what turns a Minecraft build into a hook address, and its premise is that
.data.rel.roholds pre-relocation zeros offline, so the typeinfo and vtable pointers must be rebuilt from
.rela.dynbefore anything is read. The new test builds a synthetic stripped-binary-shaped ELFwith the same section shape as the game binary and checks the derivation end to end.
Mutation-checked: skipping the relocation step fails 3 of the 8 tests.
Verification
The final commit is green on all four jobs, on GitHub's runners:
host-testsrustnative-buildrepo-integrityactionlint1.7.7 with shellcheck 0.10.0 is clean on the workflow, and shellcheck is clean ontests/cosmetics/run_tests.sh.One residual risk worth stating: the gates assert the library exports the symbols the headers
declare, but nothing on a runner loads it into a JVM or an arm64 device. That needs a device or
an emulator, and is out of scope here.