See below.
>-----Original Message-----
>From: Leif Lindholm [mailto:[email protected]]
>Sent: Thursday, May 30, 2019 9:38 AM
>To: Feng, Bob C <[email protected]>
>Cc: Rodriguez, Christian <[email protected]>; Andrew Fish
><[email protected]>; Laszlo Ersek <[email protected]>; Kinney, Michael D
><[email protected]>; [email protected]; Gao, Liming
><[email protected]>; Shi, Steven <[email protected]>; Fan, ZhijuX
><[email protected]>
>Subject: Re: Edk2 BaseTools Patches.
>
>Thanks Bob, Christian,
>
>On Thu, May 30, 2019 at 03:06:48PM +0000, Feng, Bob C wrote:
>> Thanks Christian. I add some short description for the patches.
>>
>> These 5 patches are all for binary cache feature.
>>
>> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for
>> Sources section [Patch V4 1/2] BaseTools: Add a checking for Sources
>> section in INF file
>>
>> The above 2 patches is to fix the issue that The build tool uses the
>> files list under [sources] section of INF file as a input to calculate
>> a module's hash value. But in some INF files, [sources] does not list
>> all the "source" files, missing some .h files. Path 2/2 use another
>> method to get all source files for a module and patch 1/2 do a check
>> whether [sources] list all the "source" files.
>
>I'll be honest - because of the wild variance in whether .h files are listed
>in the
>[sources] section of .inf files, I have always been unsure as to whether they
>were just being ignored (and extracted on the side via mkdep or similar).
>
>If the intent is to speed up build time, would it not be better to warn the
>user
>- so they notice the problem and fix their modules, rather than adding extra
>processing time on having the tools work with broken .inf files?
>
>This does not look like material for edk2-stable201905 to me.
The patch does warn the user, so they notice the problem and fix their modules.
And somewhere in the email thread for that patch set I mentioned the processing
time is almost negligible since the information is already available in memory
and it's just a simple set lookup for existence and warning write.
It doesn't really matter if it goes in edk2-stable201905, as long as it goes in
eventually because it does fix a bug/corner-case in the hash feature.
>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> cache This patch is to resolve the problem that Build tool dose not
>> cache the library binaries now. Whiteout this patch, there is 25%
>> extra time cost to rebuild the all module dependency libraries if
>> cache miss happen.
>
>25% is a big number, so I won't argue against this. But I also won't argue for
>it -
>the BZ was raised very late in the cycle.
>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> This patch is to make the restored binary file have the current time
>> stamp not the binary file original time stamp.
>
>I can see how the current behaviour could cause problems with some CI/build
>systems. If it is properly reviewed and tested, I am OK with this one going in
>for edk2-stable201903.
>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
>> FFS FILE This patch is to support the raw ffs file rule. Now build
>> tool does not correctly handle this case:
>>
>> [Rule.Common.USER_DEFINED.MicroCode]
>> FILE RAW = $(NAMED_GUID) {
>> $(INF_OUTPUT)/$(MODULE_NAME).bin
>> }
>
>This looks like a new feature - not something that should bypass the freeze
>period for edk2-stable201905.
>Can you explain why this is needed in the stable tag as opposed to being
>available from master the day after the tag is made?
>
>Best Regards,
>
>Leif
>
>>
>>
>> Thanks,
>> Bob
>>
>> -----Original Message-----
>> From: Rodriguez, Christian
>> Sent: Thursday, May 30, 2019 10:26 PM
>> To: Leif Lindholm <[email protected]>; Feng, Bob C
>> <[email protected]>
>> Cc: Andrew Fish <[email protected]>; Laszlo Ersek <[email protected]>;
>> Kinney, Michael D <[email protected]>; [email protected];
>> Gao, Liming <[email protected]>; Shi, Steven
>> <[email protected]>; Fan, ZhijuX <[email protected]>
>> Subject: RE: Edk2 BaseTools Patches.
>>
>> Hey Leif,
>>
>> I thought I'd help Bob and gather those BZs for each thread:
>>
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF
>> file [Patch V4 2/2] BaseTools: Refactor hash tracking after checking
>> for Sources section
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> cache
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797
>>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
>> FFS FILE
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765
>>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742
>>
>> Thanks,
>> Christian
>>
>> >-----Original Message-----
>> >From: Leif Lindholm [mailto:[email protected]]
>> >Sent: Thursday, May 30, 2019 2:28 AM
>> >To: Feng, Bob C <[email protected]>
>> >Cc: Andrew Fish <[email protected]>; Laszlo Ersek <[email protected]>;
>> >Kinney, Michael D <[email protected]>; [email protected];
>> >Gao, Liming <[email protected]>; Shi, Steven
>> ><[email protected]>; Rodriguez, Christian
>> ><[email protected]>; Fan, ZhijuX <[email protected]>
>> >Subject: Re: Edk2 BaseTools Patches.
>> >
>> >Hi Bob,
>> >
>> >On Thu, May 30, 2019 at 06:39:59AM +0000, Feng, Bob C wrote:
>> >> Hi,
>> >>
>> >> Currently, we have 5 Basetools patches which are ready to push.
>> >> Since we are in the soft-freeze phase, I'd like to ask for your
>> >> opinions if those patches can be pushed to edk2 master.
>> >
>> >To save me the time of reading through all the threads and getting to
>> >grips with all the code, could you summarise the problem these solve
>> >and the impact of not including these?
>> >
>> >Is there a BZ?
>> >
>> >Regards,
>> >
>> >Leif
>> >
>> >>
>> >> These 5 patches are to fix the issues for the build cache feature.
>> >>
>> >> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for
>> >> Sources section
>> >> https://edk2.groups.io/g/devel/topic/31835556#41642
>> >>
>> >> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF
>> >> file
>> >> https://edk2.groups.io/g/devel/topic/31835555#41641
>> >>
>> >> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> >> cache
>> >> https://edk2.groups.io/g/devel/topic/31843505#41655
>> >>
>> >> [PATCH V5] BaseTools:Make BaseTools support new rules to generate
>> >> RAW FFS FILE
>> >> https://edk2.groups.io/g/devel/topic/31830807#41571
>> >>
>> >> [PATCH] BaseTools:Update binary cache restore time to current time
>> >> https://edk2.groups.io/g/devel/topic/31819590#41468
>> >>
>> >>
>> >> Thanks,
>> >> Bob
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41684): https://edk2.groups.io/g/devel/message/41684
Mute This Topic: https://groups.io/mt/31866190/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
