artifact-uboot: opt-in cache-bust via UBOOT_HASH_EXTRA for prebuilt blobs

[igorpecovnik]

Summary

The u-boot artifact version hashed the UBOOT_TARGET_MAP string but not the identity of any prebuilt blob it points at. Bumping a blob in place (same path) left the artifact version unchanged, so the build reused the cached .deb and the new blob never got packaged.

This adds an opt-in version string UBOOT_HASH_EXTRA that a board/family sets when it ships a prebuilt or externally-fetched u-boot blob, and bumps whenever the blob changes. It folds into the artifact version's -V hash, forcing a repackage.

Why a string, not a file content-hash

The first cut parsed UBOOT_TARGET_MAP for filenames and hashed the bytes. Two problems killed that (per review):

• Parsing the map is fragile — it mixes build inputs and outputs indistinguishably (@rpardini: handle it out-of-band, don't parse it).
• Content-hashing a file doesn't work in general — a u-boot blob may be fetched at build time and not live in this repo, so it doesn't exist at matrix-prep when prepare_version runs. Reading it would fail or be non-deterministic across hosts.

A string sidesteps both: no filesystem access at matrix-prep, and it works for any blob regardless of where it lives.

Behaviour

• lib/functions/configuration/main-config.sh — UBOOT_HASH_EXTRA declared + documented centrally (with the other u-boot defaults); :- so it never clobbers a board value.
• lib/functions/artifacts/artifact-uboot.sh — appended to vars_to_hash only when set.
• config/sources/families/meson-gxl.conf — khadas-vim1 / khadas-vim2 opt in.

No churn: with UBOOT_HASH_EXTRA unset, -V is byte-identical to the pre-change baseline, so every other board's u-boot artifact is untouched. Set → distinct hash; bumping 0.11→0.17.3 → distinct again → repackage.

Note for the blob bump (#9994)

This is the manual-but-universal trade-off: the blob-bump PR (#9994) must also bump UBOOT_HASH_EXTRA (→ khadas-uboot-0.17.3) when it swaps the blob, or the cache won't invalidate.

Split out from #9994, which depends on this to actually repackage the new blobs.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces UBOOT_HASH_EXTRA as an opt-in global variable that allows boards to override u-boot artifact versioning. When set, it is appended to the hash computation in artifact_uboot_prepare_version, altering the artifact version's -V segment. Applied to khadas-vim1 and khadas-vim2 boards to synchronize prebuilt u-boot blob versions.

Changes

U-Boot artifact cache-bust with UBOOT_HASH_EXTRA

Layer / File(s)	Summary
UBOOT_HASH_EXTRA configuration declaration lib/functions/configuration/main-config.sh	Declares UBOOT_HASH_EXTRA as a new global read-only variable in do_main_configuration(), defaulting to empty if unset. Documents it as a cache-bust string for boards/families using external or prebuilt u-boot blobs.
UBOOT_HASH_EXTRA artifact version hashing lib/functions/artifacts/artifact-uboot.sh	Appends UBOOT_HASH_EXTRA to vars_to_hash when set, so it affects the computed artifact version's -V segment. Updates comments to clarify the cache-bust behavior and that no filesystem read occurs for the extra hash text.
khadas board UBOOT_HASH_EXTRA sync values config/sources/families/meson-gxl.conf	Sets UBOOT_HASH_EXTRA="khadas-uboot-0.11" for khadas-vim1 and khadas-vim2 to synchronize prebuilt u-boot blob versions with artifact versioning. Includes sync comments tied to the khadas-uboot release.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A clever hash suffix brings harmony true,
When boards fetch blobs from far and wide too,
The -V now whispers khadas-uboot-0.11,
Cache-busting in style—no secrets to tell! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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 accurately summarizes the main change: introducing an opt-in cache-bust mechanism via UBOOT_HASH_EXTRA for prebuilt u-boot blobs, which is the core objective of the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/artifact-uboot-blob-hash

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

lib/functions/artifacts/artifact-uboot.sh (1)

133-135: 💤 Low value

Consider deduplicating file paths before hashing.

The tokenization splits UBOOT_TARGET_MAP on both ; and :, treating all tokens equally. If the same file path appears in multiple roles (e.g., source and destination in blob.bin:blob.bin), it will be added to the array multiple times and hashed multiple times. This makes the hash depend on multiplicity rather than just content.

While this still achieves cache invalidation when content changes, it could cause unnecessary rebuilds if the target map is refactored to remove duplicate references. In practice, prebuilt blobs are likely referenced once, so the impact is minimal.

For robustness, consider deduplicating the array:

Optional improvement for deduplication

 declare _tok
+declare -A seen_files=()
 for _tok in ${UBOOT_TARGET_MAP//[;:]/ }; do
-	[[ -f "${_tok}" ]] && uboot_blob_files+=("${_tok}")
+	if [[ -f "${_tok}" && -z "${seen_files[${_tok}]}" ]]; then
+		uboot_blob_files+=("${_tok}")
+		seen_files["${_tok}"]=1
+	fi
 done

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/functions/artifacts/artifact-uboot.sh` around lines 133 - 135, The loop
that processes UBOOT_TARGET_MAP and populates the uboot_blob_files array does
not deduplicate file paths, so identical files appearing multiple times in the
target map (e.g., "blob.bin:blob.bin") will be added to the array multiple times
and hashed multiple times. To fix this, deduplicate the uboot_blob_files array
after the loop completes by using a bash associative array or by checking if
each file path already exists in the array before adding it. This ensures each
unique file path is only hashed once, preventing unnecessary rebuilds when the
target map is refactored to remove duplicate references.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lib/functions/artifacts/artifact-uboot.sh`:
- Around line 133-135: The loop that processes UBOOT_TARGET_MAP and populates
the uboot_blob_files array does not deduplicate file paths, so identical files
appearing multiple times in the target map (e.g., "blob.bin:blob.bin") will be
added to the array multiple times and hashed multiple times. To fix this,
deduplicate the uboot_blob_files array after the loop completes by using a bash
associative array or by checking if each file path already exists in the array
before adding it. This ensures each unique file path is only hashed once,
preventing unnecessary rebuilds when the target map is refactored to remove
duplicate references.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 638c82df-da10-4dfd-8304-50907de40967

📥 Commits

Reviewing files that changed from the base of the PR and between 3072b4a and 45d8786.

📒 Files selected for processing (1)

• lib/functions/artifacts/artifact-uboot.sh

[igorpecovnik]

@rpardini Do we have a better way?

[rpardini]

@rpardini Do we have a better way?

I'd do it "out of band". UBOOT_TARGET_MAP is already a curse in itself, parsing it for filenames is even worse as we can't tell input from output.

Maybe introduce a new array var, eg declare -a UBOOT_HASH_EXTRA_BINS=() which the board/family can fill-in when they care about it (vim1/vim2 prebuilt for example) which are then hashed without further parsing?

[rpardini]

Also: this would never work in a json/matrix prepare -- the blobs simply won't be there at that stage. (maybe for this vim1/vim2 case, those blobs are in armbian/build, it might work by accident. but won't work for eg rkbin etc).

[igorpecovnik]

Then perhaps rather this way?

[rpardini]

Then perhaps rather this way?

Yes, that can work.

[github-actions]

✅ This PR has been reviewed and approved — all set for merge!