Merge Yellow's RAUC post-install hook with the install hook #3678
Conversation
As stated in the docs [1], post-install hook is not executed if the slot already has an install hook defined. Merge the post-install hook with the install hook to fix CM5 migration for Yellow. [1] https://rauc.readthedocs.io/en/latest/using.html#slot-hooks
📝 WalkthroughWalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RAUC
participant Config
participant Boot
User->>RAUC: Initiate installation
RAUC->>Config: Check boot image hooks
Config->>RAUC: Return hooks=install;
RAUC->>Boot: Install boot image
Boot->>RAUC: Boot image installed
RAUC->>User: Installation complete
sequenceDiagram
participant User
participant RAUC
participant Config
participant Boot
User->>RAUC: Initiate installation
RAUC->>Config: Check for CM5 configuration
Config->>RAUC: CM5 section not present
RAUC->>Config: Modify config.txt for CM5
Config->>RAUC: Config updated
RAUC->>Boot: Install boot image
Boot->>RAUC: Boot image installed
RAUC->>User: Installation complete
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🧹 Outside diff range and nitpick comments (1)
buildroot-external/ota/rauc-hook (1)
Line range hint
192-194: Critical: Remove reference to non-existent post_install_boot functionThe
post_install_bootfunction has been removed, but it's still being called in the slot-post-install section. This will cause the script to fail when processing boot slot post-installation.Apply this fix:
slot-post-install) - if [ "${RAUC_SLOT_CLASS}" = "boot" ]; then - post_install_boot - fi if [ "${RAUC_SLOT_CLASS}" = "kernel" ]; then post_install_kernel fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
buildroot-external/ota/manifest.raucm.gtpl(0 hunks)buildroot-external/ota/rauc-hook(1 hunks)
💤 Files with no reviewable changes (1)
- buildroot-external/ota/manifest.raucm.gtpl
🔇 Additional comments (1)
buildroot-external/ota/rauc-hook (1)
Line range hint 177-199: Verify hook merging approach with RAUC documentation
The approach of merging the post-install hook with the install hook aligns with the PR objectives. However, we should verify that this doesn't conflict with RAUC's expected behavior for other scenarios.
Let's verify the RAUC documentation and usage:
✅ Verification successful
Let me gather more information about RAUC hooks to make a well-informed decision.
Let me check the actual implementation of these hooks and their dependencies to understand the impact of merging them.
Merging post-install hook with install hook is safe in this context
The verification shows that:
- The hooks are only used within the
rauc-hookscript and have no external dependencies - The
install_bootandpost_install_bootfunctions are self-contained - The hook merging only affects the boot slot class, keeping other slot classes (like 'spl' and 'kernel') unchanged
- No other files in the codebase reference these hooks
The approach of merging the post-install functionality into the install hook is safe as it maintains the same operational sequence while reducing complexity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for RAUC hook documentation and usage patterns
# Search for RAUC hook documentation files
fd -t f -e md -e rst . | xargs rg -l "post-install.*hook"
# Search for similar hook merging patterns in the codebase
rg -p "slot-(install|post-install)"
Length of output: 179
Script:
#!/bin/bash
# Search for RAUC hook documentation and implementation patterns
# Look for hook-related content in documentation files
fd README . | xargs rg -i "rauc.*hook|hook.*rauc"
# Look for hook implementation patterns
rg -A 5 "slot-(install|post-install)" buildroot-external/ota/rauc-hook
# Search for any hook-related comments or documentation
rg -i "hook" buildroot-external/ota/rauc-hook
Length of output: 562
Script:
#!/bin/bash
# Check the implementation of install_boot and post_install_boot functions
rg -A 10 "^install_boot\(\)" buildroot-external/ota/rauc-hook
rg -A 10 "^post_install_boot\(\)" buildroot-external/ota/rauc-hook
# Check if there are any other files referencing these hooks
rg "install_boot|post_install_boot" --type-not sh
Length of output: 614
| # Add CM5 support for Yellow. Can be removed in HAOS 15. | ||
| if [ "$RAUC_SYSTEM_COMPATIBLE" = "haos-yellow" ]; then | ||
| if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt"; then | ||
| echo "Adding CM5 config to config.txt" | ||
|
|
||
| # Remove old single device_tree config and add CM-specific ones | ||
| sed -i '/device_tree=bcm2711-rpi-cm4-ha-yellow.dtb/d; s/\[all\]/\[cm4\]\ndevice_tree=bcm2711-rpi-cm4-ha-yellow.dtb\n\n\[cm5\]\ndevice_tree=bcm2712-rpi-cm5-ha-yellow.dtb\n\n\[all\]/' "${BOOT_MNT}/config.txt" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and backup mechanism for config.txt modifications
While the CM5 support addition is good, there are several reliability concerns:
- The sed operation is complex and lacks error checking
- No backup is created before modifying config.txt
- No validation of the final content
Consider this safer approach:
if [ "$RAUC_SYSTEM_COMPATIBLE" = "haos-yellow" ]; then
if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt"; then
echo "Adding CM5 config to config.txt"
+ # Create backup
+ cp "${BOOT_MNT}/config.txt" "${BOOT_MNT}/config.txt.bak"
+
+ # Remove old device tree config
+ sed -i '/device_tree=bcm2711-rpi-cm4-ha-yellow.dtb/d' "${BOOT_MNT}/config.txt" || {
+ echo "Failed to remove old device tree config"
+ cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt"
+ exit 1
+ }
+
+ # Add new CM-specific configs
+ sed -i 's/\[all\]/[cm4]\ndevice_tree=bcm2711-rpi-cm4-ha-yellow.dtb\n\n[cm5]\ndevice_tree=bcm2712-rpi-cm5-ha-yellow.dtb\n\n[all]/' "${BOOT_MNT}/config.txt" || {
+ echo "Failed to add CM-specific configs"
+ cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt"
+ exit 1
+ }
+
+ # Verify the changes
+ if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt" || ! grep -q "bcm2712-rpi-cm5-ha-yellow.dtb" "${BOOT_MNT}/config.txt"; then
+ echo "Failed to verify CM5 configuration"
+ cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt"
+ exit 1
+ }
fi
fiWould you like me to create a GitHub issue to track the removal of this code in HAOS 15?
📝 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.
| # Add CM5 support for Yellow. Can be removed in HAOS 15. | |
| if [ "$RAUC_SYSTEM_COMPATIBLE" = "haos-yellow" ]; then | |
| if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt"; then | |
| echo "Adding CM5 config to config.txt" | |
| # Remove old single device_tree config and add CM-specific ones | |
| sed -i '/device_tree=bcm2711-rpi-cm4-ha-yellow.dtb/d; s/\[all\]/\[cm4\]\ndevice_tree=bcm2711-rpi-cm4-ha-yellow.dtb\n\n\[cm5\]\ndevice_tree=bcm2712-rpi-cm5-ha-yellow.dtb\n\n\[all\]/' "${BOOT_MNT}/config.txt" | |
| fi | |
| fi | |
| # Add CM5 support for Yellow. Can be removed in HAOS 15. | |
| if [ "$RAUC_SYSTEM_COMPATIBLE" = "haos-yellow" ]; then | |
| if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt"; then | |
| echo "Adding CM5 config to config.txt" | |
| # Create backup | |
| cp "${BOOT_MNT}/config.txt" "${BOOT_MNT}/config.txt.bak" | |
| # Remove old device tree config | |
| sed -i '/device_tree=bcm2711-rpi-cm4-ha-yellow.dtb/d' "${BOOT_MNT}/config.txt" || { | |
| echo "Failed to remove old device tree config" | |
| cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt" | |
| exit 1 | |
| } | |
| # Add new CM-specific configs | |
| sed -i 's/\[all\]/[cm4]\ndevice_tree=bcm2711-rpi-cm4-ha-yellow.dtb\n\n[cm5]\ndevice_tree=bcm2712-rpi-cm5-ha-yellow.dtb\n\n[all]/' "${BOOT_MNT}/config.txt" || { | |
| echo "Failed to add CM-specific configs" | |
| cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt" | |
| exit 1 | |
| } | |
| # Verify the changes | |
| if ! grep -q "\[cm5\]" "${BOOT_MNT}/config.txt" || ! grep -q "bcm2712-rpi-cm5-ha-yellow.dtb" "${BOOT_MNT}/config.txt"; then | |
| echo "Failed to verify CM5 configuration" | |
| cp "${BOOT_MNT}/config.txt.bak" "${BOOT_MNT}/config.txt" | |
| exit 1 | |
| } | |
| fi | |
| fi |
As stated in the docs [1], post-install hook is not executed if the slot already has an install hook defined. Merge the post-install hook with the install hook to fix CM5 migration for Yellow.
[1] https://rauc.readthedocs.io/en/latest/using.html#slot-hooks
Summary by CodeRabbit
New Features
Bug Fixes
Refactor