Re: 回复: [edk2-devel] A proposal to reduce incompatible case

["Zhiguang Liu" <zhiguang.liu@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 8226 bytes --]

Hi all,



I write a python script to do the work, including classify all the library instance, generating the including lib file.

In the attachment file, the first sheet includes all "Single-instance", which means in edk2 and edk2-platforms repo, only one inf files has the specified library name.
The second sheet includes all "Multi-Single-instance", which means in edk2 and edk2-platforms repo, for a specified module type and arch, this inf is a "Single-instance".

I think the library instance in the first two sheet can be extracted as a XXXLibs.dsc.inc



Here is the draft code

https://github.com/LiuZhiguang001/edk2/commits/extract_lib



Here is the source code of this tool

https://github.com/LiuZhiguang001/MyTool/tree/extract_lib



Please review the excel and the draft code.

If no comments, I will send out a formal patch next week.



Thanks

Zhiguang



> -----Original Message-----

> From: Liu, Zhiguang

> Sent: Saturday, November 21, 2020 2:08 PM

> To: Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel

> <ard.biesheuvel@arm.com>; gaoliming <gaoliming@byosoft.com.cn>;

> devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Feng, Bob C

> <bob.c.feng@intel.com>; Tian, Hot <hot.tian@intel.com>; 'Bret Barkelew'

> <bret@corthon.com>

> Cc: Bi, Dandan <dandan.bi@intel.com>; 'Chao Zhang'

> <chao.b.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A

> <hao.a.wu@intel.com>; 'Liming Gao' <liming.gao@intel.com>; Justen, Jordan L

> <jordan.l.justen@intel.com>; 'Andrew Fish' <afish@apple.com>; Ni, Ray

> <ray.ni@intel.com>; 'Bret Barkelew' <brbarkel@microsoft.com>; Kinney,

> Michael D <michael.d.kinney@intel.com>; debtech@gmail.com;

> awarkentin@vmware.com; michael.kubacki@outlook.com

> Subject: RE: 回复: [edk2-devel] A proposal to reduce incompatible case

>

> Hi all,

> Thanks all for the comments.

>

> Hi Jiewen:

> I think we are thinking from the different aspects.

> I want the XXPkgLib.dsc.inc to specify the default library instance that the

> package will consumes.

> You may want it to specify the default library instance that the package will

> produce.

> I now think your way makes more sense, and also align with the implement in

> NetworkPkg.

> I will follow your way when working on the demo code.

>

> Hi Bret:

> I see you point about that platform should be like platform and should only

> change in the stable tag.

> I partly agree with you, but in fact there are some projects need to keep the

> Edk2 updated frequently.

> And my proposal should also be an optional way to add the library instance.

> Platform dsc can also choose the original way. I think it is at least harmless if

> some platforms choose not to use this way.

>

> Hi Lazlo:

> I agree with you that this proposal should only extract "Single-instance".

> After all, this proposal is meant to reduce incompatible case like this

> VaribalePolicyLib.

> I want to the platform to be "Seamless update" in such case.

> However, if this lib has two instances, it is a platform's choice and platform

> owner should be aware the change.

> Therefore, "Single-instance" and "Multiple-instance" should be totally different

> case, and we should only enable this proposal to "Single-instance".

>

> Hi Liming and Ard,

> Thanks for liking this idea.

> I plan to work on the demo code next week and send it here for more

> comments.

>

> BTW, about the file name, I want to it to follow the existing rule of NetworkPkg

> and ArmVirtPkg.

> I think for MdeModulePkg, "MdeModuleLibs.dsc.inc" will be better.

>

> Thanks

> Zhiguang

>

> > -----Original Message-----

> > From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>

> > Sent: Friday, November 20, 2020 7:18 PM

> > To: Ard Biesheuvel <ard.biesheuvel@arm.com<mailto:ard.biesheuvel@arm.com>>; gaoliming

> > <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Yao, Jiewen

> > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>;

> > michael.kubacki@outlook.com<mailto:michael.kubacki@outlook.com>; awarkentin@vmware.com<mailto:awarkentin@vmware.com>;

> debtech@gmail.com<mailto:debtech@gmail.com>;

> > Feng, Bob C <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>>; Tian, Hot <hot.tian@intel.com<mailto:hot.tian@intel.com>>

> > Cc: 'Bret Barkelew' <bret@corthon.com<mailto:bret@corthon.com>>; Bi, Dandan

> > <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; 'Chao Zhang' <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>; Wang,

> > Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>;

> > 'Liming Gao' <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Justen, Jordan L

> > <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; 'Andrew Fish' <afish@apple.com<mailto:afish@apple.com>>; Ni, Ray

> > <ray.ni@intel.com<mailto:ray.ni@intel.com>>; 'Bret Barkelew' <brbarkel@microsoft.com<mailto:brbarkel@microsoft.com>>; Kinney,

> > Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>

> > Subject: Re: 回复: [edk2-devel] A proposal to reduce incompatible case

> >

> > On 11/20/20 10:02, Ard Biesheuvel wrote:

> > > On 11/20/20 8:27 AM, gaoliming wrote:

> > >> Zhiguang:

> > >>    This proposal can reduce the potential library class dependency.

> > >> Each package has its xxxPkgLib.dsc.inc file that includes the

> > >> library instances from this package.

> > >>    Platform DSC can include the required package lib.dsc.inc file

> > >> for the library instances. Can you work out the code changes to

> > >> demonstrate this idea?

> > >>

> > >

> > > +1 for this idea. This would allow us to remove a *lot* of

> > > +boilerplate

> > > in the .DSC files, and focus on the libraries that actually matter

> > > for the platform at hand.

> >

> > I feel like I'm in *cautious* agreement with this idea.

> >

> > In particular, I'd *only* like those lib class -> lib instance

> > resolutions to be extracted, to DSC include files, that *cannot*

> > involve platform choice. To put it differently, single-instance lib classes.

> >

> > ("Single-instance" can be interpreted per module type / per

> > architecture of course -- so if a lib class has exactly 1 instance for

> > PEIMs and exactly 1

> > (different) instance for DXE_DRIVERs, then those resolutions qualify

> > for

> > extraction.)

> >

> > If a library class genuinely has multiple instances in core edk2, then

> > extracting a "default" resolution would be *wrong*, in my opinion.

> > Every platform needs to make the choice explicitly, in such cases.

> > This applies even if a lib class only has two instances, a functional

> > one, and a Null one. The functional one should not be assumed default.

> >

> > Example: extracting the UefiLib resolution is valid. Extracting a

> > SerialPortLib class resolution is invalid.

> >

> > Basically, I'd like to avoid *overrides*. Overrides are terribly hard

> > to reason about.

> >

> > ... If someone wants to work on this, they'll have to implement this

> > with *super

> > small* patches, where every patch extracts resolutions for just one

> > lib class. I would reject any patch that required me to review the

> > extraction of two or more lib classes, from an OVMF or ArmVirt DSC file, at

> the same time.

> >

> > There is big potential in this proposal for making platform DSC files

> > *permanently* more difficult to understand & analyze. I don't

> > immediately see this proposal as a good solution for avoiding the

> > current kind of platform DSC breakage. Platform CI would be much

> > better, in my opinion. The proposal does seem workable, but the

> implementation needs to be surgical, IMO.

> >

> > OK, I'll get off my soap box now.

> >

> > Thanks

> > Laszlo



[-- Attachment #1.2: Type: text/html, Size: 17396 bytes --]

[-- Attachment #2: Edk2 Library All.xlsx --]
[-- Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, Size: 37609 bytes --]