Skip to content

artifact-rootfs: surface git-log failure when computing configng hash #9849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
igorpecovnik wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −3
Open

artifact-rootfs: surface git-log failure when computing configng hash #9849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions lib/functions/artifacts/artifact-rootfs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,33 @@ function artifact_rootfs_config_dump() {
# Track the latest commit touching configng's desktop definitions.
# Any change to YAML, parser, or module code in tools/modules/desktops/
# invalidates the desktop rootfs cache — the package list, browser
# mapping, tier overrides, or branding may have changed.
# mapping, tier overrides, or branding may have changed. Scoped to
# desktop builds and the desktop subtree because armbian-config is
# only invoked at build time for desktop installs
# (`module_desktops install mode=build`); for CLI builds the .deb
# is just installed and its contents don't affect the rootfs.
if [[ "${BUILD_DESKTOP}" == "yes" ]]; then
declare configng_desktops_hash="undetermined"

@coderabbitai coderabbitai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Variable still named CONFIGNG_DESKTOPS_HASH instead of CONFIGNG_HASH.

The PR objectives state the fix should "rename CONFIGNG_DESKTOPS_HASH → CONFIGNG_HASH", but the code at lines 27, 43, 46, 49, and 81 continues to use CONFIGNG_DESKTOPS_HASH and configng_desktops_hash. This name implies desktop-only scope, which contradicts the objective of applying the hash to all build types.

Also applies to: 43-43, 46-46, 49-49, 81-81

🤖 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-rootfs.sh` at line 27, The variable name
still uses CONFIGNG_DESKTOPS_HASH/configng_desktops_hash but should be renamed
to CONFIGNG_HASH/configng_hash everywhere; update the declaration (declare
configng_hash="undetermined"), all subsequent references and assignments (e.g.,
places currently referencing configng_desktops_hash or CONFIGNG_DESKTOPS_HASH),
and any export or string interpolation to use CONFIGNG_HASH/configng_hash so the
hash applies to all build types rather than only "desktops".

local configng_dir="${SRC}/cache/sources/armbian-configng"
if [[ -d "${configng_dir}/.git" ]]; then
configng_desktops_hash="$(git -C "${configng_dir}" log -1 --format=%H -- tools/modules/desktops/ 2>/dev/null || echo "unknown")"
# Capture stdout + stderr, then branch on exit status. This is
# a best-effort knob for cache-invalidation, not a build
# blocker - if git can't read this clone (torn checkout,
# stale files, broken HEAD, permissions), we warn and fall
# through to "unknown" rather than aborting an hour-long
# build. The downstream check in create-cache.sh skips the
# fingerprint fold on "unknown" / "undetermined" — the
# build still produces a valid image, it just doesn't get
# the configng-aware cache bust.
declare git_out git_rc
git_out="$(git -C "${configng_dir}" log -1 --format=%H -- tools/modules/desktops/ 2>&1)"

@coderabbitai coderabbitai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Path filter -- tools/modules/desktops/ still present in git log.

The PR objectives state the fix should "remove the path filter from git log -1" so that "any commit" in armbian-configng invalidates the cache, but line 40 still restricts the hash to commits touching tools/modules/desktops/. This means commits to CLI helpers, system modules, postinst scripts, or the parser will not invalidate the cache—the exact bug the PR claims to fix.

🤖 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-rootfs.sh` at line 40, The git invocation in
artifact-rootfs.sh still uses a path filter ("tools/modules/desktops/") so the
commit hash in git_out only reflects changes to that path; remove the path
filter so git computes the last commit for the whole repo by changing the git
command that sets git_out (the git -C "${configng_dir}" log -1 --format=%H ...
call) to not include the path argument, leaving only git -C "${configng_dir}"
log -1 --format=%H 2>&1; keep the same variable (git_out) and error handling
around that command.

git_rc=$?
if (( git_rc == 0 )); then
configng_desktops_hash="${git_out}"
else
display_alert "configng_desktops hash: git log failed (rc=${git_rc}) in ${configng_dir}" "${git_out}" "warn"
configng_desktops_hash="unknown"
fi
fi
artifact_input_variables[CONFIGNG_DESKTOPS_HASH]="${configng_desktops_hash}"
fi
Comment on lines 26 to 50

@coderabbitai coderabbitai Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

PR objectives claim BUILD_DESKTOP gate should be removed, but it remains.

The PR objectives state the fix should "remove the BUILD_DESKTOP gate" so "any commit in cache/sources/armbian-configng now bumps the rootfs cache key for all build flavours" (emphasis added). However, lines 26-50 still wrap the entire hash computation in if [[ "${BUILD_DESKTOP}" == "yes" ]], meaning CLI builds continue to skip configng cache invalidation. This contradicts the stated bug fix and leaves the original issue unresolved.

🤖 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-rootfs.sh` around lines 26 - 50, The
BUILD_DESKTOP conditional is still preventing configng cache-busting for
non-desktop builds; remove the surrounding if [[ "${BUILD_DESKTOP}" == "yes" ]]
check so the configng_desktops_hash logic always runs: always declare and
compute configng_desktops_hash (using configng_dir and the git -C ... log
command with git_out/git_rc), call display_alert on failure, set
artifact_input_variables[CONFIGNG_DESKTOPS_HASH] to the resulting hash (or
"unknown"/"undetermined" as the existing logic does), and ensure no
BUILD_DESKTOP gating remains around those statements.

Expand Down
4 changes: 3 additions & 1 deletion lib/functions/rootfs/create-cache.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ function calculate_rootfs_cache_id() {
# this, a configng commit that changes which packages a DE
# installs leaves the existing rootfs tarball untouched in the
# cache and every subsequent build cache-hits the pre-change
# version.
# version. Scoped to desktop builds: CLI images don't invoke
# armbian-config at build time, so configng content doesn't
# affect their rootfs.
#
# CONFIGNG_DESKTOPS_HASH is set by artifact_rootfs_config_dump
# above from `git log -1 -- tools/modules/desktops/` in the
Expand Down
Loading