armbian-common: rework to add more dependencies/recommends from armbian/build's CLI config#30
armbian-common: rework to add more dependencies/recommends from armbian/build's CLI config#30tabrisnet wants to merge 5 commits into
Conversation
…an/build's CLI config
WalkthroughThe PR updates debian/control for the armbian-common package, adding multiple new package names to Depends (apt-utils, ca-certificates, console-setup, e2fsprogs, initramfs-tools, iproute2, logrotate, linux-base, locales, gnupg2, tzdata) and many packages to Recommends (including bash-completion, curl, jq, rsync, wpasupplicant, wireguard-tools, u-boot-tools, etc.). No packages were removed; the change is additive and includes a minor formatting adjustment for sudo. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
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 |
leggewie
left a comment
There was a problem hiding this comment.
looking good, thank you
I haven't reviewed every single package that you added but it's easy enough to adjust if ever that became necessary. thank you for sure for adding your reasons of making some of the additions in the comment section of this PR.
I very much trust your skill and judgment. Approved. Again, thank you for your contribution. Much appreciated.
|
need to resolve a conflict now after #29 landed, will merge after that |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
debian/control (2)
18-35: Depends additions look sane for a CLI rootfs; only minor classification trade‑offs to confirmThe new
Dependsentries (apt-utils, ca-certificates, console-setup, e2fsprogs, initramfs-tools, iproute2, logrotate, linux-base, locales, gnupg2, tzdata) match the PR’s intent of making the cached rootfs self-contained and aligned with armbian/build’s CLI config.A few points you may want to double‑check before merging:
iproute2inDepends: this pushes it into the minimal “must-have” set for every armbian-common consumer. If there are environments that truly only need SSH + basic networking (and not fulliptooling), you might consider makingiproute2aRecommendsinstead, as mentioned in the PR text.e2fsprogsinDepends: this is a reasonable default for ext* rootfs resize-on-first-boot. If you later promote btrfs/xfs rootfs variants, you’ll probably also want their tooling (btrfs-progs,xfsprogs, etc.) treated consistently (either allDependsor allRecommends) to avoid asymmetric behavior on first boot.- Several of these (apt-utils, locales, tzdata, console-setup) can be quite chatty in debconf if DEBIAN_FRONTEND is not set; ensure armbian/build still forces noninteractive or seeds them appropriately so image builds remain deterministic.
Functionally this all looks fine; these are mostly codifying what’s already implicitly present via debootstrap. The above are just classification/UX considerations to confirm against your supported use-cases (CLI images vs custom derivatives, containers, etc.).
39-78: Recommends expansion aligns with armbian/build CLI packages but significantly enlarges the default base; consider whether any of these should be Suggests or in a separate metaThe enlarged
Recommendsset (bash-completion, bc, btrfs-progs, cron, curl, dbus-user-session, dosfstools, dialog, debconf-utils, debsums, fake-hwclock, fdisk, figlet, htop, init, iw, jq, less, lsof, man-db, mmc-utils, ncurses-term, parted, psmisc, rsync, rsyslog, systemd-resolved, toilet, u-boot-tools, usbutils, wget, wireguard-tools, wireless-regdb, wpasupplicant, etc.) looks like a direct transcription of the CLI config, which is good for keeping the rootfs cache and “armbian CLI” behavior in sync.A couple of potential follow‑ups to think about (not blockers):
- Some of these are clearly “core admin tools” (curl, rsync, fdisk, parted, man-db, rsyslog, u-boot-tools) and seem uncontroversial as
Recommends. Others are more cosmetic or niche (figlet/toilet, maybe wireguard-tools on boards that won’t use it) and might be better suited to eitherSuggestsor a separate “extras” metapackage if image size or boot time becomes a concern.initandsystemd-resolvedasRecommendsencode a fairly opinionated init/resolver stack. That’s probably fine for Armbian images, but if armbian-common is ever reused in container or alternate-init contexts, having these only asSuggests(or behind a more specific meta) could provide more flexibility.If the explicit goal is “armbian-common == CLI image base profile”, then this is consistent; otherwise, you may want to split “minimal base” vs “nice-to-have tooling” into separate metas down the road.
📜 Review details
Configuration used: CodeRabbit UI
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)
debian/control(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tabrisnet
Repo: armbian/apa PR: 25
File: debian/control:23-23
Timestamp: 2025-12-06T21:01:15.488Z
Learning: In the armbian/apa repository's debian/control, iputils-ping is temporarily added to armbian-common's Depends as a workaround for a capability issue where /bin/ping cannot create raw sockets. The proper solution involves: (1) PR #27, (2) removing the chmod u+s workaround code from armbian/build (armbian/build#9033), and (3) adding libcap2-bin to armbian/build's debootstrap package list so that iputils-ping's postinst script can use setcap to properly configure /bin/ping. Once these pieces are complete and merged, iputils-ping should be removed from armbian-common's Depends as it will no longer be needed.
📚 Learning: 2025-12-06T21:01:15.488Z
Learnt from: tabrisnet
Repo: armbian/apa PR: 25
File: debian/control:23-23
Timestamp: 2025-12-06T21:01:15.488Z
Learning: In the armbian/apa repository's debian/control, iputils-ping is temporarily added to armbian-common's Depends as a workaround for a capability issue where /bin/ping cannot create raw sockets. The proper solution involves: (1) PR #27, (2) removing the chmod u+s workaround code from armbian/build (armbian/build#9033), and (3) adding libcap2-bin to armbian/build's debootstrap package list so that iputils-ping's postinst script can use setcap to properly configure /bin/ping. Once these pieces are complete and merged, iputils-ping should be removed from armbian-common's Depends as it will no longer be needed.
Applied to files:
debian/control
|
this was merged via the CLI, not github tooling closing |
|
I suggest to consider moving some Depends to Recommends status to allow for deinstallation at runtime if so desired. I agree that e2fs-progs should be installed in all images including -minimal as long as they are ext2-based, but I believe it might be better handled as an explicit addition to the bootstrapped packages at compile time. -minimal should be possible to be really the absolute minimum (boots and can be accessed via ssh). There can of course be exceptions for practical purposes, hence "consider" above. |
|
I think to do this, we'd have to have another extension that knows which rootfs. I don't know if we have a variable for that yet or not. |
This is a cherry-pick, with a few teaks, from #25
apt-utilsis needed, albeit it will be installed bymmdebstrap[we have to explicitly do so], as islocalesconsole-setupis fromconfig/cli/common/debootstrap/packagesca-certificatese2fs-progsoddly isn't, but it is needed to do the FS resize that happens on first boot.btrfs,ext4,xfs, etc]?initramfs-utilsis needed for any kernel that I'm aware of. arguablyarmbian-bsporlinux-imageshould depend on this, but as its always needed, it should go into therootfscache.gnupg2is a virtual package that pulls in other GNU PG machinery, which afaik is needed for APT.aptsuggests it.config/cli/common/debootstrap/packageswhich suggests it's mandatoryiproute2seems obvious? maybe this could be demoted toRecommendstzdatathe
Recommendsare just pulled fromconfig/cli/common/main/packages