Re: [edk2-devel] Adding Bhyve support into upstream EDK2

["Laszlo Ersek" <lersek@redhat.com>]

On 03/10/20 02:50, sean.brogan via [] wrote:
> [...] There are numerous ways to keep multiple repos in sync that
> allow a user to root cause regressions.  Using submodules is one way
> to easily track the version of edk2 that was/is used in the platform
> repo and when issues are identified the commit history can be
> bisected to find the problem.  [...]
I disagree.

Running git-bisect on a commit range in the superproject will identify
the submodule update commit as the one introducing the regression, yes.

But I don't see a way forward from that point onward. The goal of
git-bisect is to identify (pinpoint) the smallest atomic change that's
related to a regression. A submodule update is the polar opposite of an
atomic change.

Delving into the submodule itself, for a git-bisect -- i.e., bisecting
the submodule between the boundaries set by the superproject's submodule
update commit -- is not something that can be relied upon.

Namely, a core trait of git-bisect is that it tests states of the whole
project that have *existed* at some point. But a combination of:

- the superproject checked out at any particular point in its own history,

- and the submodule checked out strictly *between* two such adjacent
commits that have ever been actively consumed by the superproject,

is not a state like that.

Such states may not even build. They may never have been tested, put
through CI, and so whether they work or not is inconsequential -- they
should not be *expected* to work. Such states don't qualify as
"checkpoints". That's because a submodule is unaware of all the
superprojects that consume it. Whereas, in a shared git history, every
single commit counts as a checkpoint -- a combined state that has
existed at some point.

You can solve this problem (in the separate repository model) only if
you build and test every known platform (= every known superproject)
against every single patch (not series!) that is proposed for the core
(= the submodule). Importantly, it's not enough to build and test the
superprojects at the ends of the submodule (= core) patch sets, because
that still leaves the checkpoints two far apart. The full platform
testing (for all platforms) would have to be done at every stage of the
submodule (= core) patch series.

Turning a codebase (such as core edk2) into a submodule for a
superproject (such as OVMF) is equivalent to saying, "we trust the
submodule that, at distinguished checkouts, it holds up all its
contracts". That's exactly what I don't trust the core of edk2 to do,
and those contract violations are exactly what git-bisect is designed to
root out.

The edk2 bisection that I've performed most recently was less than nine
hours ago. It identified a regression in a core (non-platform) change,
namely "ArmPkg/Library/ArmMmuLib":

https://edk2.groups.io/g/devel/message/55701

It was doable because git-bisect was aware of six patches that had been
applied in a row on ArmMmuLib against a single ArmVirtQemu platform state.

I may not be knowledgeable enough about git-bisect, of course; I've
asked my teammates internally if they have positive experience and/or
recommendations with git-bisect across submodules.

Thanks
Laszlo