mirror of
https://github.com/Klipper4FlashForge/firmware.git
synced 2026-10-21 21:02:16 -05:00
Give apk a version it can parse in CI, and say so when it cannot
ci.yml writes MOD_VER=ci into config.env, and MOD_VER is anvil-core's PKG_VERSION. apk's version grammar wants a digit first, so mkpkg refused it -- with "info field 'version' has invalid value", naming neither the package nor the value nor where it came from. A free-form version was fine for .ipk, which is why the line survived the migration unread. MOD_VER=0 now. It is pinned rather than left to the date default so two runs of one tree produce the same bytes, and 0 says "not a release" as plainly as the word did while ordering below every real version. emit_apk checks the version before calling mkpkg and names the package, the offending value and the config.env setting to fix. The grammar test covers the recipes, and could not have caught this: the value comes from config.env, so anvil-core answers with whatever the machine running the test happens to say -- a date, locally, always valid. The new gate reads MOD_VER out of the workflow files themselves, which is the only place that value is written down, with a companion asserting a workflow still pins one at all so the check cannot pass by having nothing to check. busybox's pkg.conf expanded $BUSYBOX_BIN bare and let a failed sha256sum through a pipeline. Sourced under `set -euo pipefail` it therefore failed -- correctly skipping the recipe, since pkg_recipes evaluates PKG_WHEN in a subshell, but printing "BUSYBOX_BIN: unbound variable" twice into every CI log immediately above the first real failure, where it reads like the cause. With that fixed every recipe reads under the build's own flags, so the unsigned-recipe gate now covers all of them rather than only the ones a build would make: a recipe that cannot be read is dropped by that subshell rather than reported, so the failure mode is a package silently leaving the feed. Verified with the production key: 290 static, 135 replica, 47 signed packages and a payload. MOD_VER=ci reproduced locally against the new message, and MOD_VER=0 built and pruned clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rbKEyLXrzSE6wm3iNApVp
This commit is contained in:
1 parent
734a04db45
commit
f11b63b6c1
4 files changed
+108
-9
No files matched your search
@@ -198,7 +198,13 @@ jobs:
|
||||
run: |
|
||||
cat > config.env <<'CFG'
|
||||
MOD_NAME=anvil
|
||||
MOD_VER=ci
|
||||
# 0, NOT "ci": MOD_VER is anvil-core's PKG_VERSION, and apk's version
|
||||
# grammar wants a number first -- `ci` is rejected by mkpkg with
|
||||
# "package version is invalid" and no clue which package or field.
|
||||
# It is pinned rather than left to default so two runs of one tree
|
||||
# produce the same bytes; 0 says "not a release" as plainly as the
|
||||
# word did, and orders below every real one.
|
||||
MOD_VER=0
|
||||
TARGET_MACHINE=Creator5Pro
|
||||
# No stock paths: bin/common.sh defaults them into vendor/, where
|
||||
# `make vendor` puts the pinned download. Setting them here would be
|
||||
|
||||
@@ -242,6 +242,21 @@ for r in "${RECIPES[@]}"; do
|
||||
done
|
||||
fi
|
||||
|
||||
# THE VERSION, CHECKED HERE, because mkpkg's own refusal names
|
||||
# neither the package nor the value: it says "info field 'version'
|
||||
# has invalid value" and stops. MOD_VER=ci in a workflow reached
|
||||
# this and cost a CI round trip to identify. pkg_version_ok is the
|
||||
# same grammar qa/static/test_apk.py holds every recipe to; this
|
||||
# catches the versions that come from config.env instead, which no
|
||||
# test of the recipes can see.
|
||||
pkg_version_ok "$_fullver" || {
|
||||
echo "!! $_nm's version '$_fullver' is not one apk can parse." >&2
|
||||
echo " The grammar is number{.number}{letter}{_suffix}{~hash}{-r#}:" >&2
|
||||
echo " it has to start with a digit. If that value came from" >&2
|
||||
echo " MOD_VER or another config.env setting, fix it there." >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
_out="$PKG_FEED/${_nm}-${_fullver}.apk"
|
||||
# $APK_SIGN is empty when no key is configured, and an unsigned
|
||||
# package is a valid one -- apk simply writes no signature block.
|
||||
|
||||
Vendored
+11
-3
@@ -14,13 +14,21 @@ PKG_NAME=anvil-busybox
|
||||
# a path that exists on only one of them.
|
||||
PKG_WHEN='[ -n "${BUSYBOX_BIN:-}" ] && [ -f "$BUSYBOX_BIN" ]'
|
||||
|
||||
# ${BUSYBOX_BIN:-} RATHER THAN $BUSYBOX_BIN in both expansions below, and
|
||||
# `|| true` on the hash: bin/build-packages.sh sources every pkg.conf under
|
||||
# `set -euo pipefail`, where an unset variable is an error and a failed
|
||||
# sha256sum fails the whole pipeline. pkg_recipes evaluates PKG_WHEN in a
|
||||
# subshell, so the recipe was correctly skipped either way -- but the two
|
||||
# "BUSYBOX_BIN: unbound variable" lines it printed on every CI run sat
|
||||
# immediately above the first real failure and read like its cause.
|
||||
#
|
||||
# READ OUT OF THE BINARY, because there is nothing else to read it from: no
|
||||
# tarball name, no versions.env pin, no configure script. busybox stamps
|
||||
# "BusyBox v1.31.1" into its own .rodata, and that string is the only
|
||||
# statement about this file's identity that comes from the file. A build that
|
||||
# cannot be identified gets 0, leaving PKG_STAMP_EXTRA as the whole cache key.
|
||||
PKG_VERSION=$(strings -a "$BUSYBOX_BIN" 2>/dev/null \
|
||||
| sed -n 's/^BusyBox v\([0-9][0-9.]*\).*/\1/p' | head -n1)
|
||||
PKG_VERSION=$(strings -a "${BUSYBOX_BIN:-}" 2>/dev/null \
|
||||
| sed -n 's/^BusyBox v\([0-9][0-9.]*\).*/\1/p' | head -n1 || true)
|
||||
PKG_VERSION="${PKG_VERSION:-0}"
|
||||
PKG_RELEASE=1
|
||||
|
||||
@@ -32,7 +40,7 @@ PKG_EXCLUDE=".version"
|
||||
# THE CONTENT IS THE CACHE KEY, as for anvil-core: the version cannot see the
|
||||
# input. Two different busybox builds of v1.31.1 are the same version string,
|
||||
# and swapping one for the other must rebuild.
|
||||
PKG_STAMP_EXTRA=$(sha256sum "$BUSYBOX_BIN" 2>/dev/null | cut -c1-16)
|
||||
PKG_STAMP_EXTRA=$(sha256sum "${BUSYBOX_BIN:-}" 2>/dev/null | cut -c1-16 || true)
|
||||
|
||||
# No Depends and nothing to build against. busybox is a single static-ish
|
||||
# binary from outside this build; whatever it links, it links already, and
|
||||
|
||||
+75
-5
@@ -419,10 +419,8 @@ def _recipes_built(key=None):
|
||||
"""The recipes a build would actually make, asked with a given key.
|
||||
|
||||
Not the pkg.conf glob: pkg_recipes drops anything whose PKG_WHEN is false,
|
||||
and pkgs/3rdparty/busybox is false on every build that has no stock
|
||||
firmware to take a busybox out of. Those are never sourced by
|
||||
bin/build-packages.sh's own loop, so they are not what these tests are
|
||||
about.
|
||||
such as pkgs/3rdparty/busybox on any build with no stock firmware to take
|
||||
a busybox out of. That set is what the stability check below is about.
|
||||
"""
|
||||
env = dict(os.environ)
|
||||
env.pop("APK_SIGN_KEY", None)
|
||||
@@ -435,7 +433,7 @@ def _recipes_built(key=None):
|
||||
return sorted(out.stdout.split())
|
||||
|
||||
|
||||
@pytest.mark.parametrize("recipe", _recipes_built())
|
||||
@pytest.mark.parametrize("recipe", [p.parent.name for p in _recipes()])
|
||||
@pytest.mark.parametrize("key", ["", "/nonexistent/anvil.rsa"])
|
||||
def test_a_recipe_reads_with_no_signing_key(recipe, key):
|
||||
"""A bare checkout has no signing key, and that is the CI case.
|
||||
@@ -453,6 +451,13 @@ def test_a_recipe_reads_with_no_signing_key(recipe, key):
|
||||
Both spellings of "no key" are covered: unset, and set to a path that is
|
||||
not there. The flags are the point -- _conf above runs without them and
|
||||
would have gone green throughout.
|
||||
|
||||
EVERY recipe, not just the ones a build would make. A recipe PKG_WHEN
|
||||
switches off is still SOURCED to find that out, and pkg_recipes does it in
|
||||
a subshell that discards the result -- so one that cannot be read is
|
||||
dropped silently rather than reported. busybox did exactly that, printing
|
||||
"BUSYBOX_BIN: unbound variable" into every CI log directly above the first
|
||||
real failure.
|
||||
"""
|
||||
env = dict(os.environ)
|
||||
env.pop("APK_SIGN_KEY", None)
|
||||
@@ -539,3 +544,68 @@ def test_no_key_mounts_nothing():
|
||||
assert '":ro' not in out.stdout, (
|
||||
"an unsigned build mounts something read-only that it should not:\n%s"
|
||||
% out.stdout[-2000:])
|
||||
|
||||
|
||||
def _workflow_mod_vers():
|
||||
"""Every MOD_VER a workflow writes into config.env, with its source."""
|
||||
found = []
|
||||
for wf in sorted((ROOT / ".github" / "workflows").glob("*.yml")):
|
||||
for n, line in enumerate(wf.read_text().splitlines(), 1):
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("MOD_VER="):
|
||||
found.append((wf.name, n, stripped.split("=", 1)[1].strip()))
|
||||
return found
|
||||
|
||||
|
||||
def test_every_workflow_version_is_one_apk_can_parse():
|
||||
"""anvil-core's version is MOD_VER, and CI sets it.
|
||||
|
||||
test_every_version_is_one_apk_can_parse asks the RECIPES, and anvil-core
|
||||
answers with whatever config.env happens to say on the machine running the
|
||||
test -- a date, locally, which is always valid. It cannot see that ci.yml
|
||||
writes `MOD_VER=ci`, which apk rejects: a version has to start with a
|
||||
digit. So `make packages` was green on every developer machine and red on
|
||||
every CI run, and mkpkg's message named neither the package nor the field's
|
||||
value.
|
||||
|
||||
A value that is empty or a shell expansion is skipped: bin/common.sh
|
||||
stamps today's UTC date for the first, and the second is release.yml
|
||||
deriving the version from the tag, which has its own gate in the tag
|
||||
format.
|
||||
"""
|
||||
bad = []
|
||||
for wf, line, raw in _workflow_mod_vers():
|
||||
value = raw.strip('"').strip("'")
|
||||
if not value or "$" in value:
|
||||
continue
|
||||
full = subprocess.run(
|
||||
["bash", "-c",
|
||||
'. ./bin/common.sh >/dev/null 2>&1; . ./pkgs/lib.sh; '
|
||||
'pkg_fullversion "%s" 1' % value],
|
||||
capture_output=True, text=True, cwd=str(ROOT)).stdout.strip()
|
||||
ok = subprocess.run(
|
||||
["bash", "-c",
|
||||
'. ./bin/common.sh >/dev/null 2>&1; . ./pkgs/lib.sh; '
|
||||
'pkg_version_ok "%s"' % full],
|
||||
cwd=str(ROOT), capture_output=True)
|
||||
if ok.returncode != 0:
|
||||
bad.append("%s:%s sets MOD_VER=%s -> %s" % (wf, line, raw, full))
|
||||
|
||||
assert not bad, (
|
||||
"a workflow pins a MOD_VER apk cannot parse, so `make packages` fails "
|
||||
"there and nowhere else. A version must start with a digit:\n %s"
|
||||
% "\n ".join(bad))
|
||||
|
||||
|
||||
def test_a_workflow_actually_pins_a_version():
|
||||
"""The test above passes trivially if nothing sets MOD_VER at all.
|
||||
|
||||
CI pins it so two runs of one tree produce the same bytes; if that line is
|
||||
ever dropped, the date default makes the reproducibility gate depend on
|
||||
what day it is, and the check above would go quiet rather than red.
|
||||
"""
|
||||
pinned = [f for f in _workflow_mod_vers()
|
||||
if f[2] and "$" not in f[2]]
|
||||
assert pinned, (
|
||||
"no workflow pins MOD_VER any more, so the version check above is "
|
||||
"asserting nothing and CI builds are dated rather than reproducible")
|
||||
Reference in new issue
Block a user