Skip to content

[RFD] lib / rootfs-create: add post-debootstrap hook #9085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leggewie wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −0
Open

[RFD] lib / rootfs-create: add post-debootstrap hook #9085

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/functions/rootfs/rootfs-create.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ function create_new_rootfs_cache_via_debootstrap() {
chroot_sdcard_apt_get_install "${AGGREGATED_PACKAGES_DESKTOP[@]}"
fi

# 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
Comment on lines +303 to +306
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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 2

Repository: 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 -n

Repository: 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.

Copy link
Collaborator

@tabrisnet tabrisnet Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.


# stage: check md5 sum of installed packages. Just in case. @TODO: rpardini: this should also be done when a cache is used, not only when it is created
# lets check only for supported targets only unless forced
if [[ "${DISTRIBUTION_STATUS}" == "supported" && "${FORCE_CHECK_MD5_PACKAGES:-"no"}" == "yes" ]]; then
Expand Down