From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: christian.rodriguez@intel.com) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by groups.io with SMTP; Thu, 30 May 2019 11:05:23 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 May 2019 11:05:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,532,1549958400"; d="scan'208";a="180025095" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga002.fm.intel.com with ESMTP; 30 May 2019 11:05:22 -0700 Received: from orsmsx112.amr.corp.intel.com ([169.254.3.79]) by ORSMSX101.amr.corp.intel.com ([169.254.8.107]) with mapi id 14.03.0415.000; Thu, 30 May 2019 11:05:22 -0700 From: "Christian Rodriguez" To: Leif Lindholm , "Feng, Bob C" CC: Andrew Fish , Laszlo Ersek , "Kinney, Michael D" , "devel@edk2.groups.io" , "Gao, Liming" , "Shi, Steven" , "Fan, ZhijuX" Subject: Re: Edk2 BaseTools Patches. Thread-Topic: Edk2 BaseTools Patches. Thread-Index: AdUWjQt6T2cDiXJSSJG3vQ9oXBf1IQAd5skAAARyC9AAB2LEAAADLn8AAAvzuBA= Date: Thu, 30 May 2019 18:05:22 +0000 Message-ID: <3A7DCC9A944C6149BF832E1C9B718ABC01F24F39@ORSMSX112.amr.corp.intel.com> References: <08650203BA1BD64D8AD9B6D5D74A85D160128393@SHSMSX101.ccr.corp.intel.com> <20190530092802.7belyzgxdd76ps2v@bivouac.eciton.net> <3A7DCC9A944C6149BF832E1C9B718ABC01F24E6B@ORSMSX112.amr.corp.intel.com> <08650203BA1BD64D8AD9B6D5D74A85D1601296DA@SHSMSX101.ccr.corp.intel.com> <20190530163754.shb54x46euzszpza@bivouac.eciton.net> In-Reply-To: <20190530163754.shb54x46euzszpza@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjVkZjM5NGYtYzRkYy00NjI0LWJiMDQtNjQxNDk3YjE3ODg1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicmhBOEdyQThcL2ZacG91ZW1hU0gzOWVKb3ZFUXB0NUFSdTVsMVZDSDNra3hqdVB6WEhNT3YxdTNIZUtqTTRrdHkifQ== dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable See below. >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >Sent: Thursday, May 30, 2019 9:38 AM >To: Feng, Bob C >Cc: Rodriguez, Christian ; Andrew Fish >; Laszlo Ersek ; Kinney, Michael D >; devel@edk2.groups.io; Gao, Liming >; Shi, Steven ; Fan, ZhijuX > >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 list= ed in the >[sources] section of .inf files, I have always been unsure as to whether t= hey >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 th= e user >- so they notice the problem and fix their modules, rather than adding ext= ra >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 modu= les. And somewhere in the email thread for that patch set I mentioned the p= rocessing time is almost negligible since the information is already availa= ble in memory and it's just a simple set lookup for existence and warning w= rite. It doesn't really matter if it goes in edk2-stable201905, as long as it goe= s 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/buil= d >systems. If it is properly reviewed and tested, I am OK with this one goin= g 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 =3D $(NAMED_GUID) { >> $(INF_OUTPUT)/$(MODULE_NAME).bin >> } > >This looks like a new feature - not something that should bypass the freez= e >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 ; Feng, Bob C >> >> Cc: Andrew Fish ; Laszlo Ersek ; >> Kinney, Michael D ; devel@edk2.groups.io; >> Gao, Liming ; Shi, Steven >> ; Fan, ZhijuX >> 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=3D1804 >> >> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library >> cache >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1797 >> >> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW >> FFS FILE >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1765 >> >> [PATCH] BaseTools:Update binary cache restore time to current time >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1742 >> >> Thanks, >> Christian >> >> >-----Original Message----- >> >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >> >Sent: Thursday, May 30, 2019 2:28 AM >> >To: Feng, Bob C >> >Cc: Andrew Fish ; Laszlo Ersek ; >> >Kinney, Michael D ; devel@edk2.groups.io; >> >Gao, Liming ; Shi, Steven >> >; Rodriguez, Christian >> >; Fan, ZhijuX >> >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