feat(debos): validate host build dependencies per use-case - #454
feat(debos): validate host build dependencies per use-case#454Bjordis Collaku (bjordiscollaku) wants to merge 7 commits into
Conversation
27b6172 to
f113acf
Compare
Bjordis Collaku (bjordiscollaku)
changed the title
Bjordis Collaku (bjordiscollaku)
changed the title
Bjordis Collaku (bjordiscollaku)
changed the title
Test jobs for commit f113acf |
Bjordis Collaku (bjordiscollaku)
changed the title
Loïc Minier (lool)
left a comment
Loïc Minier (lool)
left a comment
There was a problem hiding this comment.
Thanks Bjordis Collaku (@bjordiscollaku) this looks very nice! I made a number of requests for small changes
| # file, device-tree-compiler and u-boot-tools are needed by qcom-dtb-metadata | ||
| - name: Install debos and dependencies of the recipes and local tests | ||
| run: apt -y install debian-archive-keyring debos file make mmdebstrap mtools python3-pexpect python3-pytest qemu-efi-aarch64 qemu-system-arm xmlstarlet python3-defusedxml device-tree-compiler u-boot-tools | ||
| run: apt -y install debian-archive-keyring debos dosfstools file make mmdebstrap mtools python3-pexpect python3-pytest qemu-efi-aarch64 qemu-system-arm xmlstarlet python3-defusedxml device-tree-compiler u-boot-tools |
There was a problem hiding this comment.
could we drop these and set -t auto_install_deps:true during our CI?
There was a problem hiding this comment.
I'd keep the explicit list for CI, for two reasons.
That apt line installs more than the recipes' host deps: it also brings in debos itself, the local test harness (python3-pexpect, python3-pytest), qemu-*, device-tree-compiler and u-boot-tools. None of those are in any recipe's check-deps set, so auto_install_deps:true wouldn't pull them and the job would lose debos and the pytest run.
And in CI the image/flash recipes run under -b qemu, so their check runs inside the fakemachine guest where /var/lib/dpkg/status doesn't exist and it skips, so auto_install_deps is effectively inert for those two. Only rootfs runs natively. The host tools image/flash need (mkfs.vfat, mtools, and so on) have to be on the runner before debos starts, which is what this step provides.
Side note while we're here: most of that filesystem tooling is already pulled in transitively. The Debian debos package Recommends dosfstools/e2fsprogs/parted/fdisk and we install with recommends on, which is why main builds vfat+ext4 green without an explicit dosfstools. The line I added is really just belt-and-suspenders against a future --no-install-recommends.
If the duplication between the recipe lists and this step is what's bugging you, I'd rather lift the dep lists into one shared spot than drop the CI install. Happy to do that as a follow-up.
There was a problem hiding this comment.
Yes, duplication was what was on my mind; how could we avoid it?
Maybe we shouldn't be running this script from within the recipe, but just before calling debos on the recipe.
There was a problem hiding this comment.
Agreed. We could move it out of the recipe and run it just before the debos call, say from the Makefile, and keep each recipe's deps as a variable there. That way they're not repeated between the recipes and the CI apt step. What do you think?
There was a problem hiding this comment.
Yup; I think this would look clean. I wonder whether the script should have the list of use cases embedded in it, so that makefile / CI / README basically leverage check-deps.sh host , check-deps.sh image, check-deps.sh flash etc.
There was a problem hiding this comment.
Yep agreed, and the use cases would be:
- host: debos, make, qemu-system-arm
- rootfs: mmdebstrap, debian-archive-keyring
- image: dosfstools, e2fsprogs
- flash: dosfstools, mtools, xmlstarlet, python3-defusedxml, unzip, device-tree-compiler, u-boot-tools, file
- test: qemu-efi-aarch64, qemu-system-arm, python3-pexpect, python3-pytest
Makefile, CI, and README each call check-deps.sh <use-case>. Does that grouping look right?
There was a problem hiding this comment.
I haven't cross-checked the lists of packages, but +1 on the target names
Test jobs for commit b9ac53d |
Test jobs for commit d1df400 |
|
Bjordis Collaku (@bjordiscollaku) BTW I just remembered, we also have a couple more implementations of similar logic in the same repo:
but we can address these later or even let them be. Slight preference for a Python implementation eventually, but also happy with the current shell one to start with, it's simple enough. |
|
My comment would be to not include qemu in the list, since it's a dependency on the Debian package of debos |
Sounds good, I'll leave those two for a follow-up. |
Agreed, dropping |
Yeah, that sounds good to me. Thanks! |
|
|
||
| missing="" | ||
| for pkg in "$@"; do | ||
| dpkg -s "$pkg" >/dev/null 2>&1 || missing="$missing $pkg" |
There was a problem hiding this comment.
This is not a robust way of determining if a package is installed. Packages can be in a removed (but not purged) or broken state and would still appear under dpkg -s. Instead you could use apt-get install -s to see what apt-get would do, to see if any package installations are required.
I would call sudo apt-get install -y --no-install-recommends "$@" in the --install case, and apt-get install -qqs --no-install-recommends "$@"|grep -E '^(Inst|Conf)' in the non- --install case. Wouldn't that be much cleaner, and avoid having to second guess apt with dpkg?
It would have the effect of failing if an apt upgrade is pending, but generally that seems like a good idea anyway, and I prefer that over the fragility of calling dpkg directly. If you want to avoid that, then you could call dpkg-query -f '${Status}' -W "$@" and check for ok and installed in the second and third columns or similar.
There was a problem hiding this comment.
Thanks for the feedback Robie.
--install: I'll drop the pre-check and just call apt-get install -y --no-install-recommends on the set, since it's idempotent.
Validate-only: you preferred the simulate, but I'd take the dpkg-query option you offered, because of one hole I checked in the simulate. On a name apt can't resolve (a typo, or a dropped/renamed package), apt-get install -qqs <name> prints "Unable to locate package" to stderr and nothing to stdout, so grep -E '^(Inst|Conf)' matches nothing and the check reads it as satisfied. dpkg-query -f '${Status}' -W reports it not-installed there, so the gate fails closed instead.
Introduce scripts/check-deps.sh, a lightweight helper that validates
required host packages are installed before a debos recipe proceeds.
Each recipe declares its own dependency set and calls the script as its
first action (chroot: false), ensuring failures are caught immediately
with a clear diagnostic rather than mid-run with a cryptic tool error.
The script accepts an optional --install flag. When present, it runs
apt-get to install any missing packages. When absent, it reports the
missing packages to stderr and exits non-zero, directing the user to
either install them manually or pass -t auto_install_deps:true.
The Makefile sets auto_install_deps:true for the Docker container path
(USE_CONTAINER=yes), so make flash / make all work out-of-the-box
against ghcr.io/go-debos/debos:latest without requiring a pre-customised
image. The native debos path is unchanged: the check runs in
validate-only mode and fails fast if dependencies are absent.
When dpkg infrastructure is not present (e.g. inside a fakemachine qemu
VM), the script exits silently. In that environment dependencies are
expected to be installed on the real host before debos is invoked.
The GitHub Actions workflow installs all recipe dependencies on the CI
host via apt before invoking debos, including dosfstools which is now
explicitly listed. Recipes running under fakemachine skip the check
silently as described above.
Dependency sets per recipe:
qualcomm-linux-debian-rootfs.yaml: debian-archive-keyring mmdebstrap
qualcomm-linux-debian-image.yaml: dosfstools
qualcomm-linux-debian-flash.yaml: dosfstools mtools python3-defusedxml
unzip xmlstarlet
Addresses the dependency management approach discussed in PR qualcomm-linux#371.
Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
Address review feedback on the host dependency validation. Move "-t auto_install_deps:true" ahead of $(DEBOS_OPTS) in the container command so that EXTRA_DEBOS_OPTS can still override it; debos applies the last -t value given for a key. Document why auto-installing into the disposable container is safe. Expand the dpkg-database guard and the no-missing exit into explicit if/then blocks, and warn on stderr when the check is skipped. The guard tests for the dpkg database rather than the dpkg binary because a chroot:false action inside a fakemachine guest sees the host /usr, and therefore the dpkg binary, but not /var; only the database is a reliable signal of a queryable host. Log the APT refresh and the exact package set before installing, and drop apt-get's -qq so failing sources are visible. No change to the dependency sets or to the native/CI fail-fast path; the container path gains a documented opt-out and clearer diagnostics. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
The image recipe's image-partition action formats both filesystems: the vfat ESP (mkfs.vfat, from dosfstools) and the ext4 root (mkfs.ext4, from e2fsprogs). Only dosfstools was declared. e2fsprogs is Priority important in trixie, not required, so a stripped-down container is not guaranteed to ship it; list it explicitly, mirroring the vfat tool. Host dependency check only; no change to recipe logic or the produced image. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
The dependency check ran inside the recipes as a chroot:false action. Under fakemachine that runs in the guest, where /var/lib/dpkg/status does not exist, so it skipped on every fakemachine-backed build and never validated or installed anything. Move it out. check-deps.sh now holds the package lists keyed by use-case (host, rootfs, image, flash, test) so the Makefile, and later CI and the README, can call a use-case name instead of repeating a package list. The Makefile runs the check right before each debos call, the recipes carry no dependency logic anymore, and with the check on the host rather than a guest it tests for the dpkg command instead of the status file. The check validates and fails by default; AUTO_INSTALL_DEPS=yes makes it install instead. The container path keeps calling debos directly, provisioning that image is a separate concern, and the now-dead -t auto_install_deps:true flag is gone. qemu-system-arm is a Depends of debos, so it is not listed. dosfstools, e2fsprogs, parted, fdisk and udev are Recommends of debos, not Depends, and the image-partition action execs mkfs.vfat, mkfs.ext4, parted, sfdisk and udevadm, so image lists them. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
dpkg -s exits 0 for a package in config-files (removed, not purged) or a half-configured state, so a package whose binaries are gone still passed the check. Read the dpkg-query Status field and require "ok installed" instead; held packages still pass. On the install path, drop the pre-computed missing list and just run apt-get install -y --no-install-recommends on the whole set. apt is idempotent, so there is no point second-guessing it with dpkg. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
Four tools reached the build only because the old CI installed with apt Recommends enabled. This install path uses --no-install-recommends, so name each in the use-case that needs it: - linux-image-arm64 (host): a Recommends of debos. fakemachine boots a kernel from /boot and the CI job container ships none. The script installs linux-image-$(dpkg --print-architecture), and only where that meta-package resolves; a normal host has a kernel already. - ipxe-qemu (host, test): a Recommends of qemu-system-arm. It ships /usr/share/qemu/efi-virtio.rom; without it qemu will not start the default virtio-net-pci NIC, so fakemachine and the boot test fail at startup. - qemu-utils (test): a Recommends of qemu-system-arm, shipping qemu-img, which ci/qemu_test.py runs first to build the boot overlay. - ca-certificates (flash): the flash recipe fetches over https and debos verifies against the CA bundle /etc/ssl/certs/ca-certificates.crt; nothing in the install set Depends on it. udev (in the image list) is also a Recommends of debos but is not in this set: it survives --no-install-recommends via the kernel's initramfs-tools alternative dependency, and image lists it because image-partition execs udevadm directly. None of the four is new to the build; all were pulled by Recommends before. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
…ADME Replace both hand-maintained apt lines with scripts/check-deps.sh --install host rootfs image flash test. The two had drifted (the README one was missing device-tree-compiler, u-boot-tools and file); now the package names live only in check-deps.sh. Signed-off-by: Bjordis Collaku <bcollaku@qti.qualcomm.com>
d1df400 to
ab0f9d2
Compare
Bjordis Collaku (bjordiscollaku)
changed the title
ab0f9d2 to
4ba715c
Compare
Test jobs for commit ab0f9d2 |
Test jobs for commit 4ba715c |
|
This PR is stale. Add a nice message, can mention maintainers or maintainer team to draw attention |
4 participants
Nothing on main validates the host tools the builds need before running
debos, and the two hand-maintained apt lists have drifted: the README
build-deps line is missing device-tree-compiler, u-boot-tools and file
that the CI step installs. A local
makewith a tool missing failssomewhere inside debos with a confusing error rather than a clear one up
front.
check-deps.sh now owns the package lists, keyed by use-case. The Makefile
validates the right use-case before each debos call (or installs it with
AUTO_INSTALL_DEPS=yes), CI and the README run one line,
scripts/check-deps.sh --install host rootfs image flash test, and nopackage name appears anywhere else.
Validate reads the dpkg-query Status field and requires "ok installed"
rather than trusting
dpkg -s, which also exits 0 for a package inconfig-files or a half-configured state; the install path is a plain
apt-get install -y --no-install-recommends. qemu-system-arm stays offthe lists: it is a Depends of debos, so installing debos pulls it in.
Four packages are named that the old apt line never was; they reached the
build via apt Recommends and are dropped once installs use
--no-install-recommends. linux-image-arm64 is a Recommends of debos, the
kernel fakemachine boots from /boot (the CI job container ships none); the
script installs linux-image-$(dpkg --print-architecture), and only where
that meta-package resolves, since a normal host already has a kernel.
ipxe-qemu is a Recommends of qemu-system-arm and ships
/usr/share/qemu/efi-virtio.rom, without which qemu will not start the
default virtio-net-pci NIC, so fakemachine and the boot test fail at
startup. qemu-utils is a Recommends of qemu-system-arm and ships qemu-img,
which ci/qemu_test.py runs first to build the boot overlay.
ca-certificates' postinst runs update-ca-certificates to generate the CA
bundle /etc/ssl/certs/ca-certificates.crt, which debos (a Go binary)
verifies https downloads against; nothing in the install set Depends on
it. udev is in the image list for a different reason: debos's
image-partition execs udevadm trigger --settle directly. It is a
Recommends of debos, and its only other route is the kernel's
initramfs-tools alternative dependency, so image names it rather than
relying on either.
Checked by replaying the workflow in the debian:trixie CI container
(public.ecr.aws/debian/debian:trixie, arm64): union install; per-use-case
validate against present, missing and config-files states; and rootfs,
UFS image and flash builds under the qemu fakemachine backend, plus the
qemu boot test, all green in that container. That does not cover the
self-hosted arm64 runner environment (/efs, artifact upload); the CI run
on this PR exercises that.
Provisioning the debos container image stays out of scope, as does
folding in build-linux-deb.py and build-u-boot-rb1.sh.