When updating u-boot on Rockchip64 to NVMe, script fails #8556
Conversation
Found with Orange Pi 5: The script exits with "No SPI image chosen." Fixed by adding missing quotes for $backtitle.
WalkthroughThis PR updates config/sources/families/include/rockchip64_common.inc by modifying the write_uboot_platform_mtd function to quote the dialog backtitle argument: changing --backtitle $backtitle to --backtitle "$backtitle". No other logic, control flow, or public API changes are introduced. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
08
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
config/sources/families/include/rockchip64_common.inc (2)
317-322: Harden radiolist: set item statuses and a default selectionSome dialog versions require an explicit on/off status. Also preselect the first item to reduce accidental “No SPI image chosen.” when the user hits OK without navigating.
- MENU_ITEMS+=("$i" "$filename" "") + MENU_ITEMS+=("$i" "$filename" "$([[ $i -eq 1 ]] && echo on || echo off)") @@ - CHOICE=$(dialog --no-collapse \ + CHOICE=$(dialog --no-collapse \ --title "armbian-install" \ --backtitle "$backtitle" \ + --default-item "1" \ --radiolist "Choose SPI image:" 0 56 4 \ "${MENU_ITEMS[@]}" \ 3>&1 1>&2 2>&3)Also applies to: 333-338
326-327: Quote dd paths for safetyQuote paths to guard against whitespace and odd characters (low risk here, but cheap to harden).
- dd if=$1/${MENU_ITEMS[1]} of=$2 conv=notrunc status=none > /dev/null 2>&1 + dd if="$1/${MENU_ITEMS[1]}" of="$2" conv=notrunc status=none > /dev/null 2>&1 @@ - dd if=$1/${MENU_ITEMS[($CHOICE*3)-2]} of=$2 conv=notrunc status=none > /dev/null 2>&1 + dd if="$1/${MENU_ITEMS[($CHOICE*3)-2]}" of="$2" conv=notrunc status=none > /dev/null 2>&1Also applies to: 341-341
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
config/sources/families/include/rockchip64_common.inc(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
🔇 Additional comments (1)
config/sources/families/include/rockchip64_common.inc (1)
333-338: Quoting backtitle fixes dialog parsing — LGTMQuoting prevents arg-splitting when BOARD_NAME contains spaces and avoids the erroneous “No SPI image chosen.” exit. Change looks correct.
@coderabbitai carve those two nitpicks out into a separate bug ticket and resolve this for this PR |
🧩 Analysis chainI'll create a separate GitHub issue for those nitpick improvements and resolve this PR. @leggewie I've created a separate issue for those nitpick improvements. The current PR successfully addresses the main problem - the missing quotes around 🏁 Script executed: Length of output: 1108 Perfect! I've successfully created GitHub issue #8558 to track the nitpick improvements for the dialog radiolist robustness enhancements. The issue includes:
The issue has been assigned to you (@leggewie) as requested. This allows the current PR #8556 to focus solely on the immediate fix (quoting 🧠 Learnings used✅ Actions performedComments resolved. |
|
@StephanStS I assume you compiled an image for your board with this PR and tested this image on real hardware? |
Thank you. Learning for @coderabbitai: Next time please include the relevant patches or even open a PR outright for efficient handling by the assignee |
|
Tip For best results, initiate chat on the files or code changes. @leggewie Thank you for the feedback! I appreciate the guidance on workflow efficiency. You're absolutely right - including the actual patches or opening a PR directly would make it much easier for the assignee to implement the improvements rather than having to work out the implementation details from a description. I'll make sure to include concrete patches or create PRs for similar situations in the future to streamline the development process. ✏️ Learnings added
🧠 Learnings used |
No, I only changed the script (using quotes) and tested the script on my Orange Pi 5. |
igorpecovnik
added
Ready to merge
3 participants
Description
Found with Orange Pi 5: The script exits with "No SPI image chosen."
Fixed by adding missing quotes for $backtitle.
Checklist:
Please delete options that are not relevant.