Check for mk and limbo before the macOS builds - #564
Conversation
d204ad6 to
f629ae9
Compare
pdfinn
left a comment
There was a problem hiding this comment.
The premise is right — "mk present, limbo absent" is exactly the state a
half-finished bootstrap leaves, and dying minutes into the emulator compile
with limbo: command not found is a bad way to learn that. Two problems
though, and the first means the message sends people down a path that will not
work.
The bootstrap instructions are incomplete
Run ./makemk.sh, then build libmath, libbio, lib9, and libsec in order before
rebuilding limbo.
limbo/mkfile declares:
LIBS= bio
math
sec
mp
9
YFILES= limbo.y
So libmp is missing from the list, and limbo.y means utils/iyacc has to
be built first as well — Inferno's yacc, not the host's. Following the message
literally gets you to a failure a step later than before, which is worse than
no advice.
I built limbo from C on a clean tree last week while working on #559; the
order that works is:
SYSTARG=$(uname -s) OBJTYPE=<arch> ./makemk.sh
for d in lib9 libbio libmp libsec libmath utils/iyacc limbo; do
(cd $d && mk install)
donelib9 first rather than third, since everything else builds against it. That
whole sequence takes about 30 seconds, which is worth saying in the message —
people put off bootstraps that sound expensive.
The test fails on the machines the code is for
expect_refusal runs env PATH="$TMP" "$ROOT/$script", but the first thing
both scripts do is
export PATH="$ROOT/MacOSX/arm64/bin:$PATH"so on any macOS box where the native tools are actually built — which is every
machine that has run these scripts before — mk and limbo are found, the
preflight check does not fire, the build proceeds, and expect_refusal reports
build-macos-headless.sh: unexpectedly succeeded without mk limbo
It passes here on Linux arm64 precisely because MacOSX/arm64/bin does not
exist:
build_macos_preflight: PASS
So the coverage is inverted: green where the code never runs, red on the
platform it was written for. That is worse than #565's problem, because it is a
false failure rather than a false pass — someone will hit it locally and
conclude the test is broken.
Neutralising $TMP is not enough while the script re-adds the real bin
directory. Simplest fix is to move $ROOT/MacOSX/arm64/bin/{mk,limbo} aside
for the duration and restore them in the existing trap, so the script's own
PATH line finds nothing either. Copying the tree would also work but costs
more.
The check itself
The loop and the message shape are good, and doing both scripts is right — the
hole is identical and build-macos-sdl3.sh is the one AGENTS.md lists first.
One note: command -v is the correct POSIX spelling, but both scripts are
#!/bin/bash and use [[ ]], so they are bash already. Not a problem, just
inconsistent with CONTRIBUTING.md:226 in the same way the rest of these
scripts are — not something to fix here.
Fix the library list and the test's PATH handling and this is good to go.
f629ae9 to
4310307
Compare
|
Both fixed, and thank you for the second one — the test really was inverted. The bootstrap recipe. Corrected to
The test. Rather than move the real Verified on macOS with real Left the |
Both scripts assumed the native tools were already on PATH. makemk.sh builds mk alone, so a half-finished bootstrap left limbo missing and the build died several minutes in with a bare "limbo: command not found". Refuse up front when either is absent, and name the bootstrap.
…macOS
The refusal message named libmath, libbio, lib9 and libsec, but
limbo/mkfile declares LIBS="bio math sec mp 9" and YFILES=limbo.y, so
libmp was missing and Inferno's own iyacc had to be built first. The
quoted sequence now names lib9 first, then libbio, libmp, libsec,
libmath, utils/iyacc and limbo, sets ROOT and PATH, and says how long
the bootstrap takes. Verified verbatim from a clean tree on macOS
arm64; SYSTARG is pinned to MacOSX because mkfiles/ has no Darwin
entries.
The preflight test ran the scripts with an empty PATH, but both scripts
prepend $ROOT/MacOSX/arm64/bin before checking, so on any machine
where the native tools are built the tools were found, the preflight
never fired, and the test failed on the platform it targets. Move
$ROOT/MacOSX/arm64/bin/{mk,limbo} aside for the duration and restore
them in the trap, so the script's own PATH line finds nothing either.
Moving the real MacOSX/arm64/bin tools aside for the duration of the test had a signal window no trap can close: SIGKILL (CI timeout, OOM) between stash and restore left the tree without its native build tools and nothing to notice with. The test now copies the two build scripts into a fake tree with an empty bin directory and runs them from there; both scripts derive ROOT from $0, so nothing in the real tree is ever touched. Also pin the two recipe lines the refusal message must keep (SYSTARG=MacOSX and the ROOT/PATH export), which an earlier review proved are the necessary deviations from the naive spelling.
4310307 to
5a08c85
Compare
|
Rebased onto current master (df34b02). Force-pushed 4310307 -> 5a08c85.
Re-verified on macOS after the rebase.
#578 fixed the ClusterFuzzLite link break, so |
What this changes
build-macos-headless.shandbuild-macos-sdl3.shassumedmkandlimbowere already on PATH. Neither checked, so a missing tool surfaced as a bare
command not foundfrom inside the build rather than as a statement of what todo about it.
makemk.shbuildsmkalone.limbostill needs libmath, libbio, lib9 andlibsec built after that, so "mk present, limbo absent" is the state a
half-finished bootstrap leaves behind, and it is the one that costs time: the
build gets several minutes in, through the whole emulator compile, before it
reaches
limboand dies.Both scripts now refuse up front when either tool is missing, name which one,
and name the bootstrap.
Both, because the hole is identical in each and
build-macos-sdl3.shis theone AGENTS.md lists first.
Tests
tests/host/build_macos_preflight_test.shis new. For each script it coversneither tool present, and
mkpresent withlimboabsent. It stubspkg-configso the SDL3 probe cannot decide the result, and a failed assertionprints what it expected and what it got.
Against each script before this change:
After:
The exit status was already correct in both.
set -eis set and each ends in anexplicit
exit 1; a failing step has always returned non-zero. Only thepreflight was missing.