-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Improve modules compilation success ratio #9026
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.
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
Conversation
WalkthroughAdded Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
|
Confirm While this is fine, we also need to add pahole to the Different modules might also need more kernel tools built (eg |
|
@coderabbitai awake - open separate Task issues about adding |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@rpardini: I'll open two separate Task issues for you. 🏁 Script executed: Length of output: 151 @rpardini: I don't have GitHub authentication in this environment to create issues directly. Here are the two issues ready for you to create: Issue 1: Add pahole as dependency to linux-headers package RequiresTitle: Add pahole as dependency to linux-headers package Requires Labels: Task Assignee: rpardini Body: ## Context
As discussed in PR #9026, `pahole` (from the dwarves package) is required for working kernel headers where the header's .config matches /boot/config-.
## Task
Add `pahole` to the `Requires:` field of the kernel headers .deb package to ensure it's automatically installed as a dependency.
## References
- PR: https://github.com/armbian/build/pull/9026
- Related issue: https://github.com/hack3ric/mimic/issues/27
- Requested by: @rpardini
## Additional Context
Different modules (like ZFS) require pahole to successfully compile against kernel headers. Making it an explicit dependency ensures users don't encounter build failures.Issue 2: Add Linux tools/source to build resolve_btfids during linux-headers postinstTitle: Add Linux tools/source to build resolve_btfids during linux-headers postinst Labels: Task Assignee: rpardini Body: ## Context
As discussed in PR #9026, different kernel modules might need additional kernel tools built (e.g., `resolve_btfids`) for full functionality. This is typically handled in the postinst of kernel-headers.
## Task
Add enough Linux tools/source to be able to build `resolve_btfids` during linux-headers postinst.
## References
- PR: https://github.com/armbian/build/pull/9026
- Related issue: https://github.com/hack3ric/mimic/issues/27
- Requested by: @rpardini
## Additional Context
This ensures that kernel modules requiring BTF ID resolution can be built successfully against the installed kernel headers.You can create these issues manually through the GitHub UI or with your local 🧠 Learnings used |
Added both, adjusted PR title. We might even skip it from the base packages if its added to headers as dependency. |
|
✅ This PR has been reviewed and approved — all set for merge! |
|
Maybe a Rebase to clear up commit history |
or just do a squash merge. might be less effort |
rpardini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. What I wonder is, "will any of the vendor/old kernels fail due to lack of resolve_btfids?" but I guess that's a job for the CI to detect.
I wouldn't squash, btw.
Description
Adding pahole as headers dependency and add compilation of resolve_btfids tools when installing kernel header package.
Ref: https://forum.armbian.com/topic/52941-gnulinkoncethis_module-section-size-must-match-the-kernels-built-struct-module-size-at-run-time/
@leggewie This has to land also to APA, but I am not in sync with recent development. This package should be in minimal image.
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.