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

[Laszlo Ersek]

On 11/29/20 13:31, Liu, Zhiguang wrote:
> 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.

I've briefly checked the first two sheets of the spreadsheet, regarding
OvmfPkg and ArmVirtPkg. Those parts look sensible to me.

Regarding the code, namely the patch "OvmfPkg:Add OvmfLibs.dsc.inc" -- I
really dislike even briefly checking patches that are not on the list,
as we then get into a back-and-forth about them, and then it would just
make more sense to post the patch to the list, and discuss it there.
But, in this case, I did take a quick look.

I don't like the approach. First, the new DSC include file is apparently
not put to use in the OvmfPkg DSC files at all. IIUC, the DSC include
files should not only come as additions, but they should replace
existent lib class resolutions where they can. Second (and I stated this
before), given a new DSC include file, I'd like to see each affected
library class resolution to be migrated *one by one* from the DSC
file(s) to the new DSC include file. If you have 10 affected lib classes
for OVMF, that will take 10 patches for OVMF. I will have to validate
every lib class resolution movement one by one anyway, in isolation (I
won't just blindly trust the python tool that generates these new files,
sorry), and for that kind of review, I need the patches to be broken-out
accordingly.

If this creates too many patches in the series (for example the first
sheet in the spreadsheet has 144 rows), then I suggest writing a
separate series for each package, or something similar.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68082): https://edk2.groups.io/g/devel/message/68082
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]
-=-=-=-=-=-=-=-=-=-=-=-