From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: christian.rodriguez@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Thu, 23 May 2019 09:13:50 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 May 2019 09:13:49 -0700 X-ExtLoop1: 1 Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by orsmga006.jf.intel.com with ESMTP; 23 May 2019 09:13:49 -0700 Received: from orsmsx121.amr.corp.intel.com (10.22.225.226) by ORSMSX106.amr.corp.intel.com (10.22.225.133) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 23 May 2019 09:13:49 -0700 Received: from orsmsx112.amr.corp.intel.com ([169.254.3.79]) by ORSMSX121.amr.corp.intel.com ([169.254.10.64]) with mapi id 14.03.0415.000; Thu, 23 May 2019 09:13:48 -0700 From: "Christian Rodriguez" To: "devel@edk2.groups.io" , "Rodriguez, Christian" CC: "Feng, Bob C" , "Gao, Liming" , "Zhu, Yonghong" Subject: Re: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources section in INF file Thread-Topic: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources section in INF file Thread-Index: AQHVEYFxByATwr/u9UK2/5HAj4+CW6Z44Aow Date: Thu, 23 May 2019 16:13:48 +0000 Message-ID: <3A7DCC9A944C6149BF832E1C9B718ABC01F229C0@ORSMSX112.amr.corp.intel.com> References: <15A15B842F0124CD.29177@groups.io> In-Reply-To: <15A15B842F0124CD.29177@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNmQ3YzVhM2MtNTg4Yy00Y2I4LWIyOWEtMDE5MGM4MGRmMzc0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNXBMTEdoc2JPK0Zwdkg0M01OVGhaQjdJXC9jUnFQXC9yOXV1SlFyU2E5WDhzWVE5TVg1d2tVNkFNQXhhOTFLVmJiIn0= dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Return-Path: christian.rodriguez@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The performance impact seems to be irrelevant. After a few runs both on mas= ter and with the change using: build -p OvmfPkg\OvmfPkgIa32.dsc -a IA32 -t VS2015x86 I got an average build time of: No Change, Master build time Avg: 00:01:34 Header check change build time Avg: 00:01:34 Thanks, Christian >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Christian Rodriguez >Sent: Thursday, May 23, 2019 9:06 AM >To: devel@edk2.groups.io >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources >section in INF file > >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1804 > >In V2: Enable check for all builds, move conditional to hash invalidation= In the >Edk2 INF spec 3.9, it states, All HII Unicode format files must be listed= in >[Sources] section. Add a check to see if [Sources] section lists all the = "source" >type files of a module. Performance impact should be minimal with this pa= tch >since information is already being fetched for Makefile purposes. All oth= er >information is already cached in memory. No extra IO time is needed. > >Signed-off-by: Christian Rodriguez >Cc: Bob Feng >Cc: Liming Gao >Cc: Yonghong Zhu >--- > BaseTools/Source/Python/AutoGen/AutoGen.py | 6 +- > BaseTools/Source/Python/AutoGen/GenMake.py | 44 +++++++++++++ > BaseTools/Source/Python/Common/GlobalData.py | 3 +- > BaseTools/Source/Python/build/build.py | 65 ++++++++++++-------- > 4 files changed, 91 insertions(+), 27 deletions(-) > >diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py >b/BaseTools/Source/Python/AutoGen/AutoGen.py >index a843f294b9..0bc27fb2b3 100644 >--- a/BaseTools/Source/Python/AutoGen/AutoGen.py >+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py >@@ -3989,7 +3989,8 @@ class ModuleAutoGen(AutoGen): > for LibraryAutoGen in self.LibraryAutoGenList: > LibraryAutoGen.CreateMakeFile() > >- if self.CanSkip(): >+ # Don't enable if hash feature enabled, CanSkip uses timestamps = to >determine build skipping >+ if not GlobalData.gUseHashCache and self.CanSkip(): > return > > if len(self.CustomMakefile) =3D=3D 0: >@@ -4032,7 +4033,8 @@ class ModuleAutoGen(AutoGen): > for LibraryAutoGen in self.LibraryAutoGenList: > LibraryAutoGen.CreateCodeFile() > >- if self.CanSkip(): >+ # Don't enable if hash feature enabled, CanSkip uses timestamps = to >determine build skipping >+ if not GlobalData.gUseHashCache and self.CanSkip(): > return > > AutoGenList =3D [] >diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py >b/BaseTools/Source/Python/AutoGen/GenMake.py >index 0e0f9fd9b0..212ca0fa7f 100644 >--- a/BaseTools/Source/Python/AutoGen/GenMake.py >+++ b/BaseTools/Source/Python/AutoGen/GenMake.py >@@ -905,6 +905,50 @@ cleanlib: > ForceIncludedFile, > self._AutoGenObject.IncludePathList = + >self._AutoGenObject.BuildOptionIncPathList > ) >+ >+ # Check if header files are listed in metafile >+ # Get a list of unique module header source files from MetaFile >+ headerFilesInMetaFileSet =3D set() >+ for aFile in self._AutoGenObject.SourceFileList: >+ aFileName =3D str(aFile) >+ if not aFileName.endswith('.h'): >+ continue >+ headerFilesInMetaFileSet.add(aFileName.lower()) >+ >+ # Get a list of unique module autogen files >+ localAutoGenFileSet =3D set() >+ for aFile in self._AutoGenObject.AutoGenFileList: >+ localAutoGenFileSet.add(str(aFile).lower()) >+ >+ # Get a list of unique module dependency header files >+ # Exclude autogen files and files not in the source directory >+ headerFileDependencySet =3D set() >+ localSourceDir =3D str(self._AutoGenObject.SourceDir).lower() >+ for Dependency in FileDependencyDict.values(): >+ for aFile in Dependency: >+ aFileName =3D str(aFile).lower() >+ if not aFileName.endswith('.h'): >+ continue >+ if aFileName in localAutoGenFileSet: >+ continue >+ if localSourceDir not in aFileName: >+ continue >+ headerFileDependencySet.add(aFileName) >+ >+ # Ensure that gModuleBuildTracking has been initialized per arch= itecture >+ if self._AutoGenObject.Arch not in GlobalData.gModuleBuildTracki= ng: >+ GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch] = =3D >+ dict() >+ >+ # Check if a module dependency header file is missing from the m= odule's >MetaFile >+ for aFile in headerFileDependencySet: >+ if aFile in headerFilesInMetaFileSet: >+ continue >+ if GlobalData.gUseHashCache: >+ >GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch][self._AutoGen >Object] =3D 'FAIL_METAFILE' >+ EdkLogger.warn("build","Module MetaFile [Sources] is missing= local >header!", >+ ExtraData =3D "Local Header: " + aFile + " not f= ound in " + >self._AutoGenObject.MetaFile.Path >+ ) >+ > DepSet =3D None > for File,Dependency in FileDependencyDict.items(): > if not Dependency: >diff --git a/BaseTools/Source/Python/Common/GlobalData.py >b/BaseTools/Source/Python/Common/GlobalData.py >index 95e28a988f..bd45a43728 100644 >--- a/BaseTools/Source/Python/Common/GlobalData.py >+++ b/BaseTools/Source/Python/Common/GlobalData.py >@@ -110,7 +110,8 @@ gEnableGenfdsMultiThread =3D False >gSikpAutoGenCache =3D set() > > # Dictionary for tracking Module build status as success or failure -# F= alse -> >Fail : True -> Success >+# Top Dict: Key: Arch Type Value: Dictionary >+# Second Dict: Key: AutoGen Obj Value: 'SUCCESS'\'FAIL'\'FAIL_METAFI= LE' > gModuleBuildTracking =3D dict() > > # Dictionary of booleans that dictate whether a module or diff --git >a/BaseTools/Source/Python/build/build.py >b/BaseTools/Source/Python/build/build.py >index a8b4ce7375..d147e02dc6 100644 >--- a/BaseTools/Source/Python/build/build.py >+++ b/BaseTools/Source/Python/build/build.py >@@ -625,8 +625,16 @@ class BuildTask: > BuildTask._ErrorFlag.set() > BuildTask._ErrorMessage =3D "%s broken\n %s [%s]" % \ > (threading.currentThread().getName= (), Command, >WorkingDir) >- if self.BuildItem.BuildObject in GlobalData.gModuleBuildTracking= and not >BuildTask._ErrorFlag.isSet(): >- GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject] = = =3D True >+ >+ # Set the value used by hash invalidation flow in >GlobalData.gModuleBuildTracking to 'SUCCESS' >+ # If Module or Lib is being tracked, it did not fail header chec= k test, and >built successfully >+ if (self.BuildItem.BuildObject.Arch in GlobalData.gModuleBuildTr= acking >and >+ self.BuildItem.BuildObject in >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch] and >+ >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.Bui= ldI >tem.BuildObject] !=3D 'FAIL_METAFILE' and >+ not BuildTask._ErrorFlag.isSet() >+ ): >+ >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.Bui= ldI >tem.BuildObject] =3D 'SUCCESS' >+ > # indicate there's a thread is available for another build task > BuildTask._RunningQueueLock.acquire() > BuildTask._RunningQueue.pop(self.BuildItem) >@@ -1154,27 +1162,30 @@ class Build(): > # > # > def invalidateHash(self): >- # GlobalData.gModuleBuildTracking contains only modules that can= not >be skipped by hash >- for moduleAutoGenObj in GlobalData.gModuleBuildTracking.keys(): >- # False =3D=3D FAIL : True =3D=3D Success >- # Skip invalidating for Successful module builds >- if GlobalData.gModuleBuildTracking[moduleAutoGenObj] =3D=3D = True: >- continue >+ # Only for hashing feature >+ if not GlobalData.gUseHashCache: >+ return >+ >+ # GlobalData.gModuleBuildTracking contains only modules or libs = that >cannot be skipped by hash >+ for moduleAutoGenObjArch in GlobalData.gModuleBuildTracking.keys= (): >+ for moduleAutoGenObj in >GlobalData.gModuleBuildTracking[moduleAutoGenObjArch].keys(): >+ # Skip invalidating for Successful Module/Lib builds >+ if >GlobalData.gModuleBuildTracking[moduleAutoGenObjArch][moduleAutoGe >nObj] =3D=3D 'SUCCESS': >+ continue > >- # The module failed to build or failed to start building, fr= om this point >on >+ # The module failed to build, failed to start building, >+ or failed the header check test from this point on > >- # Remove .hash from build >- if GlobalData.gUseHashCache: >- ModuleHashFile =3D path.join(moduleAutoGenObj.BuildDir, >moduleAutoGenObj.Name + ".hash") >+ # Remove .hash from build >+ ModuleHashFile =3D >+ os.path.join(moduleAutoGenObj.BuildDir, moduleAutoGenObj.Name + >+ ".hash") > if os.path.exists(ModuleHashFile): > os.remove(ModuleHashFile) > >- # Remove .hash file from cache >- if GlobalData.gBinCacheDest: >- FileDir =3D path.join(GlobalData.gBinCacheDest, >moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, >moduleAutoGenObj.MetaFile.BaseName) >- HashFile =3D path.join(FileDir, moduleAutoGenObj.Name + = '.hash') >- if os.path.exists(HashFile): >- os.remove(HashFile) >+ # Remove .hash file from cache >+ if GlobalData.gBinCacheDest: >+ FileDir =3D os.path.join(GlobalData.gBinCacheDest, >moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, >moduleAutoGenObj.MetaFile.BaseName) >+ HashFile =3D os.path.join(FileDir, moduleAutoGenObj.= Name + >'.hash') >+ if os.path.exists(HashFile): >+ os.remove(HashFile) > > ## Build a module or platform > # >@@ -1829,9 +1840,11 @@ class Build(): > if self.Target =3D=3D "genmake": > return True > self.BuildModules.append(Ma) >- # Initialize all modules in tracking to Fals= e (FAIL) >- if Ma not in GlobalData.gModuleBuildTracking= : >- GlobalData.gModuleBuildTracking[Ma] =3D = False >+ # Initialize all modules in tracking to 'FAI= L' >+ if Ma.Arch not in GlobalData.gModuleBuildTra= cking: >+ GlobalData.gModuleBuildTracking[Ma.Arch]= =3D dict() >+ if Ma not in GlobalData.gModuleBuildTracking= [Ma.Arch]: >+ GlobalData.gModuleBuildTracking[Ma.Arch]= [Ma] =3D 'FAIL' > self.AutoGenTime +=3D int(round((time.time() - AutoG= enStart))) > MakeStart =3D time.time() > for Ma in self.BuildModules: >@@ -1915,6 +1928,7 @@ class Build(): > # Save MAP buffer into MAP file. > # > self._SaveMapFile (MapBuffer, Wa) >+ self.invalidateHash() > > def _GenFfsCmd(self,ArchList): > # convert dictionary of Cmd:(Inf,Arch) @@ -2013,9 +2027,11 @@ cl= ass >Build(): > if self.Target =3D=3D "genmake": > continue > self.BuildModules.append(Ma) >- # Initialize all modules in tracking to False (F= AIL) >- if Ma not in GlobalData.gModuleBuildTracking: >- GlobalData.gModuleBuildTracking[Ma] =3D Fals= e >+ # Initialize all modules in tracking to 'FAIL' >+ if Ma.Arch not in GlobalData.gModuleBuildTrackin= g: >+ GlobalData.gModuleBuildTracking[Ma.Arch] =3D= dict() >+ if Ma not in GlobalData.gModuleBuildTracking[Ma.= Arch]: >+ GlobalData.gModuleBuildTracking[Ma.Arch][Ma]= =3D 'FAIL' > self.Progress.Stop("done!") > self.AutoGenTime +=3D int(round((time.time() - AutoG= enStart))) > MakeStart =3D time.time() @@ -2103,6 +2119,7 @@ clas= s Build(): > # Save MAP buffer into MAP file. > # > self._SaveMapFile(MapBuffer, Wa) >+ self.invalidateHash() > > ## Generate GuidedSectionTools.txt in the FV directories. > # >-- >2.19.1.windows.1 > > >