Skip to content

kernel: run kernel_copy_extra_sources hooks after kernel_maybe_clean#9830

Closed
rpardini wants to merge 1 commit into
armbian:mainfrom
rpardini:pr/kernel-run-kernelcopyextrasources-hooks-after-kernelmaybeclean
Closed

kernel: run kernel_copy_extra_sources hooks after kernel_maybe_clean#9830
rpardini wants to merge 1 commit into
armbian:mainfrom
rpardini:pr/kernel-run-kernelcopyextrasources-hooks-after-kernelmaybeclean

Conversation

@rpardini
Copy link
Copy Markdown
Member

@rpardini rpardini commented May 15, 2026

  • 🌵 otherwise changes done to kernel dir go away if cleaning is requested, which has been forcing family and board code to use unrelated hooks to workaround it, which in turn requires causes CONFIG_DEFS_ONLY=yes hacks which then causes hashes to differ from actual build runs
  • 🐸 with this, now any source-level changes (incl generating patches) can now be done using kernel_copy_extra_sources; make sure to include all hashing-related code and references (eg: source SHA1 from external repos) into that hook, so changes there cause a change to the -HK
  • 🍃 Fixes: c47c937 "Add new hook to allow copying code into kernel"

Summary by CodeRabbit

  • Chores
    • Optimized kernel compilation build process order for improved consistency in the build workflow.

Review Change Stack

- otherwise changes done to kernel dir go away if cleaning is requested,
  which has been forcing family and board code to use unrelated hooks to
  workaround it, which in turn requires causes CONFIG_DEFS_ONLY=yes hacks
  which then causes hashes to differ from actual build runs
- with this, now any source-level changes (incl generating patches) can
  now be done using `kernel_copy_extra_sources`; make sure to include
  all hashing-related code and references (eg: source SHA1 from external
  repos) into that hook, so changes there cause a change to the `-HK`
- Fixes: c47c937 "Add new hook to allow copying code into kernel"
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12c0e597-d4f6-447b-8772-da7eba4f4d08

📥 Commits

Reviewing files that changed from the base of the PR and between 5e60c01 and 9a4e580.

📒 Files selected for processing (1)
  • lib/functions/compilation/kernel.sh

📝 Walkthrough

Walkthrough

The PR reorders two build preparation steps in the kernel compilation function. The conditional clean operation now executes before copying extra kernel sources, rather than after. This changes the sequence of kernel tree preparation without modifying function signatures or logic.

Changes

Kernel Build Preparation Order

Layer / File(s) Summary
Kernel build preparation order
lib/functions/compilation/kernel.sh
kernel_maybe_clean is moved to execute before kernel_copy_extra_sources in the compile_kernel function, reversing their previous order. The clean step is wrapped with logging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A rabbit hopped through the build, so keen,
Cleaning the kernel before sources are seen,
Reordered the steps with careful delight,
Making the compile flow orderly and right!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: reordering kernel build steps so copy_extra_sources runs after kernel_maybe_clean, which directly matches the changeset modification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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/small PR with less then 50 lines Needs review Seeking for review Framework Framework components 05 Milestone: Second quarter release labels May 15, 2026
@rpardini rpardini marked this pull request as ready for review May 15, 2026 11:16
@rpardini rpardini requested a review from a team as a code owner May 15, 2026 11:16
@rpardini rpardini requested review from ShravanSingh64 and matthijskooijman and removed request for a team May 15, 2026 11:16
@iav
Copy link
Copy Markdown
Contributor

iav commented May 15, 2026

The reorder only solves half of the problem. After kernel_maybe_clean, the next call in compile_kernel is kernel_main_patching (lib/functions/compilation/kernel.sh:54), which performs two more full reset+clean cycles on the kernel work tree:

  1. kernel_drivers_create_patcheskernel_drivers_prepare_harness (lib/functions/compilation/patch/drivers-harness.sh:144-146):

    run_host_command_logged git reset --hard "${kernel_git_revision}"
    run_host_command_logged git clean -fd # no -x here

    Wipes untracked files not in .gitignore.

  2. kernel_main_patching_python (lib/functions/compilation/kernel-patching.sh:73-90) — passes BASE_GIT_REVISION=${kernel_git_revision} to lib/tools/patching.py, and the comment in the code itself says:

    # Python patching will git reset to the kernel SHA1 git revision, and remove all untracked files.

    So a second reset+clean, this time effectively with -x semantics, wiping everything untracked including gitignored paths.

With this PR the pipeline becomes:

  1. kernel_maybe_cleangit clean -xfdq (when CLEAN_LEVEL=make-kernel)
  2. kernel_copy_extra_sources — drops files into the kernel tree ← new placement
  3. kernel_main_patching → two additional reset+clean passes → the just-copied files are removed
  4. configure + build — no extra sources present

Concrete example: kernel_copy_extra_sources__khadas_common_drivers creates a common_drivers/ directory under the kernel tree; even if it is gitignored and survives git clean -fd in step 3.1, the Python patcher's reset+clean in step 3.2 will still delete it.

Proposed fix: invoke kernel_copy_extra_sources after kernel_main_patching, not between clean and patching. Then the sequence becomes:

  1. clean (optional)
  2. patching (git reset --hard + git clean -fdx + apply patches)
  3. kernel_copy_extra_sources
  4. configure + build

In this ordering the copy survives all the way to the build, and the hash-folding responsibility you call out in the commit message (extension authors must fold the source SHA into the artifact hash) remains intact.

Surfaced by Codex review on our mirror PR iav/armbian#121.

@rpardini
Copy link
Copy Markdown
Member Author

Indeed. I had not fully considered this, despite having written it that way. Wifi-drivers overcome this with a special exception. I'll reconsider...

@rpardini rpardini closed this May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

05 Milestone: Second quarter release Framework Framework components Needs review Seeking for review size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

2 participants