Re: [PATCH v1] crosvm: replace monolithic DEPENDS with PACKAGECONFIG flags
Bruce Ashfield <[email protected]> Tue, 28 Jul 2026 10:37:30 -0700 (PDT)
| Newsgroups | org.yoctoproject.lists.meta-virtualization |
|---|---|
| Message-ID | <[email protected]> |
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