ci: compile for FreeBSD before merge, not after (#402); drop the simulated macro probe - #1373
Merged
Merged
Conversation
…o probe The MNT_NODEV break reached main because nothing in the tree compiled the FreeBSD branch of a platform conditional before merge. make ci compiles only the branch matching the host it runs on, the CI matrix is Linux, macOS and Windows, and the single FreeBSD compile that does exist lives in the release workflow: it runs after merge, it was continue-on-error, and publish deliberately excluded it from its gate. So a FreeBSD compile failure produced a green release that shipped without the FreeBSD asset and said nothing. Every pull request now cross-compiles the toolchain for FreeBSD (FREEBSD=1, zig cc against the pinned FreeBSD base sysroot, the same path the release leg uses) and builds and runs the C unit suite inside a native FreeBSD VM. Both are required. The release leg is required too, so a broken FreeBSD build fails the release rather than quietly dropping a platform. Mount options are now assembled from a table of the flags the platform actually defines, replacing the format string that substituted an empty string for a missing flag. A flag the OS does not have is absent from the table, so FreeBSD reports no nodev state rather than reporting it as off. Removes ci-optional-macros, added one release ago to approximate a missing MNT_NODEV by preprocessing the source and recompiling it. Compiling for the real platform supersedes it, and it was the weaker instrument twice over: it depended on a hand-maintained list of file/macro/anchor triples that every future guard had to be added to by hand, and both its awk line-insertion and its hardcoded -std= diverged from the real build before it ever caught anything. make ci is back to 9 steps. Also finishes #1368, which landed without either: fs_is_socket and os_user_id_raw were the only functions in their modules defined with no declaration in the header, and docs/stdlib-reference.md still described the stat kind encoding as 1 through 4 with no mention of fs.fs_is_socket or os.user_id. Verified: 229/229 C unit tests; aether_fs.c compiles -Werror clean both with MNT_NODEV present and with it forced absent, which is the shape that broke; test_std_fs_mounts.ae and test_fs_stat_kind_socket_fifo.ae pass. Closes #402 Closes #1368
A compile break can only be caught by compiling, so the pre-merge check has to be the real cross-compile; there is no cheaper smoke test for it, and it runs in a few minutes. The native VM job is the expensive one, and what it adds over the cross gate is runtime divergence rather than compile coverage, so it moves to push-to-main. Merges here are frequent enough that this is continuous coverage, not a nightly. This does not reintroduce what hid the MNT_NODEV break. That hid because its check was continue-on-error and excluded from the release gate, so failing looked like passing. Both FreeBSD jobs and the release leg hard-fail. Also fixes the VM prepare step: bc ships in FreeBSD base and is not a package, so pkg install bc failed the run outright.
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.
Why the FreeBSD break was invisible
MNT_NODEV(removed in FreeBSD 10, still defined on macOS and OpenBSD) brokethe FreeBSD build after passing every check we have. Three things had to line
up for that, and all three are fixed here.
make cistructurally cannot catch it. It compiles only the branch ofeach platform conditional that matches the host it runs on. On macOS the
__APPLE__ || __FreeBSD__ || __OpenBSD__branch compiles withMNT_NODEVdefined; the FreeBSD spelling of that same branch is never read.
x86_64), Windows. Linux and Windows never compile that branch at all.
fail.
build-freebsdin the release workflow wascontinue-on-error, andpublishdeliberately kept it out of itsifgate. A FreeBSD compilefailure therefore produced a green release that shipped without the
FreeBSD asset, silently. That is the actual root cause: the platform had
a soft-failing leg, so breaking it looked exactly like success.
What changed
FreeBSD is a real CI target (#402), split by what each check can actually catch:
FreeBSD / cross-compile (x86_64)compiles thetoolchain for FreeBSD via
FREEBSD=1(zig cc against the pinned FreeBSDbase sysroot), reusing the path the release leg already uses, and asserts the
output really is a FreeBSD ELF. A compile break can only be caught by
compiling, so there is no cheaper smoke test for this class; it is a few
minutes and fully deterministic. This is what would have failed on the
MNT_NODEVcommit. Toolchain downloads are cached ondeps.lock.FreeBSD / native build + unit testsboots a FreeBSD VM and runs
gmake compiler ae stdlib && gmake test, so theruntime, the kqueue poller and the stdlib are executed on the platform rather
than only type-checked for it. What this adds over the cross gate is runtime
divergence, which is far rarer than a compile break and rarely PR-specific,
and it costs tens of minutes, so it does not sit in front of every PR. Merges
here are frequent enough that this is continuous coverage rather than a
nightly. Scoped to the C unit suite deliberately: the
.aesuites andexamples shell out through the freshly built toolchain and would put a
multi-hour build inside an emulated VM.
Running the heavier job after merge is not what hid the original break.
That hid because its check was
continue-on-errorand excluded from therelease gate, so failing looked exactly like passing. Nothing here is allowed
to fail quietly: both FreeBSD jobs and the release leg hard-fail.
A broken FreeBSD build now fails the release.
build-freebsdlosescontinue-on-errorand joinspublish's gate. Shipping a release with aplatform quietly missing is worse than not shipping it, and it is what hid this.
Mount options are built from the platform's actual flag set. The old code
had a
#ifdef MNT_NODEV / #else nodev_opt = ""fallback, which substitutes adegraded value for a missing flag. Replaced by a table that lists a flag only
where the OS defines it, so FreeBSD (where nodev became a no-op before the
macro was removed) reports no nodev state instead of reporting it as off.
Removes
ci-optional-macros. Added one release ago to simulate an absentMNT_NODEVby preprocessing the source and recompiling it. Compiling for thereal platform supersedes it, and it was the weaker instrument on its own terms:
it depended on a hand-maintained list of file/macro/anchor triples that every
future guard had to be added to by hand, and both its awk line-insertion and
its hardcoded
-std=diverged from the real build before it ever caughtanything.
make ciis back to 9 steps.Finishes #1368. It landed without two things:
fs_is_socketandos_user_id_rawwere the only functions in their modules defined with nodeclaration in the header, and
docs/stdlib-reference.mdstill documented thestat kind encoding as 1 through 4, with no mention of
fs.fs_is_socketoros.user_id(). Both closed; the kind encoding,dir.list_kind, thefs.walkcallback and the
std.osfunction list now describe kinds 5 through 7 and notethey are POSIX-only.
Verification
aether_fs.ccompiles-Werrorclean both withMNT_NODEVpresent andwith it forced absent, which is the exact shape that broke the build.
test_std_fs_mounts.aeandtest_fs_stat_kind_socket_fifo.aepass.make ci,make ci-portabilityand.PHONYverified intact after the target removal, with no dangling references.
Not in this PR
#1366 (entry-file top-level functions emit bare non-static C symbols) is a real
bug but not a minor one. Fixing it properly means changing symbol linkage at
three emit sites in lockstep, and a blanket change breaks
--emit=lib,--emit=bothand--emit=csrcconsumers, whose non-ABI-representablefunctions are a documented part of the exported surface, plus
extra_sourcesCshims that reference an Aether function by name. It needs its own PR with its
own test matrix rather than riding along on a portability change.
Closes #402
Closes #1368