Re: [PATCH v2 1/2] crosvm: add recipe for ChromeOS Virtual Machine Monitor (VMM)

[Bruce Ashfield <bruce.ashfield@gmail.com>]

The big one is do_filter_minijail_cargo_config. The problem is real but
the workaround feels heavier than it needs to be — and I think we can
delete the task entirely with a one-character change to SRC_URI.

What's happening today: cargo_common_do_patch_paths() walks SRC_URI for
git entries with name= set, and for each one appends a [patch.crates-io]
entry to ${CARGO_HOME}/config.toml keyed on the name= value. So name=
minijail produces

    [patch.crates-io]
    minijail = { path = "${UNPACKDIR}/git/third_party/minijail" }

But crosvm's own upstream Cargo.toml already has

    [patch.crates-io]
    minijail = { path = "./third_party/minijail" }

Two entries, same key. Cargo doesn't allow duplicate [patch.crates-io]
keys regardless of whether the paths resolve to the same directory —
so the build breaks and the sed task exists to delete the auto-injected
line.

If we rename the SRC_URI name= to anything that isn't "minijail", the
collision disappears:

    SRC_URI = " \
        git://github.com/google/crosvm.git;branch=main;protocol=https;name=crosvm \
        git://github.com/google/minijail.git;branch=main;protocol=https;name=minijail_src;destsuffix=${BB_GIT_DEFAULT_DESTSUFFIX}/third_party/minijail \
    "

    SRCREV_crosvm       = "..."
    SRCREV_minijail_src = "..."
    SRCREV_FORMAT       = "crosvm_minijail_src"

The unpacked source still lands at the same place on disk (destsuffix
is what controls that), so crosvm's own Cargo.toml relative-path
patch still finds it and the build consumes the source as before.
The only change is that cargo_common now writes

    minijail_src = { path = "..." }

into config.toml — an entry keyed on a crate name nothing in the
dep graph references. Cargo will emit a non-fatal warning along the
lines of

    warning: Patch `minijail_src v...` was not used in the crate graph

That warning is expected and fine — please don't try to suppress it.
It's the price of having two patch tables (upstream's and OE-core's)
both pointing at the same on-disk source from different angles, and
making them silently co-exist is better than fighting either one.

With that, the entire do_filter_minijail_cargo_config task and its
addtask line can go away. A one-line comment above the SRC_URI noting
"name= is deliberately not 'minijail' so cargo_common's auto-injected
[patch.crates-io] entry doesn't collide with crosvm's own Cargo.toml,
which already patches minijail at ./third_party/minijail" would help
the next person who looks at this.

Smaller items:

  - LIC_FILES_CHKSUM points at ${COMMON_LICENSE_DIR}/BSD-3-Clause-Clear
    rather than the LICENSE file in the cloned source. The Yocto-shipped
    template is a generic copy; an edit to upstream's LICENSE (year
    bump, contributor line) won't trip do_populate_lic the way it
    should. Better as file://LICENSE;md5=<sum-from-actual-source>.

  - Three things I asked about on v4 that I don't see addressed:

    * BBCLASSEXTEND = "native" — I'd prefer we not have it in this
      first version. Do a runqemu alternative as a separate series
      and do it then.

    * DEPENDS includes wayland/wayland-native/wayland-protocols
      unconditionally. Is crosvm always built with the GPU/display
      feature on here? If those feature-bits can be disabled, please
      consider a PACKAGECONFIG so wayland is only pulled in when the
      display path is actually wanted. Acceptable to defer for now,
      but please call it out as a follow-up.

    * 966 pinned crates and a single BSD-3-Clause-Clear LICENSE
      declaration — there is almost certainly licence variation in
      that set. This isn't unique to crosvm (other recipes in the
      layer are guilty too and I'm working through them), so it
      isn't blocking, but please file/track it as a follow-up.

Bruce