[edk2-devel] [PATCH] BaseTools: Add a checking for Sources section in INF file
Liming Gao
liming.gao at intel.com
Fri May 24 04:58:53 UTC 2019
Besides, I suggest to separate the change on add new checker. Another patch is about hash logic enhancement.
Thanks
Liming
>-----Original Message-----
>From: Rodriguez, Christian
>Sent: Thursday, May 23, 2019 9:21 PM
>To: Feng, Bob C <bob.c.feng at intel.com>; devel at edk2.groups.io
>Cc: Gao, Liming <liming.gao at intel.com>; Zhu, Yonghong
><yonghong.zhu at intel.com>
>Subject: RE: [edk2-devel] [PATCH] BaseTools: Add a checking for Sources
>section in INF file
>
>Ok, I can show the build time performance impact here. Though the way I
>invalidated a module/library build is hash specific. What is the best way to
>invalidate a build in the original incremental build? Or does it matter because
>all the modules/libraries are rebuilt or checked for rebuild in the original
>incremental build?
>
>I'm guessing it's that second choice above, so I can move the conditional check
>to just the hash invalidation part and have the rest of the header source check
>always on.
>
>Thanks,
>Christian
>
>>-----Original Message-----
>>From: Feng, Bob C
>>Sent: Thursday, May 23, 2019 2:39 AM
>>To: devel at edk2.groups.io; Rodriguez, Christian
>><christian.rodriguez at intel.com>
>>Cc: Gao, Liming <liming.gao at intel.com>; Zhu, Yonghong
>><yonghong.zhu at intel.com>
>>Subject: RE: [edk2-devel] [PATCH] BaseTools: Add a checking for Sources
>>section in INF file
>>
>>Hi Christian,
>>
>>Since this is a INF spec required checking, I think the condition of this check
>>should be removed.
>>Would you show the build time to see what's the performance impact if we
>>always do this check?
>>
>>Thanks,
>>Bob
>>
>>-----Original Message-----
>>From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
>>Christian Rodriguez
>>Sent: Tuesday, May 21, 2019 10:13 PM
>>To: devel at edk2.groups.io
>>Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
>><liming.gao at intel.com>; Zhu, Yonghong <yonghong.zhu at intel.com>
>>Subject: [edk2-devel] [PATCH] BaseTools: Add a checking for Sources section
>>in INF file
>>
>>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>>
>>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
>patch
>>since information is already being fetched for Makefile purposes. All other
>>information is already cached in memory. No extra IO time is needed.
>>
>>Signed-off-by: Christian Rodriguez <christian.rodriguez at intel.com>
>>Cc: Bob Feng <bob.c.feng at intel.com>
>>Cc: Liming Gao <liming.gao at intel.com>
>>Cc: Yonghong Zhu <yonghong.zhu at intel.com>
>>---
>> BaseTools/Source/Python/AutoGen/AutoGen.py | 6 ++++--
>> BaseTools/Source/Python/AutoGen/GenMake.py | 45
>>+++++++++++++++++++++++++++++++++++++++++++++
>> BaseTools/Source/Python/Common/GlobalData.py | 3 ++-
>> BaseTools/Source/Python/build/build.py | 66
>>++++++++++++++++++++++++++++++++++++++++--------------------------
>> 4 files changed, 91 insertions(+), 29 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) == 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 = []
>>diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
>>b/BaseTools/Source/Python/AutoGen/GenMake.py
>>index 0e0f9fd9b0..8765ffa188 100644
>>--- a/BaseTools/Source/Python/AutoGen/GenMake.py
>>+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>>@@ -905,6 +905,51 @@ cleanlib:
>> ForceIncludedFile,
>> self._AutoGenObject.IncludePathList +
>>self._AutoGenObject.BuildOptionIncPathList
>> )
>>+
>>+ # Only do it during hashing feature for now to save time on clean build
>>+ # Conditional can be removed to happen on all builds for MetaFile
>>compliance
>>+ if GlobalData.gUseHashCache:
>>+ # Check if header files are listed in metafile
>>+ # Get a list of unique module header source files from MetaFile
>>+ headerFilesInMetaFileSet = set()
>>+ for aFile in self._AutoGenObject.SourceFileList:
>>+ aFileName = str(aFile)
>>+ if not aFileName.endswith('.h'):
>>+ continue
>>+ headerFilesInMetaFileSet.add(aFileName.lower())
>>+
>>+ # Get a list of unique module autogen files
>>+ localAutoGenFileSet = 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 = set()
>>+ localSourceDir = str(self._AutoGenObject.SourceDir).lower()
>>+ for Dependency in FileDependencyDict.values():
>>+ for aFile in Dependency:
>>+ aFileName = 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
>>architecture
>>+ if self._AutoGenObject.Arch not in GlobalData.gModuleBuildTracking:
>>+
>>+ GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch] = dict()
>>+
>>+ # Check if a module dependency header file is missing from the
>>module's MetaFile
>>+ for aFile in headerFileDependencySet:
>>+ if aFile not in headerFilesInMetaFileSet:
>>+
>>GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch][self._AutoGe
>n
>>Object] = 'FAIL_METAFILE'
>>+ EdkLogger.warn("build","Module MetaFile [Sources] is missing
>>local header!",
>>+ ExtraData = "Local Header: " + aFile + " not found in " +
>>self._AutoGenObject.MetaFile.Path
>>+ )
>>+
>> DepSet = 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 = False
>>gSikpAutoGenCache = set()
>>
>> # Dictionary for tracking Module build status as success or failure -# False ->
>>Fail : True -> Success
>>+# Top Dict: Key: Arch Type Value: Dictionary
>>+# Second Dict: Key: AutoGen Obj Value:
>'SUCCESS'\'FAIL'\'FAIL_METAFILE'
>> gModuleBuildTracking = 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 027061191c..0a58dd16ef 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 = "%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] = 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 check test, and
>>built successfully
>>+ if (self.BuildItem.BuildObject.Arch in GlobalData.gModuleBuildTracking
>>and
>>+ self.BuildItem.BuildObject in
>>GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch] and
>>+
>>GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.Buil
>dI
>>tem.BuildObject] != 'FAIL_METAFILE' and
>>+ not BuildTask._ErrorFlag.isSet()
>>+ ):
>>+
>>GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.Buil
>dI
>>tem.BuildObject] = 'SUCCESS'
>>+
>> # indicate there's a thread is available for another build task
>> BuildTask._RunningQueueLock.acquire()
>> BuildTask._RunningQueue.pop(self.BuildItem)
>>@@ -1154,27 +1162,27 @@ class Build():
>> #
>> #
>> def invalidateHash(self):
>>- # GlobalData.gModuleBuildTracking contains only modules that cannot
>>be skipped by hash
>>- for moduleAutoGenObj in GlobalData.gModuleBuildTracking.keys():
>>- # False == FAIL : True == Success
>>- # Skip invalidating for Successful module builds
>>- if GlobalData.gModuleBuildTracking[moduleAutoGenObj] == True:
>>- continue
>>+ # 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][moduleAutoG
>e
>>nObj] == 'SUCCESS':
>>+ continue
>>
>>- # The module failed to build or failed to start building, from 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 = path.join(moduleAutoGenObj.BuildDir,
>>moduleAutoGenObj.Name + ".hash")
>>- if os.path.exists(ModuleHashFile):
>>- os.remove(ModuleHashFile)
>>+ # Remove .hash from build
>>+ if GlobalData.gUseHashCache:
>>+ ModuleHashFile = 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 = path.join(GlobalData.gBinCacheDest,
>>moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir,
>>moduleAutoGenObj.MetaFile.BaseName)
>>- HashFile = path.join(FileDir, moduleAutoGenObj.Name + '.hash')
>>- if os.path.exists(HashFile):
>>- os.remove(HashFile)
>>+ # Remove .hash file from cache
>>+ if GlobalData.gUseHashCache and GlobalData.gBinCacheDest:
>>+ FileDir = os.path.join(GlobalData.gBinCacheDest,
>>moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir,
>>moduleAutoGenObj.MetaFile.BaseName)
>>+ HashFile = os.path.join(FileDir, moduleAutoGenObj.Name +
>>'.hash')
>>+ if os.path.exists(HashFile):
>>+ os.remove(HashFile)
>>
>> ## Build a module or platform
>> #
>>@@ -1833,9 +1841,11 @@ class Build():
>> if self.Target == "genmake":
>> return True
>> self.BuildModules.append(Ma)
>>- # Initialize all modules in tracking to False (FAIL)
>>- if Ma not in GlobalData.gModuleBuildTracking:
>>- GlobalData.gModuleBuildTracking[Ma] = False
>>+ # Initialize all modules in tracking to 'FAIL'
>>+ if Ma.Arch not in GlobalData.gModuleBuildTracking:
>>+ GlobalData.gModuleBuildTracking[Ma.Arch] = dict()
>>+ if Ma not in GlobalData.gModuleBuildTracking[Ma.Arch]:
>>+ GlobalData.gModuleBuildTracking[Ma.Arch][Ma] = 'FAIL'
>> self.AutoGenTime += int(round((time.time() - AutoGenStart)))
>> MakeStart = time.time()
>> for Ma in self.BuildModules:
>>@@ -1919,6 +1929,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) @@ -2017,9 +2028,11 @@ class
>>Build():
>> if self.Target == "genmake":
>> continue
>> self.BuildModules.append(Ma)
>>- # Initialize all modules in tracking to False (FAIL)
>>- if Ma not in GlobalData.gModuleBuildTracking:
>>- GlobalData.gModuleBuildTracking[Ma] = False
>>+ # Initialize all modules in tracking to 'FAIL'
>>+ if Ma.Arch not in GlobalData.gModuleBuildTracking:
>>+ GlobalData.gModuleBuildTracking[Ma.Arch] = dict()
>>+ if Ma not in GlobalData.gModuleBuildTracking[Ma.Arch]:
>>+ GlobalData.gModuleBuildTracking[Ma.Arch][Ma] = 'FAIL'
>> self.Progress.Stop("done!")
>> self.AutoGenTime += int(round((time.time() - AutoGenStart)))
>> MakeStart = time.time() @@ -2107,6 +2120,7 @@ class 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
>>
>>
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41300): https://edk2.groups.io/g/devel/message/41300
Mute This Topic: https://groups.io/mt/31697075/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list