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

Join devel@edk2.groups.io to automatically receive all group messages.