extension: add ksrc packaging #8211
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new shell script extension has been added to facilitate packaging the Linux kernel source into a Debian package. The script introduces three primary functions: one to set configuration variables for building the kernel source package, another to add the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
05
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
extensions/linux-source-package.sh (3)
1-1: Add a shebang or shell directive
Specify the interpreter to satisfy ShellCheck (SC2148) and ensure consistent execution. For example, add at the top:+#!/usr/bin/env bash🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
19-21: Scope variables locally and quote values
Uselocalfor function-scoped variables and quote interpolations for safety:-declare kernel_work_dir="${SRC}/cache/sources/${LINUXSOURCEDIR}" -declare CHOSEN_KSRC=linux-source-${BRANCH}-${LINUXFAMILY} +local kernel_work_dir="${SRC}/cache/sources/${LINUXSOURCEDIR}" +local CHOSEN_KSRC="linux-source-${BRANCH}-${LINUXFAMILY}"
31-33: Quote paths and handle failures when compressing config
Ensure file paths are quoted and errors are propagated:-cp "${SRC}/config/kernel/${LINUXCONFIG}.config" "default_${LINUXCONFIG}.config" -xz < ${kernel_work_dir}/.config > "${sources_pkg_dir}/usr/src/${LINUXCONFIG}_${version}_${REVISION}_config.xz" +cp "${SRC}/config/kernel/${LINUXCONFIG}.config" "default_${LINUXCONFIG}.config" || exit 1 +xz < "${kernel_work_dir}/.config" > "${sources_pkg_dir}/usr/src/${LINUXCONFIG}_${version}_${REVISION}_config.xz" || exit 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/linux-source-package.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
extensions/linux-source-package.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 3-3: ARTIFACT_IGNORE_CACHE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 4-4: KERNEL_GIT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 16-16: kernel_version_family is referenced but not assigned.
(SC2154)
[warning] 25-25: ret is referenced but not assigned.
(SC2154)
[warning] 25-25: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
[warning] 28-28: version is referenced but not assigned.
(SC2154)
🔇 Additional comments (2)
extensions/linux-source-package.sh (2)
3-4: Verify global variables are used by the build system
ShellCheck flagsARTIFACT_IGNORE_CACHEandKERNEL_GITas unused (SC2034). Confirm these globals are consumed downstream or export/remove them accordingly.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: ARTIFACT_IGNORE_CACHE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 4-4: KERNEL_GIT appears unused. Verify use (or export if used externally).
(SC2034)
53-58: VerifyDEB_STORAGEand enforce error propagation
Ensure that${DEB_STORAGE}is defined and writable sorsyncdoesn't fail silently. Also, make surefakeroot dpkg-deberrors halt the build (e.g., viaset -eor|| exit 1).
| local sources_pkg_dir tmp_src_dir | ||
| tmp_src_dir=$(mktemp -d) | ||
| trap "ret=\$?; rm -rf \"${tmp_src_dir}\" ; exit \$ret" 0 1 2 3 15 | ||
| sources_pkg_dir=${tmp_src_dir}/${CHOSEN_KSRC}_${REVISION}_all | ||
| mkdir -p "${sources_pkg_dir}"/usr/src/ \ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve trap quoting and exit handling
The trap uses double quotes causing premature expansion and references ret before assignment (SC2064, SC2154). A more robust pattern is:
-trap "ret=\$?; rm -rf \"${tmp_src_dir}\" ; exit \$ret" 0 1 2 3 15
+trap 'rc=$?; rm -rf "'"${tmp_src_dir}"'"; exit $rc' 0 1 2 3 15Alternatively, simplify to:
trap 'rm -rf "${tmp_src_dir}"' EXIT INT TERM🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 25-25: ret is referenced but not assigned.
(SC2154)
[warning] 25-25: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
🤖 Prompt for AI Agents
In extensions/linux-source-package.sh around lines 23 to 27, the trap command
uses double quotes causing premature variable expansion and references the
variable ret before it is assigned. To fix this, replace the double quotes with
single quotes in the trap command to delay expansion, or simplify the trap to
just remove the temporary directory on EXIT, INT, and TERM signals without
capturing the exit code. For example, use trap 'rm -rf "${tmp_src_dir}"' EXIT
INT TERM to ensure proper cleanup without premature expansion or undefined
variable usage.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
In the previous branch, the package contained only the archive and additional actions were required locally on the system to make it work. I'll see what I can do about it. |
igorpecovnik
added
Work in progress
Description
A (very poorly made) attempt to address #8161
It spits out some sort of kernel source package, but I think
{version}is still missing since I couldn't get it to work and probably needs more fixes until production-ready. I probably won't go further here due to lacking expertise in framework and bash foo.Maybe someone want to pick it up from here.
Also to do: Re-enable
BUILD_KSRCswitch to enable the extension.How Has This Been Tested?
Checklist: