public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: Sean <sean.brogan@microsoft.com>, devel@edk2.groups.io
Subject: Re: [edk2-devel] Adding Bhyve support into upstream EDK2
Date: Wed, 11 Mar 2020 00:34:51 +0100	[thread overview]
Message-ID: <f51c555b-bbae-70ef-b0a6-4013b3b5d5f7@redhat.com> (raw)
In-Reply-To: <21546.1583861100642932695@groups.io>

On 03/10/20 18:25, sean.brogan via [] wrote:
> I don't see the difference besides the mechanics of the operation (which
> you have described clearly).  To guarantee a repo or repos is
> "git-bisectable" you need to build and test every commit on your
> platform.  For example in the recent ArmMmuLib patchset, you were able
> to build every commit in the patch to identify which one caused the
> break.  There isn't an enforced process in place to ensure that happens
> within Edk2.  Thankfully the review process and the developers knowledge
> allowed the commits to be made in such a way that this was possible. 
> That doesn't have to change when you move to a submodule.  Also you
> could put automation in place to enforce and/or test for this scenario. 
> You can put automation in place to "integrate" into your super project
> at every commit if you really wanted to and had the resources to run
> tests on every one of those commits.  Is this type of CI done today for
> OVMF?  

Consider a patch series that needs to modify at least one core module
and at least one platform, in order to reach a desired result. Such
patch series are not infrequent: one of my recent contributions (commit
range 61d3b2d4279e..1158fc8e2c7b) modified MdeModulePkg, UefiCpuPkg, and
OvmfPkg.

When I work on a patch series (any patch series, really) I not only test
that the work builds at every stage, but also that it *functions* at
every stage (in other words that no stage in the series regresses
existing functionality, and that it indeed implements the desired step
of the feature or bugfix). This is done specifically so that, in case I
miss a use case completely, and regress it, another developer can return
to any stage across my work (regardless of "core module" vs "platform
code" distinction), via bisection, and pinpoint my mistake.

So: this kind of CI is not being done (it's not enforced), but I take it
seriously.

If we split platforms to different repos from the core, I can't even
propose a patch set, in the first place, that modifies both kinds of
modules in one logical go.

> 
> Again this is what nearly all platforms have to do today and we have a
> lot of experience with bisecting within the submodule to find the
> error.  The longer you wait between integrations the more costly the
> bisect is if you have to do it, but this is a choice of the super
> project owner / platform owner.  Today I assume you make those choices
> too, they just happen to be within the same repo.  I also assume that if
> you found the MmuLib bug in a few days you probably wouldn't bisect all
> the changes but you would review the history to intelligently guess at
> the most likely candidates and bisect within those commits.

My thinking is the inverse. Bisection cost grows logarithmically --
covering a range of 256 commits takes just one build more than covering
a range of 128 commits. To me, bisection comes first, exactly so I don't
have to think (or intelligently guess), before the issue is narrowed
down to a single patch. I have seen such a huge amount of incredible /
stunning bisection results ("I would have never thought of this!") that
I don't even try to guess, any more.

Regarding the six patches that I bisected a few days ago -- those hadn't
even been upstreamed, I was preparing to merge them, for Ard. I
collected them from three separate series (posted & reviewed on the
list), in the logical order explained by the cover letters. They were
super quirky and over my head (aarch64 MMU programming? thanks but no
thanks); I would have had zero chance to guess which one caused the
crash with "grubaa64.efi".

The bisection allowed me at least to tell Ard which one of his patches
looked risky.

... I've got almost entirely mechanical Linux kernel bisections behind
me that took 5-8 hours and pinpointed very unexpected culprit patches,
from multiple thousands of commits between releases. I could have never
tracked those bisections myself, if I had had to dig into submodules
manually.

I understand that I'm appearing too stubborn. I promise it's not just
for the sake of arguing.

Thanks,
Laszlo

> 
> In the end I just don't see the big difference to the platform (OVMF in
> this case) but I do see the reduced size/noise/content helping all
> platforms.  Success still relies on good development practices, regular
> builds, and testing.    
> 
> Thanks
> Sean
> 
>   


  parent reply	other threads:[~2020-03-10 23:34 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 16:09 Adding Bhyve support into upstream EDK2 Rebecca Cran
2020-03-06 19:54 ` Laszlo Ersek
2020-03-06 20:04   ` [edk2-devel] " Laszlo Ersek
2020-03-07  1:29 ` Yao, Jiewen
2020-03-24  1:34   ` Rebecca Cran
2020-03-25  0:04     ` Laszlo Ersek
2020-03-25 18:18       ` [EXTERNAL] " Bret Barkelew
2020-03-27 12:56         ` Laszlo Ersek
2020-03-25 18:50       ` Rebecca Cran
     [not found] ` <15F9E16A0219E7B7.19404@groups.io>
2020-03-07  1:43   ` Yao, Jiewen
2020-03-07  7:39     ` Laszlo Ersek
2020-03-07  7:52       ` Ard Biesheuvel
2020-03-08  2:40         ` Rebecca Cran
2020-03-09  6:08         ` Sean
2020-03-09 22:54           ` Laszlo Ersek
2020-03-09 23:17             ` Laszlo Ersek
2020-03-10  1:50               ` Sean
2020-03-10  9:05                 ` Laszlo Ersek
2020-03-10 17:25                   ` Sean
2020-03-10 17:54                     ` Ard Biesheuvel
2020-03-10 19:10                       ` Sean
2020-03-10 19:23                         ` Michael D Kinney
2020-03-10 19:44                           ` Sean
2020-03-10 20:04                             ` Rebecca Cran
2020-03-11  0:05                             ` Laszlo Ersek
2020-03-11  0:30                               ` Sean
2020-03-11  3:21                             ` Liming Gao
2020-03-10 23:34                     ` Laszlo Ersek [this message]
2020-03-11  0:43           ` Leif Lindholm
2020-03-07  7:53       ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f51c555b-bbae-70ef-b0a6-4013b3b5d5f7@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox