mirror of
https://github.com/Klipper4FlashForge/firmware.git
synced 2026-10-21 21:02:16 -05:00
firmwareExe ran $MODDIR/init.d/S* one at a time in the foreground, and that
loop WAS the boot order. Three of the seven were already thin wrappers over a
supervisor that was already running, so what the loop still carried was not
supervision, it was sequencing -- expressed as filenames, in a directory
nothing but the loop read.
This ships s6-rc and moves the sequencing into a database compiled at build
time. firmwareExe now starts the scanner as its own child, initialises s6-rc,
asks for one machine state, and blocks on `wait "$SCANNER_PID"`.
WHAT THE GRAPH ACTUALLY SAYS
mcu-bringup -> klipper -> ff-startup -> ui
^
moonraker
ok-all = wifi nginx moonraker camera klipper ff-startup ui
Those are the first real edges the plan has ever had; it predicted they would
not arrive until phase 7. bin/ff-startup.py became two services, which is what
made them possible: the MCU bring-up has to precede klipper and the readiness
wait has to follow it, and both facts now live where a program can read them.
Deliberately still absent: nginx before moonraker. S60nginx's header wants it,
but dependencies.d/nginx under moonraker means `s6-rc -d change nginx` stops
moonraker too -- the exact failure splitting S60web into two scripts prevented.
The cost of not writing it is a second where a reloading browser gets a refused
connection instead of a 502.
AND WHAT THEY DO NOT SAY. klipper has no notification-fd, so s6-rc calls it up
when it is forked, not when klippy is usable. ff-startup keeps its own poll of
moonraker's API. moonraker and camera DO carry one, so their readiness is real.
The asymmetry is written down at each site rather than glossed.
init.d/ IS DELETED, REVERSING PHASE 4
Phase 4 promised the S* scripts would survive as thin wrappers, because
`S60nginx restart` is in people's hands. They did not survive, and the reason
is the argument nginx/run already made: a service that only works when a shell
script happened to run first is not supervised, it is lucky. Every prerequisite
a wrapper did belongs in the run script anyway, because s6 restarts a service
without going near a shell. Once they moved, the wrappers were verb translation
in a directory whose S<NN> numbers meant nothing. s6-rc is the interface now.
anvil-service.sh goes with them -- 625 lines whose last caller was firmwareExe,
and a shared library for one consumer is the redundancy the wrappers were.
So do the fallbacks. S60nginx and S62moonraker each carried a second,
unsupervised implementation reachable only via MOD_S6=0; that switch is gone,
and with it the argument for a code path exercised only on broken printers. The
cost is stated plainly in anvil.conf: a printer whose s6 fails now has nothing.
klipperDaemon is replaced with a no-op `start`. It is FlashForge's script and
off-repo callers remain -- stock start.sh, the UI's restart path, habits over
ssh -- and each of them would fork a second klippy beside the supervised one.
WHAT SUPERVISES THE SUPERVISOR, ANSWERED
Phase 3 left this open and guessed a dead scanner was a dead printer either
way. It is not. firmwareExe's tail used to be `while true; do sleep 3600; done`
purely to satisfy app_startup.sh's watchdog, which greps ps for our name 5s in.
It waits on the scanner instead, so exiting IS the recovery: app_startup.sh --
FlashForge's own init, which we never patch -- re-execs us and the tree comes
back. Not a respawn loop in ash, which phase 3 rightly refused. It restarts
everything rather than the scanner alone, and that is written down.
MEASURED, NOT READ OFF THE MANUAL
Host- and target-compiled databases are byte-identical (db md5
80f5261437191fa591744ada6087c6f7 both ways), so the database is built here and
shipped. /run is tmpfs and writable, so the live directory needs no fallback.
Both were BLOCKING UNKNOWNs in the note and are now settled sections.
Three traps the replica found that reading could not:
* s6-svscan resolves s6-supervise through PATH. Without $MODDIR/bin on it the
scanner answers s6-svscanctl -a perfectly and then fails every spawn with
ENOENT for ever, and s6-rc-init HANGS rather than failing.
* s6-rc-init creates one symlink per service in the scandir and dies with
"File exists" if the name is taken. So the cutover is not additive, and
run-append.sh sweeps.
* s6-rc-compile does not reject a `down` file. It accepts it and SILENTLY
DROPS it -- the servicedir is byte-identical. "Down by default" can only
mean absent from the bundle, which is why mcu-bringup is the one service
outside ok-all and arrives as klipper's dependency.
execline turned out not to be optional: s6-rc GENERATES two servicedirs whose
run scripts are execline, and every oneshot's `up` goes through one of them.
The binary list was read off a real compile, not guessed -- seven execline
binaries of 53, plus six s6 ones we built and did not ship.
Two build bugs fell out. fetch-assets.sh compared against a stamp patch.sh
never wrote, so every build pulled the ~100MB toolchain the skip exists to
avoid; S6_STAMP is one string in common.sh now. And `chmod +x .../bin/s6-*`
would have missed all seven execline binaries.
qa: 83 static. The suite gained the s6-rc run scripts and klipperDaemon, which
no glob had ever covered, and lost a dead payload/init.d/S* glob -- exactly the
silently-retired check that file's own docstring warns about. The oneshots'
up/down are NOT in it: they are execline carrying shell inside a /bin/sh -c
argument, so both checkers would parse the wrapper and pass.
Not verified here: no config.env and no stock package on this machine, so the
mipsel cross-build of execline/s6-rc and the ABI gate over 33 binaries have
never run. The native compile and the database are proven; that half is not.
The replica gate is the next commit, and it still owes an answer on whether
helix-screen runs under s6-supervise at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
262 lines
12 KiB
Python
262 lines
12 KiB
Python
"""Every script parses, and none of them is secretly bash.
|
|
|
|
Ported from test/run-tests.py's check_shell_syntax and check_no_bashisms. The
|
|
logic is theirs; what changes is the granularity. There, one gate covered ~25
|
|
files and reported one bit, so a syntax error in S62moonraker and a syntax
|
|
error in patch.sh were the same red line. Here each file is its own test, named
|
|
after itself, and `pytest -k S62moonraker` runs exactly one.
|
|
|
|
The original's docstring made a deliberate choice worth preserving:
|
|
|
|
One line per check, not one per file: 25 green lines saying "syntax ok"
|
|
hide the two that matter.
|
|
|
|
That reasoning was about output noise, and it was right. pytest solves it a
|
|
different way -- passes collapse to dots, and only failures print -- so the
|
|
granularity and the quiet are no longer in tension.
|
|
|
|
WHY THE PAYLOAD IS CHECKED TWICE, WITH DIFFERENT DIALECTS
|
|
|
|
`bash -n` proves a file parses at all. shellcheck's dash dialect proves it uses
|
|
nothing bash-only, which is what matters for anything under payload/: the
|
|
printer runs busybox ash, not bash, and a bash-only construct parses perfectly
|
|
here and fails there. dash rather than sh as the dialect because busybox ash,
|
|
like dash, supports `local`, which the payload uses.
|
|
"""
|
|
import importlib.util
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
|
|
import pytest
|
|
|
|
from lib.paths import ROOT
|
|
|
|
pytestmark = pytest.mark.static
|
|
|
|
|
|
# ------------------------------------------------------------------ the sets
|
|
|
|
# Everything that must at least parse. bin/ and test/ run under bash on the
|
|
# build image; payload/ runs under the printer's ash but bash parses it too,
|
|
# and a file that does not parse under either is broken beyond dialect.
|
|
SYNTAX_GLOBS = ("bin/*.sh", "payload/*.sh", "payload/firmwareExe",
|
|
"payload/bin/klipperDaemon",
|
|
"payload/etc/s6-rc/source/*/run",
|
|
"test/integration/printer/*.sh", "qa/replica/actions/*.sh")
|
|
|
|
# NOT the oneshots' `up`/`down`. Those are execline command lines, and the
|
|
# shell they carry is inside a `/bin/sh -c "..."` argument -- `bash -n` and
|
|
# shellcheck would both parse the wrapper, pass, and check none of the code
|
|
# that actually runs. Extracting the body and checking THAT is a real test and
|
|
# belongs in the replica lane beside the rest of the s6-rc assertions.
|
|
|
|
# The subset executed by the printer's busybox ash. bin/ and test/ are
|
|
# deliberately absent: they run on the build image, where bash is the shell and
|
|
# a bashism is not a defect.
|
|
#
|
|
# qa/replica/actions/ IS here, and that is not a formality. Those scripts are
|
|
# handed to entrypoint.sh and run inside the chroot exactly as a case script
|
|
# is, so they are as exposed to ash as anything under payload/. A bashism in
|
|
# hold.sh would fail at container start and read as "the replica is broken on
|
|
# this machine" -- a harness failure wearing a machine failure's clothes, which
|
|
# is the hardest kind to diagnose.
|
|
#
|
|
# The s6-rc `run` scripts are here for the same reason and were missed until
|
|
# phase 8 moved them: s6-supervise execs them on the printer, under the
|
|
# printer's ash, and a bashism in one is a service that never starts and says
|
|
# so only in s6's own log. They lived at payload/etc/s6/*/run before the move
|
|
# and no glob covered them there either.
|
|
ASH_GLOBS = ("payload/*.sh", "payload/firmwareExe",
|
|
"payload/bin/klipperDaemon",
|
|
"payload/etc/s6-rc/source/*/run", "qa/replica/actions/*.sh")
|
|
|
|
PY_GLOBS = ("bin/*.py", "payload/*.py", "payload/bin/*.py",
|
|
"payload/klipper/extras/*.py", "test/*.py", "test/ffsim/*.py",
|
|
"test/integration/*.py", "qa/*.py", "qa/lib/*.py",
|
|
"qa/static/*.py", "qa/replica/*.py")
|
|
|
|
|
|
def _files(globs):
|
|
found = []
|
|
for pattern in globs:
|
|
found += sorted(p for p in ROOT.glob(pattern) if p.is_file())
|
|
return found
|
|
|
|
|
|
def _ids(paths):
|
|
return [str(p.relative_to(ROOT)) for p in paths]
|
|
|
|
|
|
SHELL_FILES = _files(SYNTAX_GLOBS)
|
|
ASH_FILES = _files(ASH_GLOBS)
|
|
PY_FILES = _files(PY_GLOBS)
|
|
|
|
|
|
# --------------------------------------------------------- the globs are live
|
|
#
|
|
# Each lane below asserts its own inputs exist. This is not defensive noise: it
|
|
# is the exact bug the original guarded against, quoted from check_no_bashisms:
|
|
#
|
|
# With no targets shellcheck writes usage to stderr and exits 1, leaving
|
|
# `hits` empty -- a green gate that examined nothing. check_shell_syntax
|
|
# guards this; this one did not, so moving payload/ would have retired the
|
|
# ash-compatibility check silently.
|
|
#
|
|
# Parametrized tests fail the same way, and worse: zero parameters is zero
|
|
# tests, which is not even a red line. It is an empty run that reads as clean.
|
|
|
|
def test_shell_globs_match_something():
|
|
assert SHELL_FILES, "no scripts to parse -- the globs in SYNTAX_GLOBS rotted"
|
|
|
|
|
|
def test_ash_globs_match_something():
|
|
assert ASH_FILES, "no payload scripts -- has payload/ moved?"
|
|
|
|
|
|
def test_python_globs_match_something():
|
|
assert PY_FILES, "no python files -- have the globs in PY_GLOBS rotted?"
|
|
|
|
|
|
ACTIONS = sorted((ROOT / "qa" / "replica" / "actions").glob("*.sh"))
|
|
|
|
|
|
@pytest.mark.parametrize("script", ACTIONS, ids=_ids(ACTIONS))
|
|
def test_action_scripts_are_executable(script):
|
|
"""qa/replica/actions/*.sh are handed to docker, not sourced.
|
|
|
|
bake.sh is a `--entrypoint` and the others are mounted in and run, so a
|
|
missing executable bit is not a style problem -- the container dies at
|
|
init with `exec: permission denied` before a single test runs, and the
|
|
43 errors that follow all say "could not install the package", which
|
|
points at the installer rather than at a file mode.
|
|
|
|
Caught here because it happened: bake.sh was written without the bit and
|
|
the whole lane failed in 1.6 seconds with an error about the mod. It is
|
|
the same failure the payload's own s6 `run` scripts have a check for --
|
|
"s6 reports a non-executable run in its own log and nowhere else" -- and
|
|
it deserved the same treatment here.
|
|
"""
|
|
assert os.access(str(script), os.X_OK), (
|
|
"%s is not executable, so docker cannot run it. `chmod +x` it and "
|
|
"commit the mode." % script.name)
|
|
|
|
|
|
# ------------------------------------------------------------------- it parses
|
|
|
|
@pytest.mark.parametrize("script", SHELL_FILES, ids=_ids(SHELL_FILES))
|
|
def test_parses(script):
|
|
parsed = subprocess.run(["bash", "-n", str(script)],
|
|
capture_output=True, text=True)
|
|
assert parsed.returncode == 0, parsed.stderr.strip()
|
|
|
|
|
|
# ------------------------------------------------------ it is not secretly bash
|
|
|
|
@pytest.fixture(scope="session")
|
|
def bashisms():
|
|
"""shellcheck over every payload script at once, indexed by file.
|
|
|
|
One process, not one per file. shellcheck spends most of a run starting
|
|
up, so per-file invocation cost more than everything else in this lane
|
|
combined -- but per-file REPORTING is the whole point of the port. The
|
|
gcc output format is `path:line:col: level: message [SCxxxx]`, so the
|
|
findings can be split back apart by path afterwards and each test can ask
|
|
only about itself.
|
|
"""
|
|
# A failure, not a skip. There is no configuration in which running this
|
|
# suite without shellcheck is a complete run, so calling it "skipped" would
|
|
# describe a machine that is not set up as though it were a decision
|
|
# somebody made. See qa/conftest.py.
|
|
#
|
|
# pytest.fail rather than a bare assert on which(): `assert None` is what
|
|
# an assert would lead with, and the sentence explaining how to fix it
|
|
# would be the part that got truncated in the summary.
|
|
if not shutil.which("shellcheck"):
|
|
pytest.fail(
|
|
"shellcheck is not installed, so nothing checked that the payload "
|
|
"is free of bash-only constructs -- the printer runs busybox ash. "
|
|
"The build image has it: run `make qa`, or install it locally "
|
|
"(dnf install ShellCheck / apt install shellcheck).")
|
|
|
|
checked = subprocess.run(
|
|
["shellcheck", "-s", "dash", "-f", "gcc"]
|
|
+ [str(p) for p in ASH_FILES],
|
|
capture_output=True, text=True, cwd=str(ROOT))
|
|
|
|
# SC3xxx is shellcheck's "not supported in POSIX sh" family; SC2039 is its
|
|
# older spelling of the same thing. Everything else it says here is style,
|
|
# and a gate that goes red for style gets switched off.
|
|
found = {}
|
|
for line in (checked.stdout or "").splitlines():
|
|
if "SC3" not in line and "SC2039" not in line:
|
|
continue
|
|
path = line.split(":", 1)[0]
|
|
found.setdefault(path, []).append(line)
|
|
|
|
# shellcheck exits 1 for findings, which `found` covers, but also for a
|
|
# usage or file error -- which would otherwise pass unnoticed as "no
|
|
# findings". Raised here rather than returned so it fails every test in
|
|
# the lane instead of hiding behind whichever file happens to be clean.
|
|
if not found and checked.returncode not in (0, 1):
|
|
raise AssertionError(
|
|
"shellcheck failed to run (exit %d): %s"
|
|
% (checked.returncode, (checked.stderr or "").strip()[:200]))
|
|
return found
|
|
|
|
|
|
@pytest.mark.parametrize("script", ASH_FILES, ids=_ids(ASH_FILES))
|
|
def test_no_bash_only_constructs(script, bashisms):
|
|
# shellcheck echoes back the path it was given. It was given absolute
|
|
# paths, but match on both spellings so a future cwd-relative call site
|
|
# does not silently stop matching and report every file clean.
|
|
rel = str(script.relative_to(ROOT))
|
|
hits = bashisms.get(str(script), []) + bashisms.get(rel, [])
|
|
assert not hits, "bash-only constructs:\n " + "\n ".join(hits)
|
|
|
|
|
|
# ------------------------------------------------------- every name is bound
|
|
|
|
# One test, not one per file: pyflakes resolves names within a module, so the
|
|
# per-file split buys nothing here and costs an interpreter start each. The
|
|
# failure message names the files itself.
|
|
def test_no_undefined_names():
|
|
"""A name that does not exist anywhere it is read.
|
|
|
|
py_compile catches typos in syntax; nothing caught typos in names, so a
|
|
rename that renamed an assignment and missed one of its uses stayed green
|
|
through both lanes and waited for the line to execute on the printer. That
|
|
is how `_plate_check` came to reach for `z_trigger` when what it had bound
|
|
was `station_z` -- on the one path that runs BEFORE a nozzle is allowed to
|
|
descend.
|
|
"""
|
|
# The debian package ships the module without putting a pyflakes
|
|
# executable on PATH, so shutil.which() -- the shape the shellcheck test
|
|
# above uses -- reports it missing on the very image that has it.
|
|
#
|
|
# A failure, not a skip, for the same reason as shellcheck above.
|
|
if importlib.util.find_spec("pyflakes") is None:
|
|
pytest.fail(
|
|
"pyflakes is not installed, so nothing checked that every name is "
|
|
"bound -- which is the gate that catches a rename missing one of "
|
|
"its uses. The build image has it: run `make qa`, or install it "
|
|
"locally (apt install python3-pyflakes / pip install pyflakes).")
|
|
|
|
checked = subprocess.run(
|
|
[sys.executable, "-m", "pyflakes"] + [str(p) for p in PY_FILES],
|
|
capture_output=True, text=True, cwd=str(ROOT))
|
|
|
|
# Only the fatal findings count. pyflakes also reports unused imports and
|
|
# the like, which are untidy but cannot fail at runtime.
|
|
fatal = ("undefined name", "referenced before assignment",
|
|
"syntax error", "invalid syntax")
|
|
hits = [line for line in (checked.stdout or "").splitlines()
|
|
if any(marker in line.lower() for marker in fatal)]
|
|
|
|
assert hits or checked.returncode in (0, 1), (
|
|
"pyflakes failed to run (exit %d): %s"
|
|
% (checked.returncode, (checked.stderr or "").strip()[:200]))
|
|
|
|
assert not hits, "names used but never bound:\n " + "\n ".join(hits)
|