[RFD] lib / rootfs-create: add post-debootstrap hook #9085
Conversation
WalkthroughA new extension hook Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
github-actions
bot
added
size/small
leggewie
changed the title
![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 (1)
lib/functions/rootfs/rootfs-create.sh (1)
304-305: Expand the hook description for extension developers.The current description is brief. Consider documenting the rootfs state and available resources to help extension developers understand what operations are safe and supported at this hook point.
Apply this diff to provide more context:
call_extension_method "post_debootstrap_customize" <<- 'POST_DEBOOTSTRAP_CUSTOMIZE' - allow customization of rootfs after debootstrap has completed + Customize the rootfs after debootstrap and all package installations. + At this point: chroot is mounted, qemu binary is deployed, all main and desktop packages are installed. + Use this hook for modifications requiring a working chroot environment before cleanup operations begin. POST_DEBOOTSTRAP_CUSTOMIZE
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/functions/rootfs/rootfs-create.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: The `debootstrap` tool doesn't use `apt`, which is important context when reviewing apt-related configuration in rootfs creation code.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Applied to files:
lib/functions/rootfs/rootfs-create.sh
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: The `debootstrap` tool doesn't use `apt`, which is important context when reviewing apt-related configuration in rootfs creation code.
Applied to files:
lib/functions/rootfs/rootfs-create.sh
| # customization hook after debootstrap phase | ||
| call_extension_method "post_debootstrap_customize" <<- 'POST_DEBOOTSTRAP_CUSTOMIZE' | ||
| allow customization of rootfs after debootstrap has completed | ||
| POST_DEBOOTSTRAP_CUSTOMIZE |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for call_extension_method patterns and read context around lines 303-306
rg -n "call_extension_method" lib/functions/rootfs/rootfs-create.sh -A 2 -B 2Repository: armbian/build
Length of output: 300
🏁 Script executed:
# Also read the specific lines mentioned in the review
sed -n '280,370p' lib/functions/rootfs/rootfs-create.sh | cat -nRepository: armbian/build
Length of output: 5423
Hook placement is appropriate; verify with test implementation.
The hook is correctly positioned after all package installations and while the chroot is still mounted (line 365 for unmount). This provides a functional environment for rootfs customization before cleanup operations begin (autoremove at line 330, qemu undeploy at line 357).
Since this is completely untested, verify by:
- Creating a test extension implementing
post_debootstrap_customize__test_hook - Confirming the chroot environment is accessible for customization operations
- Validating that changes persist through cleanup and final image creation
🤖 Prompt for AI Agents
In lib/functions/rootfs/rootfs-create.sh around lines 303 to 306, the new
post_debootstrap_customize hook is placed correctly but untested; add a small
test extension named post_debootstrap_customize__test_hook that runs inside the
chroot (e.g., creates a marker file and modifies a simple config), run the full
create flow to ensure the chroot is writeable and the marker persists after
autoremove/qemu-undeploy and final image creation, and update CI or local test
script to execute this test extension and assert the marker file exists in the
final rootfs image; if the marker is not present, adjust hook invocation context
(mount/chroot environment variables, bind-mounts, or call_extension_method
invocation) so the hook runs with the same chroot environment used for package
installs.
There was a problem hiding this comment.
fwiw this concept has been tested [but not this specific code] in #9000.
So yes, it should be tested to be sure there isn't a trivial error, but the concept and placement is reasonably solid.
There was a problem hiding this comment.
@tabrisnet, thank you for the clarification! It's reassuring to know that the concept and placement have been validated through testing in PR #9000. That gives much more confidence in the approach.
Given that context, my recommendation shifts to simply verifying the specific implementation details here—checking for any trivial errors like typos in the hook name, ensuring the heredoc syntax is correct, and confirming the hook is callable from extensions. A quick smoke test with a minimal extension implementing post_debootstrap_customize__test_hook should be sufficient to validate this specific code works as expected.
The placement after package installations and before cleanup operations is indeed the right spot for rootfs customization.
✏️ Learnings added
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:12.924Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: The `debootstrap` tool doesn't use `apt`, which is important context when reviewing apt-related configuration in rootfs creation code.
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-13T23:44:04.995Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the schedule_deletion for /var/lib/dbus/machine-id is necessary because: (1) the file is created by the dbus package during installation, (2) package installation timing varies during the build process and is being actively worked on, (3) scheduling the deletion ensures it's removed at the end regardless of when the package gets installed, (4) the file must not exist on first-boot so the system generates a fresh machine-id.
Learnt from: leggewie
Repo: armbian/build PR: 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`.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.
Thank you, @tabrisnet for the confirmation. Do you have anything to add in terms of above questions or something else to take into consideration? The implementation of the hook is slightly different to what you had in #9000 |
2 participants
another carve-out from #9000
Apparently, it might be advantageous / necessary to add a hook to the framework to allow for customization of the rootfs just after debootstrap has done its thing. Now, this is certainly way ahead of my comprehension so I add this here only as an RFD to see if this would be a step in the right direction and if I got anything completely wrong.
this is completely untested as of now.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.