Re: [edk2] Request to add new edk2-libc repository

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

Hello Mike,

On 04/20/19 02:07, Kinney, Michael D wrote:
> Hello,
>
> There were no objections to the following RFC to add
> a new edk2-libc repository.
>
>     https://edk2.groups.io/g/devel/message/35211
>
> I have entered the following Feature Request Bugzilla
>
>     https://bugzilla.tianocore.org/show_bug.cgi?id=1734
>
> I have posted a version of the edk2-libc repository for
> review at the following location.  It includes all the
> history for the three packages from the edk2 repository.
>
>     https://github.com/mdkinney/edk2-libc
>
> Please review this branch.

Can we document how this branch was extracted from the edk2 history? (I
assume some form of branch rewriting.)

If it's documented already, then I apologize for missing it.

> There is a single commit to update the Readme.md and Maintainers.txt
> that scopes them to this new edk2-libc repository.

So it seems that said single commit (7e1bdd700213, "edk2-libc: Reduce
scope of Readme.md and Maintainer.txt", 2019-04-19) is the only manually
written one (not a result of branch rewriting). Is that right?

... Possible improvements for this commit:

- Can we not duplicate the "Tianocore Stewards" section? Perhaps we can
  include a pointer to the edk2 "Maintainers.txt" file.

- "Responsible Disclosure, Reporting Security Issues": according to
  <https://bugzilla.tianocore.org/show_bug.cgi?id=1510#c1>, no issues
  found under StdLib will be classified as security issues. I think we
  should reflect that decision here.

- "EDK II Releases": I think if we split off StdLib, then "releases"
  will have to be defined from scratch. (But see my general objection
  (1) below, anyway.)

----*----

I've now re-read my comments under the RFC:

  [edk2] [RFC v2] Proposal to add edk2-libc

  https://lists.01.org/pipermail/edk2-devel/2019-January/035341.html
  https://edk2.groups.io/g/devel/message/35321
  http://mid.mail-archive.com/578c1f6c-945e-2f00-0cb4-d67f9dbdd50e@redhat.com

There I wrote,

> I'm not sure how closely the StdLib internals are tied to day-to-day
> changes in core edk2; that is, whether we should keep those histories
> interlinked. That's something for the StdLib maintainers to evaluate.
> Personally I don't remember many StdLib changes, so there seems to be
> a genuine separation that supports the new repo idea.

I'm not going back on those specific thoughts, but I'd like to voice my
disagreement on two points, one general and one specific.

(1) My general objection is that this change seems to set a precedent
    for fragmenting the edk2 repository into multiple repositories. I'm
    opposed to that. I'm *now* seeing the removal of StdLib as an action
    for establishing prior art while it doesn't hurt in practice, and
    then using it as "past evidence" in support of splitting off more
    packages and modules. While I don't particularly mind StdLib, I
    definitely object to such a *trend*. (When I last commented on the
    RFC, in January, I didn't expect it to become a trend. I do worry
    about it now.)

(2) My specific objection is that "Applications/OrderedCollectionTest"
    is a unit test application for MdePkg's OrderedCollectionLib class.
    This application has two relevant traits:

    (2a) it depends on stdio for consuming input and producing output
         (please see the commit message on 424d84556d4d, "AppPkg:
         introduce OrderedCollectionTest", 2014-08-12),

    (2b) it must be in sync with the OrderedCollectionLib class, and the
         (main) OrderedCollectionLib instance(s), at all times.

    Due to (2b), I don't think this application should be removed from
    the core edk2 repository (it's a validation tool). And, wrt. (2a), I
    wouldn't like to give up the option of writing test apps /
    "validators" that consume LibC -- the standard C library APIs allow
    contributors to focus on the interfaces and tasks they actually want
    to test.

    I believe that, for "Applications/OrderedCollectionTest", it should
    be "sort of" OK to split off StdLib; given that the application
    assumes that StdLib "just works", and StdLib is not the app's main
    focus. The standard C interfaces are specified separately
    (independently of edk2), and so OrderedCollectionTest can be written
    against ISO C, and we can expect users to make "some version" of
    StdLib available through PACKAGES_PATH.

    The same is not true of the
    OrderedCollectionTest<->OrderedCollectionLib connection, which is
    why I think the app itself should remain in core edk2. Perhaps we
    should move the app under "MdeModulePkg/Application" first.

I'm not sure about the validation role of the other apps under AppPkg.
For example, "AppPkg/Applications/Sockets" used to be whole-sale helpful
for SNP driver testing. However, I agree it is different: first because
we now have HTTP boot over both IPv4 and IPv6, which is a good way to
test TCP and everything below, and second because an SNP driver again
implements standardized interfaces (namely from the UEFI spec), so an
external project such as the UEFI SCT can be used to check SNP drivers.

However, lib classes are entirely internal to edk2, and so if an app
exists to validate instances of a given lib class, then the app too
should stay within edk2.

----*----

BTW... it now occurs to me that once in the past you referred to a unit
test suite for SafeIntLib:

  https://lists.01.org/pipermail/edk2-devel/2018-February/021551.html
  http://mid.mail-archive.com/E92EE9817A31E24EB0585FDF735412F5B896820A@ORSMSX113.amr.corp.intel.com

In my opinion, that application too should be brought into core edk2. It
should share a common git history with the SafeIntLib class, and the
(main) SafeIntLib instance.


Sorry about the wall of text; in summary, I'd like to preserve
OrderedCollectionTest in core edk2, and I'd like to speak out very
clearly against setting a trend for fragmenting edk2 into a multitude of
sub-repositories. I don't mind StdLib and AppPkg in isolation.

Thanks,
Laszlo