Re: [edk2-devel] [PATCH] BaseTools: Add a checking for Sources section in INF file
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 ; devel@edk2.groups.io >Cc: Gao, Liming ; Zhu, Yonghong > >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@edk2.groups.io; Rodriguez, Christian >> >>Cc: Gao, Liming ; Zhu, Yonghong >> >>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@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >>Christian Rodriguez >>Sent: Tuesday, May 21, 2019 10:13 PM >>To: devel@edk2.groups.io >>Cc: Feng, Bob C ; Gao, Liming >>; Zhu, Yonghong >>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 >>Cc: Bob Feng >>Cc: Liming Gao >>Cc: Yonghong Zhu >>--- >> 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 >>
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@edk2.groups.io; Rodriguez, Christian
>
>Cc: Gao, Liming ; Zhu, Yonghong
>
>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@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Christian Rodriguez
>Sent: Tuesday, May 21, 2019 10:13 PM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong
>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
>Cc: Bob Feng
>Cc: Liming Gao
>Cc: Yonghong Zhu
>---
> 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 aFi
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@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Christian
Rodriguez
Sent: Tuesday, May 21, 2019 10:13 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming ;
Zhu, Yonghong
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
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
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
[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
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
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._AutoGenObject]
= '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():
