Skip to content

Normalize kernel config hash inputs#9277

Open
iav wants to merge 3 commits intoarmbian:mainfrom
iav:fix/kernel-config-hash-normalization
Open

Normalize kernel config hash inputs#9277
iav wants to merge 3 commits intoarmbian:mainfrom
iav:fix/kernel-config-hash-normalization

Conversation

@iav
Copy link
Contributor

@iav iav commented Jan 20, 2026

Summary

Three improvements to kernel artifact versioning.

1. Deduplicate config hash inputs

Problem
The kernel config hash (kernel_config_modification_hash) was sensitive to:

  • Option insertion order from different hooks
  • Overridden assignments (same key set multiple times with different values)

This caused identical effective configurations to produce different hashes.

Solution
Use tac | sort -t '=' -uk 1,1 pipeline to keep only the last assignment per key:

  • tac reverses order so last occurrence becomes first
  • sort -uk 1,1 keeps first occurrence of each unique key

Result: same effective config always produces the same hash.

2. Add hook for customizing version parts

Add artifact_kernel_version_parts hook that allows extensions to customize the kernel artifact version string:

  • artifact_version_parts: associative array of key=value pairs. Extensions can add, modify, or remove parts.
  • artifact_version_part_order: array defining assembly order. Extensions can reorder or add new keys.

3. Extension: kernel-version-toolchain

Example extension that adds compiler identifier (e.g., gcc13.3, clang18.1) to kernel artifact version string.

Enable with: ENABLE_EXTENSIONS="kernel-version-toolchain"

This ensures kernel cache invalidation when the toolchain changes.

Result

  • Hash is stable for the same effective option set
  • Extensions can customize artifact version string
  • Toolchain identification available as opt-in extension

Summary by CodeRabbit

  • Improvements

    • More consistent kernel-configuration hashing by collapsing duplicate assignments and producing a stable, compact version suffix only when non-empty.
    • Adjusted final version assembly behavior when skipping the makefile version to ensure a predictable leading digit and suffix handling.
  • New Features

    • Centralized, ordered version-suffix composition via configurable parts and ordering.
    • Optional extension to capture toolchain major.minor (gcc/clang) as a version component.

@iav iav requested a review from a team as a code owner January 20, 2026 05:05
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

Reworks kernel artifact versioning: deduplicates kernel-config assignments keeping last per key before hashing, introduces global ordered version-part structures and a hook for extensions, builds a conditional suffix (supports internal keys), and adds an extension that appends a toolchain identifier into the version-part order.

Changes

Cohort / File(s) Summary
Kernel artifact hashing & version parts
lib/functions/artifacts/artifact-kernel.sh
Replace direct hash of all kernel_config_modifying_hashes with reverse→dedupe (keep last assignment per key) then hash; add declare -A -g artifact_version_parts and declare -a -g artifact_version_part_order; call artifact_kernel_version_parts hook; assemble ordered common_version_suffix (supports internal _ keys used without key names); adjust final version assembly for KERNEL_SKIP_MAKEFILE_VERSION=yes and non-skip paths.
Toolchain version extension
extensions/kernel-version-toolchain.sh
Add artifact_kernel_version_parts__add_toolchain() which detects KERNEL_COMPILER (default gcc), locates the compiler binary, queries -dumpfullversion -dumpversion, extracts major.minor to form gccX.Y or clangX.Y (fallback unknown), logs debug alert, sets artifact_version_parts["_T"], and appends "0085-_T" to artifact_version_part_order; tolerant of missing binaries/output.

Sequence Diagram(s)

sequenceDiagram
    participant KernelScript as KernelScript
    participant HookExtension as HookExtension
    participant SystemCompiler as SystemCompiler
    participant Hasher as Hasher
    rect rgba(200,230,255,0.5)
    KernelScript->>KernelScript: collect kernel config assignment lines
    KernelScript->>KernelScript: reverse order (tac) → dedupe by key (sort -uk) to keep last assignment
    KernelScript->>Hasher: compute hash of deduped config
    end
    KernelScript->>HookExtension: call artifact_kernel_version_parts hook
    HookExtension->>SystemCompiler: locate compiler binary and request version (-dumpfullversion -dumpversion)
    SystemCompiler-->>HookExtension: return version (or none)
    HookExtension-->>KernelScript: set artifact_version_parts["_T"] and append "0085-_T" to artifact_version_part_order
    KernelScript->>KernelScript: assemble ordered version suffix from artifact_version_parts + part order
    KernelScript->>Hasher: include suffix in final artifact version output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibbled config lines, kept the last nibble neat,

I ordered tiny parts so versions hop to the beat,
I sniffed the toolchain, named _T with a wink,
A suffix made tidy—quicker than you think.
Hop—hashes and tags now bounce in sync.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Normalize kernel config hash inputs' directly matches the primary change: deduplicating config hash inputs to normalize/stabilize the hashing process described in objective 1.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

  • 136.113.208.247/32 (new)
  • 34.170.211.100/32
  • 35.222.179.152/32

Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist.


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 and usage tips.

@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines 02 Milestone: First quarter release Needs review Seeking for review Framework Framework components labels Jan 20, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@lib/functions/compilation/armbian-kernel.sh`:
- Around line 520-540: The sort invocations that build
opts_n_norm/opts_y_norm/opts_m_norm and opts_val_keys are locale-dependent and
can yield non-deterministic ordering; update the commands that use sort -u and
sort to force a stable C locale (e.g., prefix with LC_ALL=C) so the arrays
(opts_n_norm, opts_y_norm, opts_m_norm, opts_val_keys) and derived
kernel_config_modifying_hashes are built deterministically across environments.
Ensure you apply this change to every occurrence of sort used for these lists in
the block surrounding opts_n_norm/opts_y_norm/opts_m_norm and opts_val_keys.
- Around line 519-540: The variable opts_val_keys is being created in function
scope (global) and should be local like the other arrays; declare opts_val_keys
as a local variable (e.g., add it to the existing local/declare array line with
opts_n_norm/opts_y_norm/opts_m_norm) so it doesn't leak to global scope, then
keep the subsequent mapfile usage and for-loop over opts_val_keys unchanged;
ensure you reference the same symbol names (opts_val_keys, opts_n_norm,
opts_y_norm, opts_m_norm, kernel_config_modifying_hashes) when updating the
declaration.

@iav iav marked this pull request as draft January 20, 2026 05:50
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from a8ff3b5 to 9b3d44d Compare January 20, 2026 18:15
@iav iav marked this pull request as ready for review January 21, 2026 02:08
@iav iav force-pushed the fix/kernel-config-hash-normalization branch 2 times, most recently from f88a095 to d2cf555 Compare January 23, 2026 06:26
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from d2cf555 to ca9e397 Compare January 24, 2026 20:22
declare -a toolchain_vars_to_hash=("${KERNEL_COMPILER}" "${toolchain_version}")
declare hash_variables="undetermined"
do_normalize_src_path="no" calculate_hash_for_variables "${toolchain_vars_to_hash[@]}"
declare toolchain_hash="${hash_variables}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we have to do a hash?
What all parses this field?
can we just say gcc12.2.0 or clang17.0.6
It just doesn't feel like there is enough entropy here to need hash compression.
@coderabbitai

Copy link
Collaborator

Choose a reason for hiding this comment

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

tabris@brunnt:~/build/armbian-build$ gcc -dumpfullversion -dumpversion
12.2.0

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find what parses or uses the artifact version string
echo "=== Searching for artifact_version usage ==="
rg -n -C3 'artifact_version' --type=sh -g '!*.md' | head -100

echo -e "\n=== Searching for common_version_suffix usage ==="
rg -n -C3 'common_version_suffix' --type=sh

echo -e "\n=== Searching for version string parsing patterns ==="
rg -n -C3 'S[0-9a-f]+-D[0-9a-f]+-P[0-9a-f]+' --type=sh

echo -e "\n=== Looking for constraints on kernel version format ==="
rg -n -C3 'Version:' lib/functions/artifacts/artifact-kernel.sh lib/functions/compilation/kernel*.sh

Length of output: 13977


@tabrisnet, good question! Based on my analysis of the codebase:

What parses the version string:
The artifact_version is used as an opaque identifier - it's not programmatically parsed by the build system. It appears in:

  • Debian package control files (Version: field)
  • U-Boot's CONFIG_LOCALVERSION (visible in boot logs)
  • Display messages and cache lookups
  • Package version mapping

Your concern is valid:
The toolchain info has very low entropy (just compiler name + major.minor version). Hashing "gcc" + "13.3" into a 4-character hash reduces human readability without providing meaningful compression benefits.

Plain text would work:
Using -Tgcc13.3 or -Tclang17.0 instead of -T${toolchain_hash_short} would be:

  • More informative for human inspection (boot logs, package lists, debugging)
  • Similar or shorter in length (7-12 chars vs 6 chars for -T{hash})
  • Valid in Debian version strings (dots are allowed)
  • Not breaking any parsing logic (since none exists)

The tradeoff:
The current approach maintains consistency with the existing pattern (all other components like -S, -D, -P use 4-char hashes). However, those components have high entropy (git SHAs, patch hashes), while toolchain has low entropy.

Recommendation:
Plain text like -Tgcc13.3 or -Tclang17.0 would be more useful for human operators inspecting versions, boot logs, or package lists. The hash compression provides no practical benefit here.

Would you like the PR author to consider switching to plain text for the toolchain component?


🧬 Code Graph Analysis Results
  • File: lib/functions/general/hash-files.sh, lines 10-28
    function calculate_hash_for_all_files_in_dirs() {
    declare -a dirs_to_hash=("$@")
    declare -a files_to_hash=()
    for dir in "${dirs_to_hash[@]}"; do

    skip if dir doesn't exist...

    if [[ ! -d "${dir}" ]]; then
    display_alert "calculate_hash_for_all_files_in_dirs" "skipping non-existent dir "${dir}"" "debug"
    continue
    fi
    mapfile -t files_in_dir < <(find -L "${dir}" -type f)
    if [[ ${#files_in_dir[@]} -eq 0 ]]; then
    display_alert "calculate_hash_for_all_files_in_dirs" "empty dir "${dir}"" "debug"
    continue
    fi
    files_to_hash+=("${files_in_dir[@]}")
    done

    calculate_hash_for_files "${files_to_hash[@]}"
    }

  • File: lib/functions/general/hash-files.sh, lines 48-82
    function calculate_hash_for_files() {
    hash_files="undetermined" # outer scope

    relativize the files to SRC

    declare -a files_to_hash=("$@")
    declare -a files_to_hash_relativized=()
    for file in "${files_to_hash[@]}"; do
    # remove the SRC/ from the file name
    file="${file#${SRC}/}"
    files_to_hash_relativized+=("${file}")
    done

    sort the array files_to_hash; use sort and readfile

    declare -a files_to_hash_sorted
    mapfile -t files_to_hash_sorted < <(for one in "${files_to_hash_relativized[@]}"; do echo "${one}"; done | LC_ALL=C sort -h) # "human" sorting

    display_alert "calculate_hash_for_files:" "files_to_hash_sorted: ${files_to_hash_sorted[*]}" "debug"

    if the array is empty, then there is nothing to hash. return 16 zeros

    if [[ ${#files_to_hash_sorted[@]} -eq 0 ]]; then
    display_alert "calculate_hash_for_files:" "files_to_hash_sorted is empty" "debug"
    hash_files="0000000000000000"
    return 0
    fi

    declare full_hash
    full_hash="$(cd "${SRC}" && sha256sum "${files_to_hash_sorted[@]}")"
    hash_files="$(echo "${full_hash}" | sha256sum | cut -d' ' -f1)" # hash of hashes
    hash_files="${hash_files:0:16}" # shorten it to 16 characters

    if [[ "${SHOW_DEBUG}" == "yes" ]]; then
    display_alert "Hash for files:" "$hash_files" "debug"
    display_alert "Full hash input for files:" "\n${full_hash}\n" "debug"
    fi
    }

  • File: lib/functions/general/hash-files.sh, lines 106-126
    function calculate_hash_for_variables() {
    hash_variables="undetermined" # outer scope
    declare -a values_to_hash=("$@")

    declare all_values="${values_to_hash[*]}" # expand...
    declare all_values_pre_normalize="${all_values}"
    if [[ "${do_normalize_src_path:-"yes"}" == "yes" ]]; then
    all_values="${all_values//${SRC}/}" # remove all occurences of ${SRC} from all_values
    fi

    hash_variables="$(echo "${all_values}" | sha256sum | cut -d' ' -f1)" # outer scope
    display_alert "calculate_hash_for_variables all_values_pre_normalize" "${all_values_pre_normalize}" "debug"
    if [[ "${do_normalize_src_path:-"yes"}" == "yes" ]]; then
    display_alert "calculate_hash_for_variables normalized" "${all_values}" "debug"
    else
    display_alert "calculate_hash_for_variables normalized?" "no" "debug"
    fi
    display_alert "calculate_hash_for_variables hashed" "${hash_variables}" "debug"

    return 0
    }

  • File: lib/functions/general/hash-files.sh, lines 31-46
    function calculate_hash_for_bash_deb_artifact() {

    the same as calculate_hash_for_files, but:

    - each param is automatically prefixed with "${SRC}/lib/functions" if it does not start with a slash

    - "compilation/packages/utils-dpkgdeb.sh" is automatically added

    declare -a deb_bash_arr=()
    for one in "$@"; do
    # if the param does not start with a slash, prefix it with "${SRC}/lib/functions", for convenience
    if [[ "${one}" != /* ]]; then
    one="${SRC}/lib/functions/${one}"
    fi
    deb_bash_arr+=("${one}")
    done
    deb_bash_arr+=("${SRC}/lib/functions/compilation/packages/utils-dpkgdeb.sh")
    calculate_hash_for_files "${deb_bash_arr[@]}"
    return 0
    }

  • File: lib/functions/general/memoize-cached.sh, lines 18-107
    function run_memoized() {
    declare var_n="${1}"
    shift
    declare cache_id="${1}"
    shift
    declare memoized_func="${1}"
    shift
    declare extra_args=("${@}")

    shellcheck disable=SC2178 # nope, that's a nameref.

    declare -n MEMO_DICT="${var_n}" # nameref

    #display_alert "memoize" "before" "info"
    #debug_dict MEMO_DICT

    MEMO_DICT+=(["MEMO_TYPE"]="${cache_id}")
    declare single_string_input="${cache_id}"
    single_string_input="$(declare -p "${var_n}")" # this might use random order...

    MEMO_DICT+=(["MEMO_INPUT_HASH"]="$(echo "${var_n}-${single_string_input}--$(declare -f "${memoized_func}")" "${extra_args[@]}" | sha256sum | cut -f1 -d' ')")

    declare disk_cache_dir="${SRC}/cache/memoize/${MEMO_DICT[MEMO_TYPE]}"
    mkdir -p "${disk_cache_dir}"
    declare disk_cache_file="${disk_cache_dir}/${MEMO_DICT[MEMO_INPUT_HASH]}"

    declare -i memoize_cache_ttl=${memoize_cache_ttl:-3600} # 1 hour default; can be overriden from outer scope

    Lock with timeout and user feedback

    exec {lock_fd}> "${disk_cache_file}.lock" || exit_with_error "failed to open lock file"

    Try non-blocking flock first

    if ! flock -n "${lock_fd}"; then
    # Lock is held by another process, inform user and wait with periodic feedback
    display_alert "Waiting for lock" "another build may be running; check: docker ps -a | grep armbian" "info"

      declare -i lock_wait_interval=${MEMOIZE_FLOCK_WAIT_INTERVAL:-10}  # seconds between retries/messages
      declare -i lock_max_wait=${MEMOIZE_FLOCK_MAX_WAIT:-0}             # 0 = infinite (default for compatibility)
      declare -i lock_total_wait=0
      declare -i lock_acquired=0
    
      while [[ "${lock_acquired}" -eq 0 ]]; do
      	# Try with timeout
      	if flock -w "${lock_wait_interval}" "${lock_fd}"; then
      		lock_acquired=1
      	else
      		lock_total_wait=$((lock_total_wait + lock_wait_interval))
      		display_alert "Still waiting for lock" "waited ${lock_total_wait}s; Ctrl+C to abort" "warn"
    
      		# Check max wait timeout (0 = infinite)
      		if [[ "${lock_max_wait}" -gt 0 && "${lock_total_wait}" -ge "${lock_max_wait}" ]]; then
      			display_alert "Lock wait timeout" "exceeded ${lock_max_wait}s; check for stale containers: docker ps -a | grep armbian" "err"
      			exit_with_error "flock() timed out after ${lock_total_wait}s - possible stale build process"
      		fi
      	fi
      done
    
      display_alert "Lock obtained after waiting" "${lock_total_wait}s" "info"
    

    else
    display_alert "Lock obtained" "${disk_cache_file}.lock" "debug"
    fi

    if [[ -f "${disk_cache_file}" ]]; then
    declare disk_cache_file_mtime_seconds
    disk_cache_file_mtime_seconds="$(stat -c %Y "${disk_cache_file}")"
    # if disk_cache_file is older than the ttl, delete it and continue.
    if [[ "${disk_cache_file_mtime_seconds}" -lt "$(($(date +%s) - memoize_cache_ttl))" ]]; then
    display_alert "Deleting stale cache file" "${disk_cache_file}" "debug"
    rm -f "${disk_cache_file}"
    else
    display_alert "Using memoized ${var_n} from ${disk_cache_file}" "${MEMO_DICT[MEMO_INPUT]}" "debug"
    display_alert "Using cached" "${var_n}" "info"
    # shellcheck disable=SC1090 # yep, I'm sourcing the cache here. produced below.
    source "${disk_cache_file}"
    return 0
    fi
    fi

    if cache miss, run the memoized_func...

    display_alert "Memoizing ${var_n} to ${disk_cache_file}" "${MEMO_DICT[MEMO_INPUT]}" "debug"
    display_alert "Producing new & caching" "${var_n}" "info"
    ${memoized_func} "${var_n}" "${extra_args[@]}"

    ... and save the output to the cache; twist declare -p's output due to the nameref

    declare -p "${var_n}" | sed -e 's|^declare -A ||' > "${disk_cache_file}"

    ... unlock.

    flock -u "${lock_fd}"

    return 0
    }


🧠 Learnings used
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.

Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.

Learnt from: EvilOlaf
Repo: armbian/build PR: 8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.

Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-17T05:09:02.306Z
Learning: In the Armbian build system, kernel patches (e.g., in patch/kernel/archive/sunxi-6.18/patches.armbian/) contain device tree overlays and other code that have existed for years and are maintained/ported across kernel versions. When reviewing PRs that modify these patch files, focus on the actual changes being made (e.g., Makefile fixes, new additions) rather than reviewing the entire existing content within the patch as if it were new code. The patch file contents are existing, stable code unless explicitly modified in the PR diff.

Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-edge.config:80-82
Timestamp: 2025-09-27T21:50:04.845Z
Learning: In the Armbian build system, kernel configuration files are generated through this automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be done this way too.
I would like to hear the opinions of other participants.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member

@rpardini rpardini Jan 24, 2026

Choose a reason for hiding this comment

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

There's a reason I've not hashed the toolchain: cache reuse. If there's a kernel that matches your config, patches, drivers, hooks, etc exactly, except for the toolchain, there's no reason not to use it.

The downside is possible module vs image matching problems (which haven't been reported for a long time, but still possible). (edit: this would happen with dkms etc, where user uses a different toolchain to build a module, vs the toolchain that built the image+other modules. there's a very high chance that the kernel a user gets via apt was cross-built, so it will never match exactly, but apparently does match "enough" so jammy/noble -> trixie still work).

I'd instead add the toolchain not to the version hash, but to the local worktree directory name. That way one can build with different toolchains more easily (switch Docker image etc) and confirm when needed.

Also: the toolchain during version determination (run in JSON matrix-producing code) could be different from the toolchain actually building it.

That said: $KERNEL_COMPILER should be a constant after certain cleanups.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh...
I got into this precisely because I needed to distinguish between the kernels built by gcc and clang in order to see the difference between them in working with dkms.

But if so, then I'd probably prefer to just throw away this piece associated with hashing.

Or, so that I can still distinguish between these kernels on my own lab — provide a place for a hook that will just add a compiler to the hash at me, but won't change the idea of cache reuse for the other world.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So this isn't disagreement per-se, but it is another reason to not use a hash if we do add it to the cache keys.

Copy link
Member

Choose a reason for hiding this comment

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

Oh... I got into this precisely because I needed to distinguish between the kernels built by gcc and clang in order to see the difference between them in working with dkms.

But if so, then I'd probably prefer to just throw away this piece associated with hashing.

Or, so that I can still distinguish between these kernels on my own lab — provide a place for a hook that will just add a compiler to the hash at me, but won't change the idea of cache reuse for the other world.

Yeah. A hash-toolchain extension could be -- it would only set a variable, and the version calculation could take that into account, and add the compiler & its version into the hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/functions/compilation/armbian-kernel.sh`:
- Around line 522-559: The mapfile/printf pipeline can produce a single
empty-string entry when arrays like opts_n, opts_y, opts_m or opts_val are
empty, which results in bogus entries like "=n" added to
kernel_config_modifying_hashes and passed to scripts/config; fix by guarding the
mapfile and downstream loops with non-empty checks (e.g. if [[ ${`#opts_n`[@]} -gt
0 ]]; then mapfile -t opts_n_norm < <(printf '%s\n' "${opts_n[@]}" | LC_ALL=C
sort); for opt_n in "${opts_n_norm[@]}"; do
kernel_config_modifying_hashes+=("${opt_n}=n"); done; fi) and do the same for
opts_y, opts_m and opts_val/opts_val_keys, and only call kernel_config_set_n /
kernel_config_set_y / kernel_config_set_m / kernel_config_set_val when the
corresponding arrays/keys are non-empty so no empty option is ever added or
passed to scripts/config.

@iav iav force-pushed the fix/kernel-config-hash-normalization branch from 1b09619 to 0a15028 Compare January 24, 2026 22:46
@github-actions github-actions bot added size/small PR with less then 50 lines and removed size/medium PR with more then 50 and less then 250 lines labels Jan 24, 2026
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from 0a15028 to e564229 Compare January 25, 2026 02:02
@iav iav requested a review from igorpecovnik as a code owner January 25, 2026 02:02
@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines and removed size/small PR with less then 50 lines labels Jan 25, 2026
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from e564229 to 823d9cd Compare January 25, 2026 17:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/functions/artifacts/artifact-kernel.sh`:
- Around line 193-201: The assembled common_version_suffix can be empty, but
artifact_version is later built by appending a dash regardless, producing a
trailing '-' (invalid Debian version). Update the artifact_version assembly to
only append the "-" and common_version_suffix when common_version_suffix is
non-empty; reference the variables artifact_version, common_version_suffix,
artifact_version_parts and artifact_version_part_order to locate the logic and
guard the concatenation so no dash is added when the suffix is empty.

@iav iav force-pushed the fix/kernel-config-hash-normalization branch from 823d9cd to 709cba2 Compare January 25, 2026 18:11
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from 709cba2 to 76c5db6 Compare January 28, 2026 02:11
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/functions/artifacts/artifact-kernel.sh`:
- Around line 130-136: The dedup pipeline using tac | sort -uk is not stable, so
it may not keep the last assignment per key; update the command that populates
kernel_config_modifying_hashes_reduced to use a stable sort (preserve input
order for equal keys) by adding the -s flag (e.g., printf '%s\n'
"${kernel_config_modifying_hashes[@]}" | tac | LC_ALL=C sort -s -t '=' -uk 1,1),
leaving the tac and subsequent hash computation
(kernel_config_modification_hash) intact so the final sha256 is computed from
the correctly reduced list.

@iav iav force-pushed the fix/kernel-config-hash-normalization branch 3 times, most recently from d7d4008 to 6e7951d Compare February 1, 2026 18:26
Copy link
Member

@rpardini rpardini left a comment

Choose a reason for hiding this comment

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

This is nice, thanks. Lets have it and run some builds

@github-actions github-actions bot removed the Needs review Seeking for review label Feb 1, 2026
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from 6e7951d to c6e8200 Compare February 2, 2026 16:20
@github-actions github-actions bot added the Needs review Seeking for review label Feb 2, 2026
iav and others added 3 commits February 3, 2026 23:20
Reduce kernel_config_modifying_hashes to last assignment per key before
hashing, so that overridden config options do not cause unnecessary
cache invalidation. Uses tac|sort to implement last-value-wins
deduplication.

Co-Authored-By: tabrisnet <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace single-line version suffix assembly with an extensible two-array
approach: artifact_version_parts (associative, key=value) and
artifact_version_part_order (indexed, "NNNN-KEY" for sortable insertion).

Extensions can add, modify, or remove parts via the
artifact_kernel_version_parts hook. Keys starting with "_" are
internal-only and not prefixed in the output.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add opt-in extension that includes gcc/clang major.minor version in the
kernel artifact version string for cache invalidation when the toolchain
changes. Enable with ENABLE_EXTENSIONS="kernel-version-toolchain".

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@iav iav force-pushed the fix/kernel-config-hash-normalization branch from c6e8200 to f4e26e0 Compare February 3, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First 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.

3 participants