fix(kernel-config): append missing newline to .config before edits#9681
fix(kernel-config): append missing newline to .config before edits#9681igorpecovnik merged 2 commits intomainfrom
Conversation
Kernel's ./scripts/config appends via `echo >>`; configs without a trailing newline end up with the first new option glued to the last line, silently dropping subsequent custom_kernel_config hooks. Fixes: #9680
iav
requested review from
catalinii and
martinayotte
and removed request for
a team
📝 WalkthroughWalkthroughAdded a trailing-newline Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
bot
added
size/small
kernel_config_modifying_hashes was declared locally in
artifact_kernel_prepare_version(), and the kernel_config_set_{y,n,m,string,val}
helpers never wrote to it. Hooks that used those helpers (e.g.
custom_kernel_config) would be applied to .config but invisible to
the kernel-artifact cache fingerprint, so a cached artifact built
without the hook could be reused for a later build that added one.
Declare the array globally, make every setter append its change, and
initialize the array in call_extensions_kernel_config as a safety net
for non-artifact call paths.
Refs: #9680
Co-authored-by: EvilOlaf <50047739+EvilOlaf@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/functions/compilation/armbian-kernel.sh (1)
693-730: Note: Duplicate hash entries when using opts_ arrays (harmless).*With the new hash tracking in
kernel_config_set_*helpers, options processed viaopts_*arrays are now recorded twice:
- Explicitly at lines 697-711 (first pass)
- Again when
kernel_config_set_*is called at lines 715-729 (second pass)This is harmless because:
- The deduplication step in
artifact-kernel.sh(lines 133-135) usessort -uk 1,1to keep only one entry per key- During version calculation,
.configdoesn't exist, so the second pass doesn't run—no duplicates- During actual builds, duplicates occur but the hash has already been calculated
No action needed; just flagging for awareness. If you want to eliminate the redundancy, you could remove the explicit additions in the first pass and rely solely on the helper functions, but the current approach is clearer and the overhead is negligible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/functions/compilation/armbian-kernel.sh` around lines 693 - 730, The first pass in armbian_kernel_config_apply_opts_from_arrays is redundantly appending entries to kernel_config_modifying_hashes for opts_n/opts_y/opts_m/opts_val and then the kernel_config_set_* helpers append them again; to eliminate duplicate hash entries remove the explicit first-pass loops that do kernel_config_modifying_hashes+=("${...}") for opts_n, opts_y, opts_m and the opts_val loop, and rely on kernel_config_set_n/kernel_config_set_y/kernel_config_set_m/kernel_config_set_val to record changes, keeping the second-pass application of .config intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/functions/compilation/armbian-kernel.sh`:
- Around line 693-730: The first pass in
armbian_kernel_config_apply_opts_from_arrays is redundantly appending entries to
kernel_config_modifying_hashes for opts_n/opts_y/opts_m/opts_val and then the
kernel_config_set_* helpers append them again; to eliminate duplicate hash
entries remove the explicit first-pass loops that do
kernel_config_modifying_hashes+=("${...}") for opts_n, opts_y, opts_m and the
opts_val loop, and rely on
kernel_config_set_n/kernel_config_set_y/kernel_config_set_m/kernel_config_set_val
to record changes, keeping the second-pass application of .config intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc2c93eb-3605-4bca-acfa-a13a7e4ec479
📒 Files selected for processing (3)
lib/functions/artifacts/artifact-kernel.shlib/functions/compilation/armbian-kernel.shlib/functions/compilation/kernel-config.sh
First pass is load-bearing for |
|
The author of the original bug report closed it, but if I understand what's happening correctly, it still makes sense to fix it. Invalidating the cache is the right thing to do. |
|
✅ This PR has been reviewed and approved — all set for merge! |
2 participants
Summary
Issue #9680 report:
kernel_config_set_*calls fromcustom_kernel_confighooks silently lost, "fixed" by adding a blank line to the defconfig. Two independent bugs combine into this behaviour:Text-level (commit 1):
./scripts/config --enable Xappends new options viaecho >>. A defconfig shipped without a trailing newline ends up with the first new option glued onto the last line (# CONFIG_OLDOPT is not setCONFIG_NEWOPT=y), whichmake olddefconfigthen discards. 93 files inconfig/kernel/*.configcurrently lack a trailing newline. Normalise.configonce after copying.Cache-level (commit 2):
kernel_config_modifying_hasheswasdeclare -alocal toartifact_kernel_prepare_version(), and thekernel_config_set_{y,n,m,string,val}helpers never appended to it. Hook-driven config changes never reached the cache fingerprint, so a cached artifact built without the hook got reused even after the hook was added. Declare the array globally, push from every setter, and initialise it defensively incall_extensions_kernel_config. Co-authored with EvilOlaf, who identified the cache path in the issue thread.Fixes #9680
Test plan
scripts/config --enable NEWon a no-trailing-newline input produces# CONFIG_OLDOPT is not setCONFIG_NEWOPT=y).BOARD=nanopi-r6s BRANCH=vendor(useslinux-rk35xx-vendor.config, the exact defconfig from the issue report) withARTIFACT_IGNORE_CACHE=yes.Summary by CodeRabbit
Release Notes