Skip to content

cli: don't prepare the host for config-only artifact dumps - #10485

Open
iav wants to merge 1 commit into
armbian:mainfrom
iav:fix/artifact-config-dump-json-hostrelease
Open

cli: don't prepare the host for config-only artifact dumps#10485
iav wants to merge 1 commit into
armbian:mainfrom
iav:fix/artifact-config-dump-json-hostrelease

Conversation

@iav

@iav iav commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

artifact-config-dump-json died with error 43 on any plain checkout:
CONFIG_DEFS_ONLY=yes makes prep_conf_main_minimal_ni() skip
check_basic_host(), but the artifact handler still reaches prepare_host(),
whose late_prepare_host_dependencies() requires HOSTRELEASE and HOSTARCH.

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 runs
Python 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() as obtain_hostarch_only().

Tested on an arm64 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. An armhf target (helios4, mvebu) behaves
the same; HOSTARCH=amd64 leaves artifact_version byte-identical.

Repro before the fix:

./compile.sh artifact-config-dump-json WHAT=kernel BOARD=helios64 BRANCH=edge

Summary by CodeRabbit

  • Bug Fixes
    • Improved host architecture detection and initialization during artifact version aggregation.
    • Configuration-only exports now avoid unnecessary host preparation when artifact aggregation is not required.
  • Refactor
    • Streamlined host architecture detection while preserving existing validation behavior.
    • Reorganized internal library loading without changing functionality.
  • Chores
    • Updated autogenerated-file references for improved maintenance clarity.

`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
@github-actions github-actions Bot added 08 Milestone: Third quarter release size/medium PR with more then 50 and less then 250 lines Needs review Seeking for review Framework Framework components labels Aug 20, 2026
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d7207a23-4f78-429f-9841-26554d7d0e61

📥 Commits

Reviewing files that changed from the base of the PR and between ae15649 and ba9b01c.

📒 Files selected for processing (4)
  • lib/functions/cli/cli-artifact.sh
  • lib/functions/host/host-release.sh
  • lib/functions/main/start-end.sh
  • lib/library-functions.sh

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

Host initialization and artifact aggregation

Layer / File(s) Summary
Host architecture detection
lib/functions/host/host-release.sh
obtain_hostarch_only() now encapsulates host architecture detection. The existing host release and architecture check calls this function.
Conditional host setup and aggregation
lib/functions/main/start-end.sh, lib/functions/cli/cli-artifact.sh
Configuration-only dumps skip host preparation unless artifact version aggregation is required. Aggregation initializes missing HOSTRELEASE and HOSTARCH values.

Generated library sourcing

Layer / File(s) Summary
Generated library comments and source order
lib/library-functions.sh
Autogenerated-file comments reference ./lib/tools/gen-library.sh. The compilation/stubble.sh source block now follows compilation/patch/patching.sh.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ba9b0

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)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: skipping host preparation for config-only artifact dumps.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@iav

iav commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Beyond fixing the crash, this makes an existing exact answer usable. The artifact hash is the digest of every input (git SHA, patches, .config, drivers, framework bash), so running this command on the base and on a PR branch and diffing artifact_version deterministically answers "rebuild or cache hit?" -- the question #10473 currently predicts with a model.

That was impractical until now: on a plain checkout the command died with error 43, and in CI it only worked inside an already-prepared environment. It now runs out of the box and, for WHAT=kernel, touches nothing on the host.

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.

@rpardini

Copy link
Copy Markdown
Member

Hmm. Isn't this covered by PRE_PREPARED_HOST (https://github.com/armbian/build/blob/main/lib/tools/common/armbian_utils.py#L471-L475 - as called by the actual matrix-prepare GHA step?

(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)

@iav

iav commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Nothing loses its host preparation. In GHA the invoker still prepares it and passes PRE_PREPARED_HOST=yes — unchanged. Real builds (build/kernel/rootfs, i.e. anything without CONFIG_DEFS_ONLY) still go through prepare_host() — unchanged. The dump itself doesn't need a prepared host, it only prints variables; the exception is artifacts whose version requires aggregation (WHAT=rootfs), which runs python from the host deps and calls assert_prepared_host() — that path still prepares, which is why the skip is conditional. Net effect: the host is prepared exactly when something is about to be built, and a hand-run dump on a fresh checkout prints the vars instead of exiting 43 with HOSTRELEASE is not set.

@rpardini

Copy link
Copy Markdown
Member

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.

@iav

iav commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

We won't know until we try™

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

08 Milestone: Third quarter release Framework Framework components Needs review Seeking for review size/medium PR with more then 50 and less then 250 lines

Development

Successfully merging this pull request may close these issues.

2 participants