bsp: use kernel version argument in ABL postinst scripts (fixes #10108) - #10109
#10109bsp: use kernel version argument in ABL postinst scripts (fixes #10108)#10109igorpecovnik merged 2 commits intoarmbian:mainarmbian/build:mainfrom rorystandley:fix-abl-postinst-kernel-version-10108rorystandley/build:fix-abl-postinst-kernel-version-10108Copy head branch name to clipboard
Conversation
The zz-update-abl-kernel postinst hooks located kernel, initrd and dtb paths via shell wildcards (/boot/vmlinuz-*-sm8250 etc). Once a second kernel version is installed, the globs expand to multiple files: - mkbootimg receives multiple ramdisk arguments and aborts with "ValueError: Unrecognized arguments", failing the kernel postinst - gzip/cat silently concatenate multiple kernels and dtbs into the boot image Debian kernel hooks pass the kernel version as $1, so use it to build exact paths. When invoked manually without the argument, fall back to the newest installed kernel (sort -V). Fail early with a clear error if the kernel or ramdisk is missing. Initialise extraargs and abl_boot_partition_label before sourcing armbianEnv.txt to satisfy the shellcheck lint gate (SC2154). Applies to all six ABL BSPs: xiaomi-elish, oneplus-kebab, ayn-thor, ayn-odin2, ayn-odin2mini, ayn-odin2portal. Fixes: armbian#10108
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughSix board-specific ChangesABL Kernel Script Version Resolution
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 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 |
Hey @rorystandley! 👋Thanks for submitting your first pull request to the Armbian project — we're excited to have you contributing! 🧡 If you'd like to stay informed about project updates or collaborate more closely with the team, Also, don’t forget to ⭐ star the repo if you haven’t already — and welcome aboard! 🚀 |
|
Ready for review - could someone add the "needs review" label? I can't set it myself. Thanks |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bsp/ayn-odin2/zz-update-abl-kernel (1)
3-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffVersion resolution and validation logic looks correct.
Falls back to the latest installed kernel via
sort -V, and fails fast with clear messages if kernel/ramdisk are missing. This resolves the wildcard-expansion problem described in the PR.One maintainability note: this exact ~17-line block (only the SoC suffix differs) is duplicated verbatim across all six
zz-update-abl-kernelscripts in this PR. Consider extracting it into a shared snippet/function sourced by each board script to avoid future drift (e.g., a bug fix applied to one board getting missed in the others).🤖 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 `@packages/bsp/ayn-odin2/zz-update-abl-kernel` around lines 3 - 19, The version resolution and validation block in the zz-update-abl-kernel script is duplicated across multiple board variants, which will make future fixes easy to miss. Extract this shared logic into a reusable helper or sourced snippet, then have each board-specific script call it while only keeping the SoC-specific kernel pattern or identifiers local. Keep the existing version="$1", fallback lookup with sort -V, and kernel/ramdisk validation behavior centralized so all scripts stay in sync.
🤖 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 `@packages/bsp/ayn-odin2/zz-update-abl-kernel`:
- Around line 3-19: The version resolution and validation block in the
zz-update-abl-kernel script is duplicated across multiple board variants, which
will make future fixes easy to miss. Extract this shared logic into a reusable
helper or sourced snippet, then have each board-specific script call it while
only keeping the SoC-specific kernel pattern or identifiers local. Keep the
existing version="$1", fallback lookup with sort -V, and kernel/ramdisk
validation behavior centralized so all scripts stay in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37bb7758-8c84-41f5-b71f-f107080b308e
📒 Files selected for processing (6)
packages/bsp/ayn-odin2/zz-update-abl-kernelpackages/bsp/ayn-odin2mini/zz-update-abl-kernelpackages/bsp/ayn-odin2portal/zz-update-abl-kernelpackages/bsp/ayn-thor/zz-update-abl-kernelpackages/bsp/oneplus-kebab/zz-update-abl-kernelpackages/bsp/xiaomi-elish/zz-update-abl-kernel
|
Re the CodeRabbit nitpick about the duplicated version-resolution block: it's a fair point, but these hooks are standalone scripts shipped to each device's rootfs (the board configs install them into |
|
✅ This PR has been reviewed and approved — all set for merge! |
Description
The
zz-update-abl-kernelpostinst hooks located kernel, initrd and dtb paths via shell wildcards (/boot/vmlinuz-*-sm8250etc.). Once a second kernel version is installed, the globs expand to multiple files:mkbootimgreceives multiple ramdisk arguments and aborts withValueError: Unrecognized arguments, failing the kernel postinst and blocking kernel upgradesgzip/catsilently concatenate multiple kernels and dtbs into the boot imageDebian kernel hooks in
/etc/kernel/postinst.d/receive the kernel version as$1, so the scripts now use it to build exact paths. When invoked manually without the argument, they fall back to the newest installed kernel (sort -V, so e.g. 6.18.35 ranks above 6.9.1). The scripts now also fail early with a clear error if the kernel or ramdisk image is missing.All six ABL BSP scripts get the same fix for consistency: xiaomi-elish, oneplus-kebab, ayn-thor, ayn-odin2, ayn-odin2mini, ayn-odin2portal. Note that only xiaomi-elish and oneplus-kebab currently install the hook via their board config (
install -Dm655 ... $destination/etc/kernel/postinst.d/inconfig/boards/*.conf); the AYN scripts carry the same latent bug.Lint-driven cleanup:
extraargsandabl_boot_partition_labelare initialised beforesource /boot/armbianEnv.txtin xiaomi-elish and oneplus-kebab to satisfy the shellcheck gate (SC2154, flagged now that these files are touched). Behaviour-safe: the scripts already guard on[ -n "$abl_boot_partition_label" ], and this also stops stray environment values leaking into the boot image cmdline.GitHub issue reference: #10108
Jira reference number: N/A — community bugfix
How Has This Been Tested?
bash -non all six scriptsshellcheck --shell=bash --severity=warning --exclude=SC2034,SC2207,SC2046,SC2086,SC2206) — all six clean/bootsimulation with three installed versions (6.9.1 / 6.18.10 / 6.18.35): the old glob expands to 3 files; with the fix, the hook-argument path resolves exactly one kernel/initrd pair, and the no-argument fallback picks 6.18.35 (newest bysort -V), not 6.9.1 (lexical winner)Checklist:
Summary by CodeRabbit