fix: P6 and P8, plus the Android NDK r29 and ocr-review CI breakages - #206
Merged
Merged
Conversation
Four independent fixes, grouped because they are all release blockers for v2026.09.12. P6 -- DB_BACKUP_NO_LOGS was accepted and silently ignored The flag appeared exactly twice in the tree: its #define and db_backup.c's accepted-flag mask. It was read NOWHERE, so a backup taken with it copied every log file. Documented behaviour is "back up only the *.db files, do not backup the log files" (docs_src/api/c/envbackup.md). Fixed by guarding the backup_read_log_dir call. Checked the interaction rather than assuming: copy_min stays 0 when logs are skipped, and the DB_BACKUP_UPDATE consistency test is `remove_max < copy_min`, which is never true at 0, so NO_LOGS+UPDATE does not false-trigger. Measured after: 2 db files and 0 log files, against a control backup with 62. P8 -- DB_NOFLUSH made an environment unusable LAST_PANIC_CHECK_BEFORE_IO tested the PUBLIC DB_ENV_NOFLUSH and expanded to an unconditional `return (0)' inside __os_physwrite's write loop and both arms of __os_io. So under DB_NOFLUSH every read and write in the library reported success and moved no data: SIGBUS in __env_alloc_init with a zero-length region file on a shared environment, DB_PAGE_NOTFOUND on the first DB->open on a private one. Present verbatim since Oracle 5.1.29 -- 15 years. The clause could NOT simply be deleted. env_open.c sets NOPANIC|NOFLUSH during panic teardown, and NOPANIC makes PANIC_ISSET false, so PANIC_CHECK alone would not stop the I/O on that path -- the NOFLUSH test is what does. Two unrelated meanings were sharing one bit. Split them: panic teardown now sets a new internal-only DB_ENV_NOIO (0x00400000, a process-local dbenv->flags bit, so no region-layout and no ABI consequence -- the three-gate question), and the macro tests that. DB_ENV_NOFLUSH keeps its one real meaning, "do not flush the cache on close", which was already correctly implemented at env_open.c:844. Perl's constants.h still resolves DB_ENV_NOFLUSH to the same value, so the bindings are unaffected. Retiring the XFAILs, and replacing the teeth they provided All three of P6/P7/P8 flipped from XFAIL to PASS with NO EDIT to their tests, which is what the XFAIL mechanism was for. Their XFAIL branches are now FAIL branches: with the defects fixed, the old signature means a regression. That removes the tier's must-fail teeth -- FLAGAPI_STRICT=1 now passes, so the step asserting it must FAIL had nothing left to fail on, which is precisely the vacuous-green shape this project has nine recorded instances of. Replaced with test/c/flagapi-sabotage.sh: it removes the DB_BACKUP_NO_LOGS guard (which IS defect P6), rebuilds, and requires backup_nologs specifically to FAIL, restoring the tree on every exit path via a trap. Verified in both directions -- clean tree 29/29 PASS, sabotaged tree caught with "backup_nologs: FAIL". Android cross-build on NDK r29 The per-API wrapper (aarch64-linux-android24-clang) is a shell script that execs a sibling `clang'. NDK r29 ships the wrapper but not that sibling, so the wrapper existed, was executable, and died with "bin/clang: No such file or directory" -- which meson surfaced only as "Unknown compiler(s)". The existing `[ -x "$CC" ]' guard passed, because the file was there; testing -x is not enough, the compiler has to be RUN. Now probes with --version and falls back to the real clang with an explicit --target, which is all the wrapper was doing. The --target goes in meson's [binaries] list rather than c_args so it also applies to meson's own sanity check. Also verifies llvm-ar/strip/ranlib and prints the bin/ listing on failure instead of failing obscurely. ocr-review The repo variable was created as OCR_BEDROCK_VARIABLE while all eight references read OCR_BEDROCK_MODEL, so the model string arrived empty and the guard tripped on every run. Renamed the variable (config, not code). The second half of that failure is NOT fixable from the repository and is now documented in the workflow header: vars.AWS_ROLE_ARN points into AWS account 492372116879, which is no longer active, so the OIDC credential step cannot mint credentials until it is repointed. Both ocr-review and the Android job are continue-on-error/advisory, so neither blocks a merge.
Contributor
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Contributor
ABI diff vs
|
… +x the sabotage script Two gates caught two real defects in the previous commit, both in the "generated files that are committed" class: 1. dist/s_execbits found test/c/flagapi-sabotage.sh at mode 100644. The local chmod +x never reached git's index, so the script would have been unrunnable for everyone else -- and since it IS the flag tier's teeth, the tier would have lost its must-fail arm silently. Fixed with git update-index --chmod=+x. 2. windows msbuild failed with "DB_ENV_NOIO: undeclared identifier" in five os_windows translation units. build_windows/db.h and build_android/db.h are COMMITTED generated files, so adding a flag to src/dbinc/db.in does not reach them until s_windows/s_android are run. Regenerated both. Regenerating also swept up pre-existing drift in the Android headers, unrelated to this change and now correct: clib_port.h still carried WinCE comments, and db_config.h and db_int.h still referenced VxWorks (HAVE_MUTEX_VXWORKS, selectLib.h) long after those ports were removed. The committed copies had not been regenerated since. build_windows/db.h moved by exactly 8 lines and none of the five CRLF phantom files appear, so the skip-worktree set is still doing its job.
…st tag
The previous commit fixed the wrong half. It handled "the per-API clang wrapper
exists but cannot exec its sibling clang", which was the visible symptom on r29 --
but the diagnostic it added then reported bin/ as EMPTY, which means the
hard-coded host tag was wrong and every path built from it pointed at nothing.
linux-x86_64 is simply not what r29 ships under prebuilt/.
So the assumption itself was the defect, in both places it appears:
- the host tag: now tries the known tags (linux-x86_64, linux-aarch64 and the
Darwin pair), takes the first that actually HAS a bin/, and failing that
finds any executable clang anywhere under the NDK root.
- the compiler: unchanged from the previous commit -- probe with --version and
fall back to bare clang plus an explicit --target, because testing -x cannot
tell a working wrapper from one whose sibling is missing.
Both fallbacks were verified against constructed NDK trees rather than only in
CI: a tree with the unusual host tag "linux-musl-x86_64" and no per-API wrapper
at all is driven correctly ("using .../clang --target=aarch64-linux-android24"),
and a tree with an empty prebuilt/ now fails naming the real problem and listing
what it did find.
That second check caught a bug in this commit's own error path: `dirname' of an
empty find result yields ".", so the failure message listed the repository
directory as the NDK's bin/. Guarded.
…lang-NN
Third pass, and the previous two were each fixing a symptom one layer out.
What r29 actually ships, read off the failure output rather than guessed: bin/
contains the per-API wrappers for EVERY api level (aarch64-linux-android21-clang
through -android30-clang, plus the ++ forms) and NO bare clang at all. The
wrappers exec "$(dirname $0)/clang", which therefore does not exist, so each one
dies with "bin/clang: No such file or directory" -- and meson surfaces that only
as "Unknown compiler(s)".
The previous commit's fallback looked for exactly "$BIN/clang", which is the one
name r29 does not provide, so it reported "no runnable NDK compiler" while the
compiler was sitting right there under a versioned name.
Now: probe the wrapper by RUNNING it; if that fails, try $BIN/clang, then
$BIN/clang-NN newest-first, then any executable clang or clang-NN anywhere under
the NDK root, accepting the first that answers --version. Drive whatever is found
with an explicit --target, which is all the wrapper was doing. The --target is
passed via meson's [binaries] list rather than c_args so it also covers meson's
own compiler sanity check.
Verified against a constructed NDK in exactly r29's shape -- wrappers present,
no bare clang, real driver as clang-21 -- which now resolves correctly:
note: ...aarch64-linux-android24-clang is not runnable;
using .../clang-21 --target=aarch64-linux-android24
and against the same tree with clang-21 removed, where the diagnostic now prints
WHY the wrapper failed (the exec error) plus the non-wrapper contents of bin/,
instead of a bare list truncated to wrappers.
The lesson worth keeping: "file exists and is executable" says nothing about a
wrapper script, and the first two attempts here both trusted a path I had not
run.
…iler Fourth pass. The compiler now resolves on r29 -- the note fires and clang-NN is found -- and the next hardcoded path immediately failed: llvm-ar, llvm-strip and llvm-ranlib were still addressed as "$BIN/<tool>", and r29 does not put them there either. The pattern was the defect, not any one path. Each of these four commits fixed a single assumed location while leaving its siblings assumed, so every pass moved the failure one line down. All tool lookups now go through one find_tool helper: $BIN/<tool>, then $BIN/<tool>-NN newest-first, then anywhere under the NDK root, then the un-prefixed name (llvm-ar and ar are interchangeable on a clang toolchain). The resolved values flow into the meson cross file and the strip invocation instead of being re-derived from $BIN. Verified against a constructed tree in r29's shape with the llvm tools deliberately moved OUT of bin/ into a sibling prebuilt path: both the compiler (clang-21 plus --target) and the relocated tools resolve. Whether this is the last relocation I cannot claim -- I have been wrong about that three times in a row, and the honest statement is that the script no longer hardcodes any toolchain path, so the next layout change should be absorbed rather than needing a fifth commit. The job is continue-on-error, so it has never blocked the release.
Fifth pass, and this one removes the requirement rather than chasing the path.
The r29 failure was "NDK tool not found: llvm-strip" and "llvm-ranlib" while
llvm-ar resolved fine. Two causes, both mine:
- the fallback searched with `find -type f', and llvm-strip and llvm-ranlib are
SYMLINKS (to llvm-objcopy and llvm-ar) in some NDK layouts, so a -type f
search skips them entirely while llvm-ar, a real file, is found.
- more importantly, they were treated as required at all. meson builds a shared
library perfectly well with no strip or ranlib entry in the cross file, and
this script's own strip step was already best-effort with `|| true'. Being
strict about two optional tools is what turned a layout change into a hard
error.
So: find_tool now accepts -type f OR -type l and also tries the un-prefixed name
(llvm-ar and ar are interchangeable here), and only the ARCHIVER is required.
Missing strip/ranlib print a note and are OMITTED from the cross file rather than
emitted as empty paths that meson would try to exec. The strip step is guarded.
Verified against two constructed trees, checking the generated cross file both
times rather than only the exit status:
bin/ = clang + llvm-ar only -> two notes, build proceeds, cross file has
c/cpp/ar and no strip or ranlib lines
bin/ = clang + all three tools -> strip and ranlib present, and correctly
under [binaries], not [host_machine]
That last check is why the insertion uses sed against the `ar =' line instead of
appending to the file, which would have put them in the wrong section.
The compiler and the archiver now resolve on r29, and the failure moved to the link step: ERROR: Unable to detect linker for compiler `.../clang-21 --target=... -Wl,--version` error: unable to execute command: Executable "ld.lld" doesn't exist! ld.lld IS present in that same bin/ -- it appeared in the directory listing an earlier pass printed. The problem is a consequence of bypassing the per-API wrapper: clang locates ld.lld relative to what it considers its install root, and clang-21 invoked by absolute path does not treat that bin/ as one. The wrapper had been supplying this implicitly. -B<dir> names the tool directory explicitly, so it goes into the [binaries] entry alongside --target. Verified on a constructed r29-shaped tree by reading the generated cross file, not just the exit status: c = ['.../bin/clang-21', '--target=aarch64-linux-android24', '-B.../bin'] Each pass here has been the same mistake in a new place: replacing one thing the wrapper did without asking what else it did. --target, the tool lookups, and now -B are all things it was doing for us.
…er validated Stepping back after five passes at r29's layout. The thing I should have checked first: this job has failed on EVERY run since it was added -- 18 consecutive failures across three days -- and continue-on-error reported the workflow as "success" throughout, so nothing surfaced it. The pin's own comment said r29 "matches the nixpkgs ndk-bundle this path was validated against", and that was simply not true: there is no run in which it worked. So the Android cross-build was never green, and I have been treating a never-validated pin as a regression. r29 ships the per-API wrappers (aarch64-linux-androidNN-clang for every level) but NOT the sibling `clang' those wrappers exec, and its ld.lld is not where a directly-invoked clang-NN looks for it. Bypassing the wrapper therefore trades one missing tool for the next, which is exactly what each pass found: clang, then llvm-ar, then llvm-strip/ranlib, then ld.lld. Pinned r27c, an LTS release with the conventional layout. The discovery work in build_android.sh is kept -- it is what makes the script survive a layout change instead of hard-coding five paths -- but the pin is now a version whose layout is known good, so the job can actually prove the cross-build rather than exercise fallbacks. If r27c goes green, the job should lose continue-on-error in a follow-up, because an advisory gate that has never once passed is not advisory, it is decorative.
…ailing ocr-review has been failing on every PR. Half of that was the variable-name mismatch fixed earlier (OCR_BEDROCK_VARIABLE vs OCR_BEDROCK_MODEL); the other half cannot be fixed from the repository, because vars.AWS_ROLE_ARN points into AWS account 492372116879, which is no longer active, so the OIDC step cannot mint credentials. Until that is repointed, the honest behaviour is to SKIP, not to fail. A guard step checks AWS_ROLE_ARN, AWS_REGION and OCR_BEDROCK_MODEL, emits a ::notice naming whichever are missing, and every subsequent step is conditioned on it. The reason to bother, given the job is already continue-on-error: an advisory job that fails on every single PR trains everyone to ignore it, and then it is not advisory, it is noise that hides the next real failure. This is the same shape as the Android job, which turned out to have never passed in 18 runs while the workflow reported success. No pre-existing `if:' conditions were replaced -- verified the diff removes no lines.
…link
r27c went green, then went red on the very next run with the SAME pin. That can
only be the environment, and it was: local-cache: true restores a partially
populated NDK, the per-API wrapper becomes unrunnable, and build_android.sh's
fallback then quietly resolves clang-18 directly and fails much later at the
linker with "Unable to detect linker for compiler".
Two changes, because the caching was the cause but the fallback is what made it
confusing:
- local-cache: false. A fresh download costs a couple of minutes and makes a
green run mean the toolchain is actually complete.
- a link probe before meson runs: compile and link a trivial shared object with
the resolved compiler, and fail immediately if that does not work. meson
reports a missing ld.lld as a problem with the COMPILER, which sends you to
the wrong place; the probe says the linker is absent and, when it sees ld.lld
in the error, names the cache as the likely cause. It also stops a
fallback-resolved clang from being accepted when it cannot produce a binary.
The fallback now also announces that a working NDK should not need it, so its
appearance in a log reads as a symptom rather than as normal operation.
Probe verified both ways against constructed toolchains: one whose clang answers
--version but fails to link reports "the resolved toolchain cannot link" plus the
ld.lld hint, and one that links cleanly prints "== link probe: ok".
…is unset The config check added in the previous commit was necessary but not sufficient. vars.AWS_ROLE_ARN is present and well-formed, so the guard passed, and the job still failed -- on the assume itself: Could not assume role with OIDC: Not authorized to perform sts:AssumeRoleWithWebIdentity That is the dead account (492372116879) and it is not detectable by inspecting configuration; it only shows up on attempting the call. So the credentials step is now continue-on-error with an id, a following step emits a ::notice explaining what to repoint, and each downstream step additionally requires steps.aws.outcome == 'success'. Result: with no Bedrock config the job skips, with config but a dead role it skips with a note naming the fix, and with a live role it reviews as before. It stops being a red mark on every pull request for a reason no contributor can act on.
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
1 participant
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.
Four release blockers for v2026.09.12.
P6 —
DB_BACKUP_NO_LOGSaccepted and silently ignoredThe flag appeared exactly twice in the tree (its
#defineanddb_backup.c's accepted-flag mask) and was read nowhere, so a backup taken with it copied every log file. Fixed by guarding thebackup_read_log_dircall.Checked the interaction rather than assuming:
copy_minstays 0 when logs are skipped and theDB_BACKUP_UPDATEtest isremove_max < copy_min, never true at 0.Measured after: 2 db files and 0 log files, against a control backup with 62.
P8 —
DB_NOFLUSHmade an environment unusableLAST_PANIC_CHECK_BEFORE_IOtested the publicDB_ENV_NOFLUSHand expanded to an unconditionalreturn (0)inside__os_physwrite's write loop and both arms of__os_io. Every read and write reported success and moved no data — SIGBUS with a zero-length region file (shared),DB_PAGE_NOTFOUND(private). Present verbatim since Oracle 5.1.29.It could not simply be deleted:
env_open.csetsNOPANIC|NOFLUSHduring panic teardown, andNOPANICmakesPANIC_ISSETfalse — so theNOFLUSHtest is what actually stops the I/O there. Two unrelated meanings shared one bit.Split them: teardown now sets internal-only
DB_ENV_NOIO(0x00400000, a process-localdbenv->flagsbit — no region-layout, no ABI consequence).DB_ENV_NOFLUSHkeeps its one real meaning, already implemented atenv_open.c:844.Retiring the XFAILs, and replacing the teeth they provided
All three of P6/P7/P8 flipped XFAIL→PASS with no edit to their tests — what the mechanism was for. Their XFAIL branches are now FAIL branches.
That removes the tier's must-fail teeth:
FLAGAPI_STRICT=1now passes, so the step asserting it must fail had nothing left to fail on — the vacuous-green shape with nine recorded instances here. Replaced withtest/c/flagapi-sabotage.sh, which removes theDB_BACKUP_NO_LOGSguard (that is P6), rebuilds, and requiresbackup_nologsspecifically to FAIL, restoring the tree via a trap on every exit path.backup_nologs: FAILAndroid cross-build on NDK r29
The
aarch64-linux-android24-clangwrapper is a shell script that execs a siblingclang. r29 ships the wrapper but not the sibling, so it existed, was executable, and died withbin/clang: No such file or directory— which meson surfaced only asUnknown compiler(s). The[ -x "$CC" ]guard passed because the file was there; testing-xis not enough, the compiler has to be run.Now probes with
--versionand falls back to the realclangwith an explicit--target. The flag goes in meson's[binaries]list, notc_args, so it applies to meson's own sanity check too.ocr-review
The repo variable was created as
OCR_BEDROCK_VARIABLEwhile all eight references readOCR_BEDROCK_MODEL— the model string arrived empty. Renamed (config, not code).The second half is not fixable from the repository and is now documented in the workflow header:
vars.AWS_ROLE_ARNpoints into AWS account492372116879, which is no longer active, so the OIDC step cannot mint credentials until it's repointed. Both this job and Android arecontinue-on-error.