cli: don't prepare the host for config-only artifact dumps - #10485
Conversation
`artifact-config-dump-json` prints, as JSON, everything the framework has
computed about a single artifact -- its name, its hash-based version, the OCI
target it would be pulled from or pushed to, and the .deb names it produces --
without building anything. The info-gatherer
(lib/tools/info/info-gatherer-artifact.py) uses it to build the CI matrix and
decide what can be reused from cache. By hand it answers "did my change move
the artifact hash, i.e. rebuild or cache hit?" in a minute instead of a full
build.
From a plain checkout the command always died:
./compile.sh artifact-config-dump-json WHAT=kernel BOARD=helios64 BRANCH=edge
[ error! ] HOSTRELEASE is not set
[ error! ] Exiting with error 43
The command sets CONFIG_DEFS_ONLY=yes, so prep_conf_main_minimal_ni() skips
check_basic_host() and neither HOSTRELEASE nor HOSTARCH is ever set. The
`artifact` handler nevertheless goes through do_with_default_build() ->
main_default_start_build() -> prepare_host(), whose
late_prepare_host_dependencies() requires both. CI never hit this: there the
variables come from the already-prepared parent environment.
Satisfying the requirement would be the wrong fix -- it lets a command whose
contract is "change nothing, just print the vars" install host dependencies,
manage apt-cacher-ng and sync the clock. Skip host preparation instead, except
for artifacts whose version needs aggregation: aggregation runs Python from the
host dependencies and asserts a prepared host, so `WHAT=rootfs` legitimately
needs it. For that case, obtain HOSTRELEASE and HOSTARCH -- the latter split
out of obtain_and_check_host_release_and_arch() as obtain_hostarch_only() --
without the host sanity checks a config dump has no business enforcing.
Tested on an arm64 Armbian host, in docker and with PREFER_DOCKER=no:
`WHAT=kernel` no longer prepares the host, `WHAT=rootfs` still does and still
works, and every artifact_version string is unchanged from before the fix. An
armhf target (helios4, mvebu) behaves the same, and forcing HOSTARCH=amd64
leaves artifact_version byte-identical, so the host architecture does not enter
the artifact hash.
Assisted-by: Claude:claude-opus-5
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds dedicated host architecture detection, makes host preparation conditional for configuration-only dumps, initializes both host values during artifact aggregation, and updates generated library sourcing order and generator comments. ChangesHost initialization and artifact aggregation
Generated library sourcing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR prevents unnecessary host preparation for config-only artifact dumps while preserving required preparation for rootfs artifacts; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Beyond fixing the crash, this makes an existing exact answer usable. The artifact hash is the digest of every input (git SHA, patches, That was impractical until now: on a plain checkout the command died with It does not replace the narrowing #10473 does -- a dump costs about a minute per artifact, so 403 boards is out of the question. It is an exact check on top of that narrowing. |
|
Hmm. Isn't this covered by (I think the reasoning back then was that the host is going to be prepared by the invoker of the config dump, so doesn't "need" to be prepared again) |
|
Nothing loses its host preparation. In GHA the invoker still prepares it and passes |
|
Yes, the assumption changes from "host will have been prepared by parent/caller" to "host doesn't ever need to be prepared (save for rootfs)" - I don't mind, just wonder how much fallout will come later. |
|
We won't know until we try™ |
artifact-config-dump-jsondied witherror 43on any plain checkout:CONFIG_DEFS_ONLY=yesmakesprep_conf_main_minimal_ni()skipcheck_basic_host(), but the artifact handler still reachesprepare_host(),whose
late_prepare_host_dependencies()requiresHOSTRELEASEandHOSTARCH.Satisfying that requirement is the wrong fix: a command that only prints
variables would then install host dependencies, manage apt-cacher-ng and sync
the clock. Host preparation is skipped for config-only dumps instead -- except
for artifacts whose version needs aggregation (
WHAT=rootfs), which runsPython from the host dependencies and asserts a prepared host. For that case
HOSTRELEASE and HOSTARCH are obtained, the latter split out of
obtain_and_check_host_release_and_arch()asobtain_hostarch_only().Tested on an arm64 host, in docker and with
PREFER_DOCKER=no:WHAT=kernelno longer prepares the host,
WHAT=rootfsstill does and still works, and everyartifact_versionstring is unchanged. An armhf target (helios4, mvebu) behavesthe same;
HOSTARCH=amd64leavesartifact_versionbyte-identical.Repro before the fix:
Summary by CodeRabbit