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

[Zhiguang Liu]

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 at redhat.com>
> Sent: Friday, November 20, 2020 7:18 PM
> To: Ard Biesheuvel <ard.biesheuvel at arm.com>; gaoliming
> <gaoliming at byosoft.com.cn>; devel at edk2.groups.io; Yao, Jiewen
> <jiewen.yao at intel.com>; Liu, Zhiguang <zhiguang.liu at intel.com>;
> michael.kubacki at outlook.com; awarkentin at vmware.com;
> debtech at gmail.com; Feng, Bob C <bob.c.feng at intel.com>; Tian, Hot
> <hot.tian at intel.com>
> Cc: 'Bret Barkelew' <bret at corthon.com>; Bi, Dandan <dandan.bi at intel.com>;
> 'Chao Zhang' <chao.b.zhang at intel.com>; Wang, Jian J <jian.j.wang at intel.com>;
> Wu, Hao A <hao.a.wu at intel.com>; 'Liming Gao' <liming.gao at intel.com>;
> Justen, Jordan L <jordan.l.justen at intel.com>; 'Andrew Fish' <afish at apple.com>;
> Ni, Ray <ray.ni at intel.com>; 'Bret Barkelew' <brbarkel at microsoft.com>;
> Kinney, Michael D <michael.d.kinney at 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67776): https://edk2.groups.io/g/devel/message/67776
Mute This Topic: https://groups.io/mt/78384573/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-