From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 23 Apr 2019 05:25:43 -0700 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8DA482D7E0; Tue, 23 Apr 2019 12:25:41 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-235.rdu2.redhat.com [10.10.120.235]) by smtp.corp.redhat.com (Postfix) with ESMTP id A17872719E; Tue, 23 Apr 2019 12:25:36 +0000 (UTC) Subject: Re: [edk2] Request to add new edk2-libc repository To: "Kinney, Michael D" , "devel@edk2.groups.io" Cc: "Carsey, Jaben" , "Daryl McDaniel (edk2-lists@mc2research.org)" , "leif.lindholm@linaro.org" , "Andrew Fish (afish@apple.com)" References: From: "Laszlo Ersek" Message-ID: <8c6e03ea-a928-48f0-4e4f-3bb021ea6e40@redhat.com> Date: Tue, 23 Apr 2019 14:25:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 23 Apr 2019 12:25:42 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 , 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