[PATCH v1] crosvm: replace monolithic DEPENDS with PACKAGECONFIG flags

[PATCH v1] crosvm: replace monolithic DEPENDS with PACKAGECONFIG flags

[Keerthivasan Raghavan]

Previously, all crosvm feature dependencies were unconditionally added
to DEPENDS, with no way to enable or disable individual Cargo features
at the distro or machine level. Replace this with PACKAGECONFIG entries
that map each feature to its build-time dependencies. The default set
mirrors upstream crosvm defaults minus GPU, preserving existing
behaviour while allowing GPU (and its wayland pull-in) to be opted in
explicitly. Pass --no-default-features with the resolved PACKAGECONFIG
flags so Cargo feature selection is driven entirely from the recipe.

Tested by inspecting crosvm-image-minimal-qemux86-64.rootfs.manifest
to verify that installed packages match the expected PACKAGECONFIG and
dependency settings. Boot-to-shell into a crosvm guest was verified on
qemu x86-64 to confirm correct runtime behaviour. Build was also
verified on qemu aarch64.

Signed-off-by: Keerthivasan Raghavan <kraghava@qti.qualcomm.com>
---
 recipes-extended/crosvm/crosvm_0.1.0.bb | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/recipes-extended/crosvm/crosvm_0.1.0.bb b/recipes-extended/crosvm/crosvm_0.1.0.bb
index c192ae25..6a876fa3 100644
--- a/recipes-extended/crosvm/crosvm_0.1.0.bb
+++ b/recipes-extended/crosvm/crosvm_0.1.0.bb
@@ -28,9 +28,25 @@ SRCREV_FORMAT = "crosvm_minijail"
 # consistency, not an upstream project release.
 PV = "0.1.0+git"
 
-# TODO: Break this down into pkgconfig flags mapped to
-# rust cargo feature flags.
-DEPENDS += "libcap wayland wayland-native protobuf-native wayland-protocols clang-native"
+
+DEPENDS += "libcap clang-native"
+
+# The default set of packageconfig has been mapped to
+# crosvm default features with gpu removed.
+PACKAGECONFIG ??= "audio balloon document-features qcow usb libvda-stub net slirp"
+
+PACKAGECONFIG[audio]             = "--features audio,,"
+PACKAGECONFIG[balloon]           = "--features balloon,,"
+PACKAGECONFIG[document-features] = "--features document-features,,"
+PACKAGECONFIG[gpu]               = "--features gpu,,wayland wayland-native wayland-protocols,"
+PACKAGECONFIG[qcow]              = "--features qcow,,"
+PACKAGECONFIG[usb]               = "--features usb,,"
+PACKAGECONFIG[libvda-stub]       = "--features libvda-stub,,"
+PACKAGECONFIG[net]               = "--features net,,"
+PACKAGECONFIG[slirp]             = "--features slirp,,"
+
+# Disable upstream Cargo defaults and drive features entirely via PACKAGECONFIG
+CARGO_BUILD_FLAGS:append = " --no-default-features ${PACKAGECONFIG_CONFARGS}"
 
 REQUIRED_DISTRO_FEATURES = "kvm"
 
-- 
2.34.1

Re: [PATCH v1] crosvm: replace monolithic DEPENDS with PACKAGECONFIG flags

[Bruce Ashfield]

Thanks Keerthivasan — this cleanly addresses the TODO we left in v4 and
it's pretty close for a v1. A few questions before I take it; so let's
do them in a v2.

1. protobuf-native

   The old DEPENDS carried protobuf-native unconditionally, and the
   new base DEPENDS drops it without mapping it into any PACKAGECONFIG
   entry. Please confirm whether it's actually unused (in which case
   removing it as dead weight is fine — a one-liner in the commit
   message noting so would be great), or whether it's needed by a
   specific feature (in which case it should be attached to that
   feature's PACKAGECONFIG line, not left dangling).

   The test you ran ("default PACKAGECONFIG boots on x86-64") wouldn't
   necessarily exercise this — if some non-default feature (GPU,
   something else) pulls in protobuf codegen via a build.rs, that path
   is off in your test. A quick check with `--all-features` (or the
   full upstream default set) would answer it.

2. config-file in the default set

   Upstream crosvm's default features are approximately:

     ["audio", "balloon", "config-file", "document-features",
      "gpu", "libvda-stub", "net", "qcow", "slirp", "usb"]

   Your PACKAGECONFIG default is "audio balloon document-features qcow
   usb libvda-stub net slirp" — i.e. upstream's default minus gpu
   *and* minus config-file. GPU-off is deliberate and explained; the
   config-file omission isn't. Is that intentional (and if so, why)
   or an oversight? I'd lean toward matching upstream on this so our
   "default minus GPU" claim is exact.

3. document-features in the default set

   document-features is an attribute-macro crate whose sole purpose
   is generating documentation from feature declarations — it's a
   build-time doc helper, not a runtime feature. If this recipe isn't
   building or shipping docs by default (and it isn't), enabling it
   by default doesn't serve anyone. Please drop it from the default
   PACKAGECONFIG set. Keep it as a PACKAGECONFIG entry so someone
   who does want the doc build can flip it on.

I'll wait on a v2. Everything else looks good.

Bruce