Extension: UFS for image generation pipeline #8474
Open
+14
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
08
WalkthroughA new shell function, Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
added
05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
extensions/ufs.sh (4)
9-11: Version gating logic looks good; consider slightly tighter messaging.Comparison via linux-version is correct. Minor wording nit in message to align with package naming.
- exit_with_error "UFS extension requires sfdisk >= 2.41 (from util-linux). Current version: ${sfdisk_version}" + exit_with_error "UFS extension requires util-linux (sfdisk) >= 2.41. Detected: ${sfdisk_version}"
12-12: Avoid duplicate “-ufs” suffix if the hook runs more than once.Be defensive and only append if not present.
- EXTRA_IMAGE_SUFFIXES+=("-ufs") + if [[ " ${EXTRA_IMAGE_SUFFIXES[*]} " != *" -ufs "* ]]; then + EXTRA_IMAGE_SUFFIXES+=("-ufs") + fi
13-13: Don’t overwrite SECTOR_SIZE if already set; keep it global.Respect an existing value and still ensure global scope.
- declare -g SECTOR_SIZE=4096 + if [[ -z ${SECTOR_SIZE-} ]]; then + declare -g SECTOR_SIZE=4096 + fi
1-1: Clarify the host requirement wording.This is a util-linux (sfdisk) 2.41+ requirement; Debian 13 (Trixie) is an example host that satisfies it.
-# Create UFS aligned image (requires >= Debian 13 (Trixie) Host) +# Create UFS-aligned image (requires util-linux/sfdisk >= 2.41; e.g., Debian 13 (Trixie) or newer)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/ufs.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: EvilOlaf
PR: armbian/build#8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In the Armbian build system's kernel configuration, when CONFIG_EXT2_FS=y and CONFIG_EXT3_FS=y are both set to built-in, EXT4 filesystem support becomes automatically available/hard-coded without requiring an explicit CONFIG_EXT4_FS line in the configuration file. This is specific to how Armbian handles kernel configuration and differs from typical standalone Linux kernel configuration behavior.
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Applied to files:
extensions/ufs.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/ufs.sh
📚 Learning: 2025-07-27T13:03:53.146Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:2664-2671
Timestamp: 2025-07-27T13:03:53.146Z
Learning: In the Armbian build system's kernel configuration, when CONFIG_EXT2_FS=y and CONFIG_EXT3_FS=y are both set to built-in, EXT4 filesystem support becomes automatically available/hard-coded without requiring an explicit CONFIG_EXT4_FS line in the configuration file. This is specific to how Armbian handles kernel configuration and differs from typical standalone Linux kernel configuration behavior.
Applied to files:
extensions/ufs.sh
📚 Learning: 2025-06-22T00:45:26.182Z
Learnt from: djurny
PR: armbian/build#8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Applied to files:
extensions/ufs.sh
Comment on lines
+5
to
+8
| sfdisk_version=$(sfdisk --version | awk '/util-linux/ {print $NF}') | ||
| if [[ -z "${sfdisk_version}" ]]; then | ||
| exit_with_error "sfdisk not found - please install util-linux / fdisk >= 2.41 package" | ||
| fi |
There was a problem hiding this comment.
Guard against errexit on missing sfdisk; use a presence check before version parsing.
With errexit enabled globally, sfdisk --version inside $(...) will terminate the shell if sfdisk is absent. Guard presence first and sanitize version parsing.
- sfdisk_version=$(sfdisk --version | awk '/util-linux/ {print $NF}')
- if [[ -z "${sfdisk_version}" ]]; then
- exit_with_error "sfdisk not found - please install util-linux / fdisk >= 2.41 package"
- fi
+ if ! command -v sfdisk >/dev/null 2>&1; then
+ exit_with_error "sfdisk not found. Please install util-linux (provides sfdisk) >= 2.41."
+ fi
+ # Extract the util-linux version and strip any non-numeric characters for robustness
+ sfdisk_version="$(sfdisk --version 2>/dev/null | awk '/util-linux/ {print $NF}' | tr -cd '0-9.')"
+ if [[ -z "${sfdisk_version}" ]]; then
+ exit_with_error "Unable to determine util-linux version from 'sfdisk --version'."
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| sfdisk_version=$(sfdisk --version | awk '/util-linux/ {print $NF}') | |
| if [[ -z "${sfdisk_version}" ]]; then | |
| exit_with_error "sfdisk not found - please install util-linux / fdisk >= 2.41 package" | |
| fi | |
| if ! command -v sfdisk >/dev/null 2>&1; then | |
| exit_with_error "sfdisk not found. Please install util-linux (provides sfdisk) >= 2.41." | |
| fi | |
| # Extract the util-linux version and strip any non-numeric characters for robustness | |
| sfdisk_version="$(sfdisk --version 2>/dev/null | awk '/util-linux/ {print $NF}' | tr -cd '0-9.')" | |
| if [[ -z "${sfdisk_version}" ]]; then | |
| exit_with_error "Unable to determine util-linux version from 'sfdisk --version'." | |
| fi |
🤖 Prompt for AI Agents
In extensions/ufs.sh around lines 5 to 8, the script runs `sfdisk --version`
directly which causes the shell to exit if sfdisk is missing due to errexit. To
fix this, first check if the sfdisk command exists using a command presence
check like `command -v sfdisk` before attempting to parse its version. This
prevents the script from exiting prematurely and allows for a proper error
message if sfdisk is not installed.
|
" Correctly: Not working on Ubuntu Noble with 2.39 " Am I reading this correctly, your extension does not work on Ubuntu Noble compile host? That would be a dealbreaker IMHO. |
here it isn’t. The code regarding armbian is already merged. This Extension just makes it "easier" to integrate into CI for automated UFS builds. Also the code in the util-linux tools is comparatively very new that’s why it just isn’t available in pre Debian 13 distro's. I tried opening an issue with Canonical for Noble but they’re not interested in backporting. |
igorpecovnik
approved these changes
Aug 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This extension checks if the installed sfdisk version supports declaring sector sizes (version 2.41 or newer) and sets up configuration variables needed to create a UFS-aligned images
Related to #8053
How Has This Been Tested?
Checklist: