Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members

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

On 10/29/23 20:01, Pedro Falcato wrote:
> On Sun, Oct 29, 2023 at 1:30 PM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 10/29/23 03:16, Pedro Falcato wrote:
>>>> diff --git a/Maintainers.txt b/Maintainers.txt
>>>> index 3f40cdeb5554..2b03ccbe54aa 100644
>>>> --- a/Maintainers.txt
>>>> +++ b/Maintainers.txt
>>>> @@ -93,7 +93,7 @@ M: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
>>>>  RISCV64
>>>>  F: */RiscV64/
>>>>  M: Sunil V L <sunilvl@ventanamicro.com> [vlsunil]
>>>> -R: Daniel Schaefer <git@danielschaefer.me> [JohnAZoidberg]
>>>> +R: Andrei Warkentin <andrei.warkentin@intel.com> [andreiw]
>>>>
>>>>  LOONGARCH64
>>>>  F: */LoongArch64/
>>>> @@ -157,16 +157,6 @@ R: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
>>>>  R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
>>>>  R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
>>>>
>>>> -ArmVirtPkg: modules used on Xen
>>>> -F: ArmVirtPkg/ArmVirtXen.*
>>>> -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
>>>> -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
>>>> -F: ArmVirtPkg/PrePi/
>>>> -F: ArmVirtPkg/XenAcpiPlatformDxe/
>>>> -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
>>>> -F: ArmVirtPkg/XenioFdtDxe/
>>>> -R: Julien Grall <julien@xen.org> [jgrall]
>>>
>>> ArmVirtPkg Xen modules seize to have a dedicated maintainer. Can the
>>> generic ArmVirtPkg maintainers handle *more code* (particularly,
>>> functionality that's not trivial to test, unless you actively use
>>> Xen)?
>>
>> An alternative to removing this entire section is to replace Julien's
>> line with the following status line:
>>
>> S: Orphan
>>
>> The definition in Maintainers.txt is:
>>
>>      Orphan:     No current maintainer [but maybe you could take the
>>                  role as you write your new code].
>>
>> I think this might be clearer for all three of: contributors, consumers,
>> and existent maintainers.
>>
>> - Contributors: An ArmVirtPkg maintainer may techincally merge your
>> code, but you won't get substantive feedback
>>
>> - Consumers: you can build and run this code, but if it breaks, you get
>> to keep both parts
>>
>> - Existent ArmVirtPkg maintainers: you can rest assured in the knowledge
>> that you are not saddled with deep technical reviews for this subsystem
>> that you can't even boot in your environment. You're only responsible
>> for the technical act of merging patches.
> 
> I agree with this solution, but I do think there should be a "time
> limit" for orphaned code. You don't want to keep orphaned code for too
> long, this is not a practice we should support (which may lead to
> corporate code dumps where corps just dump a bunch of patches on the
> mailing list, fire and forget, and they're still "supported").

Very difficult question; there could be end-users relying on the feature
still, without anyone shouldering the maintenance costs :( I certainly
see your point, I just can't either agree *or* disagree with it!

(I recently cleaned up even *build* breakages in edk2-platforms; I also
found libraries that were not used *at all* by in-tree platforms. Those
obviously come from ancient corporate code-drops, and nobody must have
built them very recently, but if we remove them, we could still
ultimately harm end-users.)

[...]


>> This leads me to my main point: it may be time for edk2 to adopt a
>> leaner contribution process.
>>
>> We can insist on no patch going in without maintainer approval, but that
>> -- i.e., maintainer authority -- only works as long as it goes hand in
>> hand with maintainer responsibility: timely reviews. If the community
>> cannot offer enough working hours for reviewing patches for a subsystem,
>> then the requirements to contribute to that subsystem should be relaxed.
>> The other alternative is that the subsystem goes into stasis, where it
>> becomes effectively impossible to contribute to a subsystem.
>>
>> (NB this "relaxation of contribution rules" is entirely orthogonal to
>> using a mailing list vs. github pull requests. I still strongly prefer
>> the mailing list.)
>>
>> So maybe we could say, if there is no patch review for like 7 working
>> days (approx. one and half calendar weeks), then the patch should be
>> merged by default. Put differently, switch from "rejected by default" to
>> "accepted by default".
> 
> I understand your idea. I do, however, see it going in 2 different ways:
> 1) This pressures maintainers/corporations to be faster at reviewing
> patches, keeping a smooth flow of careful, reviewed patches. Things
> continue to work smoothly.
> 2) Maintainers keep being unresponsive, patches get "auto merged" and
> people need to deal with any ensuing breakage. Things /may/ regularly
> break.

You are too polite; it's quite likely that things *will* break :)


> IMO, this solution does not solve anything. If maintainers are short
> on time (or simply spread too thin), they will still be short on time
> unless corps give them more time for FOSS work. This just adds a fear
> factor ("Complete shite may be automerged if you don't have people on
> it!!").

The alternative is stasis, as I wrote above. Everything grinds to a
halt, and people can't proceed with careful, justified work either.

The true solution is for the orgs behind the maintainers to invest more
resources into maintenance. If that's not happening (and I don't see it
happening), then you can only choose between stasis and chaos.

Presently, I'm trending towards chaos. Here's why:

- because it lets *me* do my work at least,

- it lets me assist with review & testing for such subsystems that I may
not be *directly* interested in.

Stasis means my own work is blocked, and that I cannot help others
progress with their work either (if my review or testing is ignored by a
maintainer, then the submitter isn't any better off).

... I think my stance on this may have changed in the last two years.
During that time, I focused on the libguestfs mailing list. Very small
community, mostly based on trust, fast moving, much more relaxed towards
reviews (especially iterated reviews). The same circumstances don't
apply to edk2-devel 1-to-1 of course, but it still made the faster,
lighter pace of development attractive to me.

Obviously I'm still a fan of a super-strict process as well, but *then*
the entire community must put in the resources! And that's not happening
on edk2.


> And frankly, I find it hard to find a solution for this problem, since
> the vast majority of the firmware product sits *outside* of the
> open-source repos, sometimes with gnarly forks in the way. How do we
> convince more people to engage out in the open?

We can't.

They don't see a return on their investment -- and their opinion is
likely supported by hard numbers.

(I suspect that a heavy-weight, slow review process is a repelling factor.)

[...]


>> Same story could apply here -- we could orphan S3 stuff as well.
>>
>> However, I can't deny I'm quite cranky at the thought of S3 breaking, at
>> least in my trusty old configurations, so I'd certainly like to keep an
>> eye on the S3 modules -- even if that only consisted of me
>> regression-testing patches under OVMF (and not providing "expert
>> feedback" on patch contents).
>>
> 
> I know Intel has abandoned S3 over the (equally cursed!) S0ix/S2idle.

Huh, that's news to me. Thanks for the heads-up!


> I don't know how the situation is on AMD/ARM. Do QEMU/libvirt/whatever
> use S3 for anything useful? I know they do roughly support it.

Putting a guest to S3 sleep is good for two purposes:

- It's better than just pausing the guest, because the guest will be
aware that time stops / there's going to be a time discontinuity.

- If you want to put the *host* to S3 sleep, while guests exist, that's
a can of worms, and if you put the guests to S3 sleep *first*, it
becomes a lesser can of worms. :)

Regarding "roughly" -- as long as guest kernel drivers participate, S3
should be stable, I don't perceive anything to be "rough" about it.

(Note that "guest kernel drivers participate" is a serious condition --
it's the reason why RHEL does *not* support S3 for guests :))


>>>> @@ -326,7 +302,6 @@ F: MdeModulePkg/Library/DxeSecurityManagementLib/
>>>>  F: MdeModulePkg/Universal/PCD/
>>>>  F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
>>>>  F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
>>>> -R: Dandan Bi <dandan.bi@intel.com> [dandanbi]
>>>>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
>>>
>>> Down to one reviewer.
>>
>> I'll try to assist whenever I can, wherever I notice something
>> interesting -- I'm quite sure I'm going to be overwhelmed incredibly
>> quickly, but at least I have that intent right now.
> 
> I really appreciate your involvement in EDK2, but if the alternative
> for "Liming 'owns' half of EDK2.git" is "Liming AND Laszlo 'own' half
> of EDK2.git", I don't see how this can be sustainable without
> maintainer burnout :/

*ownership* is not sustainable, indeed.

But what can we do? If a reviewer won't (or can't) continue in their
original role, we can't force them. So we either redistribute, or just
let go of the strict review requirement (or a mixture of both).

NB, one special case of "letting go of the strict review requirement" is
*forking* edk2. Basically switching from an "upstream first" to an
"upstream second" (or well, "upstream never") approach.

AIUI, Project Mu is "upstream second". Early Linaro trees were (again,
AIUI) "upstream second". The "gnarly forks" you mention are "upstream
never".

Easing up on the review requirements could be a means to preserve the
centrality / primacy (?) of edk2. We'd not "insist" on reviews, and
correspondingly, we'd not "own" subsystems. Dunno, just brainstorming.

[...]


> How is CSM looking in the virtualization space? Individual QEMU users
> probably wouldn't miss it much (since SeaBIOS exists and even is the
> default), but I don't know what you folks are using for firmware.

CSM is incredibly complicated and effectively undebuggable. I had had
some serious struggles with it, years ago, back when KVM's emulation for
real mode was rougher around the edges. Those experiences scarred me
enough that I upfront told everyone on my team that I would never ever
consider supporting CSM in any RHEL build of OVMF.

Then, in a virtual machine (considering the hardware description, such
as the libvirt domain XML -- not meaning an already installed guest
OS!), it's really simple to pick your firmware (this was a great point
that Gerd made originally). So if you want BIOS, just pick SeaBIOS and
be done with it.

David Woodhouse had a very special (arguably: "obscure") use case in
AWS, if I remember correctly. That was the sole reason why we had
migrated CSM from Intel(Framework)ModulePkg to OVMF:

  https://bugzilla.tianocore.org/show_bug.cgi?id=1811

But now, upon the removal of David's "R" entry, I figure there's just
nobody left who cares.

[...]

>>>> @@ -648,12 +603,10 @@ M: Zhichao Gao <zhichao.gao@intel.com> [ZhichaoGao]
>>>>  SignedCapsulePkg
>>>>  F: SignedCapsulePkg/
>>>>  W: https://github.com/tianocore/tianocore.github.io/wiki/SignedCapsulePkg
>>>> -M: Jian J Wang <jian.j.wang@intel.com> [jwang36]
>>>
>>> Unmaintained
>>
>> Probably best to mark it as orphaned then!
> 
> Wasn't Intel big on capsules?

This is another instance of the same discussion on orphaned modules :)

I can imagine that SignedCapsulePkg is so stable / finished that it
needs no dedicated maintainers. That doesn't mean it's fallen out of use.

If you think about it, stable software that never gets new features,
only the occasional bugfix, is the best software for *some* users.

(For example, I know I want *exactly that*, of my desktop environment!)

[...]

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110310): https://edk2.groups.io/g/devel/message/110310
Mute This Topic: https://groups.io/mt/102245264/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-