Re: [edk2-devel] [PATCH v4 1/5] BaseTools: Improve the cache hit in the edk2 build cache
For all 5 patches in patch set: Acked-by: Christian Rodriguez
>-Original Message-
>From: Shi, Steven
>Sent: Wednesday, August 14, 2019 11:11 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C
>; Rodriguez, Christian
>; Johnson, Michael
>; Shi, Steven
>Subject: [PATCH v4 1/5] BaseTools: Improve the cache hit in the edk2 build
>cache
>
>From: "Shi, Steven"
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1927
>
>Current cache hash algorithm does not parse and generate
>the makefile to get the accurate dependency files for a
>module. It instead use the platform and package meta files
>to get the module depenedency in a quick but over approximate
>way. These meta files are monolithic and involve many redundant
>dependency for the module, which cause the module build
>cache miss easily.
>This patch introduces one more cache checkpoint and a new
>hash algorithm besides the current quick one. The new hash
>algorithm leverages the module makefile to achieve more
>accurate and precise dependency info for a module. When
>the build cache miss with the first quick hash, the
>Basetool will caculate new one after makefile is generated
>and then check again.
>
>Cc: Liming Gao
>Cc: Bob Feng
>Signed-off-by: Steven Shi
>---
> BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 21
>+
> BaseTools/Source/Python/AutoGen/CacheIR.py | 28
>
> BaseTools/Source/Python/AutoGen/DataPipe.py | 8
> BaseTools/Source/Python/AutoGen/GenMake.py | 223
>+++
>+++---
>---
>---
> BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 639
>+++
>+++
>+++
>+++
>+++
>+++
>+++
>+++
>+++
>+++-
>
> BaseTools/Source/Python/Common/GlobalData.py | 9 +
> BaseTools/Source/Python/build/build.py | 129
>+++
>++
> 7 files changed, 863 insertions(+), 194 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
>b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
>old mode 100644
>new mode 100755
>index e583828741..a84ed46f2e
>--- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
>@@ -182,6 +182,12 @@ class AutoGenWorkerInProcess(mp.Process):
> GlobalData.gDisableIncludePathCheck = False
> GlobalData.gFdfParser = self.data_pipe.Get("FdfParser")
> GlobalData.gDatabasePath = self.data_pipe.Get("DatabasePath")
>+GlobalData.gBinCacheSource = self.data_pipe.Get("BinCacheSource")
>+GlobalData.gBinCacheDest = self.data_pipe.Get("BinCacheDest")
>+GlobalData.gCacheIR = self.data_pipe.Get("CacheIR")
>+GlobalData.gEnableGenfdsMultiThread =
>self.data_pipe.Get("EnableGenfdsMultiThread")
>+GlobalData.file_lock = self.file_lock
>+CommandTarget = self.data_pipe.Get("CommandTarget")
> pcd_from_build_option = []
> for pcd_tuple in self.data_pipe.Get("BuildOptPcd"):
> pcd_id = ".".join((pcd_tuple[0],pcd_tuple[1]))
>@@ -193,10 +199,13 @@ class AutoGenWorkerInProcess(mp.Process):
> FfsCmd = self.data_pipe.Get("FfsCommand")
> if FfsCmd is None:
> FfsCmd = {}
>+GlobalData.FfsCmd = FfsCmd
> PlatformMetaFile =
>self.GetPlatformMetaFile(self.data_pipe.Get("P_Info").get("ActivePlatform")
>,
>
> self.data_pipe.Get("P_Info").get("WorkspaceDir"))
> libConstPcd = self.data_pipe.Get("LibConstPcd")
> Refes = self.data_pipe.Get("REFS")
>+
[edk2-devel] [Patch V3] BaseTools: Fix checking for Sources section in INF file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
The check to see if [Sources] section lists all the header type
files of a module is missing the exclusion of source files that
fall under the scope of Package includes. This change adds the
exclusions.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
---
BaseTools/Source/Python/AutoGen/GenMake.py | 25 ++---
BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 15 +++
2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 5802ae5ae4..499ef82aea 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -906,8 +906,14 @@ cleanlib:
self._AutoGenObject.IncludePathList +
self._AutoGenObject.BuildOptionIncPathList
)
+# Get a set of unique package includes from MetaFile
+parentMetaFileIncludes = set()
+for aInclude in self._AutoGenObject.PackageIncludePathList:
+aIncludeName = str(aInclude)
+parentMetaFileIncludes.add(aIncludeName.lower())
+
# Check if header files are listed in metafile
-# Get a list of unique module header source files from MetaFile
+# Get a set of unique module header source files from MetaFile
headerFilesInMetaFileSet = set()
for aFile in self._AutoGenObject.SourceFileList:
aFileName = str(aFile)
@@ -915,24 +921,37 @@ cleanlib:
continue
headerFilesInMetaFileSet.add(aFileName.lower())
-# Get a list of unique module autogen files
+# Get a set 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
+# Get a set of unique module dependency header files
# Exclude autogen files and files not in the source directory
+# and files that are under the package include list
headerFileDependencySet = set()
localSourceDir = str(self._AutoGenObject.SourceDir).lower()
for Dependency in FileDependencyDict.values():
for aFile in Dependency:
aFileName = str(aFile).lower()
+# Exclude non-header files
if not aFileName.endswith('.h'):
continue
+# Exclude autogen files
if aFileName in localAutoGenFileSet:
continue
+# Exclude include out of local scope
if localSourceDir not in aFileName:
continue
+# Exclude files covered by package includes
+pathNeeded = True
+for aIncludePath in parentMetaFileIncludes:
+if aIncludePath in aFileName:
+pathNeeded = False
+break
+if not pathNeeded:
+continue
+# Keep the file to be checked
headerFileDependencySet.add(aFileName)
# Ensure that gModuleBuildTracking has been initialized per
architecture
diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index ed6822334e..aaea8767ef 100644
--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
@@ -1114,6 +1114,21 @@ class ModuleAutoGen(AutoGen):
def IncludePathLength(self):
return sum(len(inc)+1 for inc in self.IncludePathList)
+## Get the list of include paths from the packages
+#
+# @IncludesList list The list path
+#
+@cached_property
+def PackageIncludePathList(self):
+IncludesList = []
+for Package in self.Module.Packages:
+PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
+IncludesList = Package.Includes
+if Package._PrivateIncludes:
+if not self.MetaFile.Path.startswith(PackageDir):
+IncludesList =
list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
+return IncludesList
+
## Get HII EX PCDs which maybe used by VFR
#
# efivarstore used by VFR may relate with HII EX PCDs
--
2.22.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45460): https://edk2.groups.io/g/devel/message/45460
Mute This Topic: https://groups.io/mt/32841936/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2] BaseTools: Fix checking for Sources section in INF file
Yes that can be done. Please add the BZ dependency on Bugzilla.
Thanks,
Christian
>-Original Message-
>From: Feng, Bob C
>Sent: Thursday, August 1, 2019 3:57 PM
>To: devel@edk2.groups.io; Rodriguez, Christian
>
>Cc: Gao, Liming
>Subject: RE: [edk2-devel] [Patch V2] BaseTools: Fix checking for Sources
>section in INF file
>
>Hi Christian and Liming,
>
>Since this patch changes AutoGen.py, I hope this patch can be created based
>on Multiple-processes AutoGen patch set, and pushed after Multiple-
>processes AutoGen being pushed.
>
>Thanks,
>Bob
>
>
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Christian Rodriguez
>Sent: Thursday, August 1, 2019 11:33 PM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C ; Gao, Liming
>
>Subject: [edk2-devel] [Patch V2] BaseTools: Fix checking for Sources section in
>INF file
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>
>The check to see if [Sources] section lists all the header type files of a
>module
>is missing the exclusion of source files that fall under the scope of Package
>includes. This change adds the exclusions.
>
>Signed-off-by: Christian Rodriguez
>Cc: Bob Feng
>Cc: Liming Gao
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py | 15 +++
>BaseTools/Source/Python/AutoGen/GenMake.py | 25
>++---
> 2 files changed, 37 insertions(+), 3 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index 2df055a109..02bf58160b 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -3454,6 +3454,21 @@ class ModuleAutoGen(AutoGen):
> def IncludePathLength(self): return sum(len(inc)+1 for inc in
>self.IncludePathList) +## Get the list of include paths from the packages+
>#+# @IncludesList list The list path+#+
>@cached_property+
>def PackageIncludePathList(self):+IncludesList = []+for
>Package in
>self.Module.Packages:+PackageDir = mws.join(self.WorkspaceDir,
>Package.MetaFile.Dir)+IncludesList = Package.Includes+
>if
>Package._PrivateIncludes:+if not
>self.MetaFile.Path.startswith(PackageDir):+IncludesList =
>list(set(Package.Includes).difference(set(Package._PrivateIncludes)))+
>return IncludesList+ ## Get HII EX PCDs which maybe used by VFR # #
>efivarstore used by VFR may relate with HII EX PCDsdiff --git
>a/BaseTools/Source/Python/AutoGen/GenMake.py
>b/BaseTools/Source/Python/AutoGen/GenMake.py
>index 5802ae5ae4..1df55e5d61 100644
>--- a/BaseTools/Source/Python/AutoGen/GenMake.py
>+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>@@ -906,8 +906,14 @@ cleanlib:
> self._AutoGenObject.IncludePathList +
>self._AutoGenObject.BuildOptionIncPathList
>) +# Get a
>set of unique package includes from MetaFile+parentMetaFileIncludes =
>set()+for aInclude in self._AutoGenObject.PackageIncludePathList:+
>aIncludeName = str(aInclude)+
>parentMetaFileIncludes.add(aIncludeName.lower())+ # Check if header
>files are listed in metafile-# Get a list of unique module header
>source files
>from MetaFile+# Get a set of unique module header source files from
>MetaFile headerFilesInMetaFileSet = set() for aFile in
>self._AutoGenObject.SourceFileList: aFileName = str(aFile)@@
>-915,24
>+921,37 @@ cleanlib:
> continue
> headerFilesInMetaFileSet.add(aFileName.lower()) -
># Get a list of unique module autogen files+# Get a set 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+# Get a set of unique module dependency
>header files # Exclude autogen files and files not in the source
>directory+
># and files that are under the package include list
>headerFileDependencySet = set() localSourceDir =
>str(self._AutoGenObject.SourceDir).lower() for Dependency in
>FileDependencyDict.values(): for aFile in Dependency:
>aFileName = str(aFile).lower()+# Exclude non-header files
> if
>not aFileName.endswith('.h'): continue+#
>Exclude autogen
>files if aFileName in localAutoGenFileSet:
>continue+
>#
[edk2-devel] [Patch V2] BaseTools: Fix checking for Sources section in INF file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
The check to see if [Sources] section lists all the header type
files of a module is missing the exclusion of source files that
fall under the scope of Package includes. This change adds the
exclusions.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 15 +++
BaseTools/Source/Python/AutoGen/GenMake.py | 25 ++---
2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 2df055a109..02bf58160b 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3454,6 +3454,21 @@ class ModuleAutoGen(AutoGen):
def IncludePathLength(self):
return sum(len(inc)+1 for inc in self.IncludePathList)
+## Get the list of include paths from the packages
+#
+# @IncludesList list The list path
+#
+@cached_property
+def PackageIncludePathList(self):
+IncludesList = []
+for Package in self.Module.Packages:
+PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
+IncludesList = Package.Includes
+if Package._PrivateIncludes:
+if not self.MetaFile.Path.startswith(PackageDir):
+IncludesList =
list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
+return IncludesList
+
## Get HII EX PCDs which maybe used by VFR
#
# efivarstore used by VFR may relate with HII EX PCDs
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 5802ae5ae4..1df55e5d61 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -906,8 +906,14 @@ cleanlib:
self._AutoGenObject.IncludePathList +
self._AutoGenObject.BuildOptionIncPathList
)
+# Get a set of unique package includes from MetaFile
+parentMetaFileIncludes = set()
+for aInclude in self._AutoGenObject.PackageIncludePathList:
+aIncludeName = str(aInclude)
+parentMetaFileIncludes.add(aIncludeName.lower())
+
# Check if header files are listed in metafile
-# Get a list of unique module header source files from MetaFile
+# Get a set of unique module header source files from MetaFile
headerFilesInMetaFileSet = set()
for aFile in self._AutoGenObject.SourceFileList:
aFileName = str(aFile)
@@ -915,24 +921,37 @@ cleanlib:
continue
headerFilesInMetaFileSet.add(aFileName.lower())
-# Get a list of unique module autogen files
+# Get a set 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
+# Get a set of unique module dependency header files
# Exclude autogen files and files not in the source directory
+# and files that are under the package include list
headerFileDependencySet = set()
localSourceDir = str(self._AutoGenObject.SourceDir).lower()
for Dependency in FileDependencyDict.values():
for aFile in Dependency:
aFileName = str(aFile).lower()
+# Exclude non-header files
if not aFileName.endswith('.h'):
continue
+# Exclude autogen files
if aFileName in localAutoGenFileSet:
continue
+# Exclude include out of local scope
if localSourceDir not in aFileName:
continue
+# Exclude files covered by package includes
+pathNeeded = True
+for aIncludePath in parentMetaFileIncludes:
+if aIncludePath in aFileName:
+pathNeeded = False
+break
+if not pathNeeded:
+continue
+# Keep the file
headerFileDependencySet.add(aFileName)
# Ensure that gModuleBuildTracking has been initialized per
architecture
--
2.22.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44799): https://edk2.groups.io/g/devel/message/44799
Mute This Topic: https://groups.io/mt/32680387/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] BaseTools: Fix checking for Sources section in INF file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
The check to see if [Sources] section lists all the header type
files of a module is missing the exclusion of source files that
fall under the scope of Package includes. This change adds the
exclusions.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 15 +++
BaseTools/Source/Python/AutoGen/GenMake.py | 15 ---
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 2df055a109..02bf58160b 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3454,6 +3454,21 @@ class ModuleAutoGen(AutoGen):
def IncludePathLength(self):
return sum(len(inc)+1 for inc in self.IncludePathList)
+## Get the list of include paths from the packages
+#
+# @IncludesList list The list path
+#
+@cached_property
+def PackageIncludePathList(self):
+IncludesList = []
+for Package in self.Module.Packages:
+PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
+IncludesList = Package.Includes
+if Package._PrivateIncludes:
+if not self.MetaFile.Path.startswith(PackageDir):
+IncludesList =
list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
+return IncludesList
+
## Get HII EX PCDs which maybe used by VFR
#
# efivarstore used by VFR may relate with HII EX PCDs
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 5802ae5ae4..8dd08201aa 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -906,8 +906,14 @@ cleanlib:
self._AutoGenObject.IncludePathList +
self._AutoGenObject.BuildOptionIncPathList
)
+# Get a set of unique package includes from MetaFile
+parentMetaFileIncludes = set()
+for aInclude in self._AutoGenObject.PackageIncludePathList:
+aIncludeName = str(aInclude)
+parentMetaFileIncludes.add(aIncludeName.lower())
+
# Check if header files are listed in metafile
-# Get a list of unique module header source files from MetaFile
+# Get a set of unique module header source files from MetaFile
headerFilesInMetaFileSet = set()
for aFile in self._AutoGenObject.SourceFileList:
aFileName = str(aFile)
@@ -915,24 +921,27 @@ cleanlib:
continue
headerFilesInMetaFileSet.add(aFileName.lower())
-# Get a list of unique module autogen files
+# Get a set 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
+# Get a set 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()
+aFileDirectory = os.path.dirname(aFileName)
if not aFileName.endswith('.h'):
continue
if aFileName in localAutoGenFileSet:
continue
if localSourceDir not in aFileName:
continue
+if aFileDirectory in parentMetaFileIncludes:
+continue
headerFileDependencySet.add(aFileName)
# Ensure that gModuleBuildTracking has been initialized per
architecture
--
2.22.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44691): https://edk2.groups.io/g/devel/message/44691
Mute This Topic: https://groups.io/mt/32669900/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] BaseTools: Cannot store library cache of different arch together
Hi Steven, This looks good, thank you. Thanks, Christian From: Shi, Steven Sent: Tuesday, June 11, 2019 8:30 PM To: Rodriguez, Christian ; devel@edk2.groups.io Cc: Gao, Liming ; Feng, Bob C Subject: RE: [PATCH] BaseTools: Cannot store library cache of different arch together Sorry, the CopyFileOnChange() will ensure only once IO store/restore writing for each library. The extra IO read is ok. Thanks Steven Shi Intel\SSG\FID\Firmware Infrastructure From: Shi, Steven Sent: Wednesday, June 12, 2019 11:24 AM To: Rodriguez, Christian mailto:christian.rodrig...@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Gao, Liming mailto:liming@intel.com>>; Feng, Bob C mailto:bob.c.f...@intel.com>> Subject: RE: [PATCH] BaseTools: Cannot store library cache of different arch together Hi Christian, For the extra IO accesses for duplicated library, I plan to introduce the CopyFileOnChange() function to solve it. Below is the CopyFileOnChange() BZ, and I haven't sent its patch yet. The CopyFileOnChange() will ensure only once IO store/restore access for each library. To avoid duplicated library access is critical not only for the performance, but also for the writing data race in multi-threads. Basetool need CopyFileOnChange function to avoid cache file writing race in multi-thread build https://bugzilla.tianocore.org/show_bug.cgi?id=1894 Thanks Steven Shi Intel\SSG\FID\Firmware Infrastructure > -Original Message- > From: Rodriguez, Christian > Sent: Tuesday, June 11, 2019 11:41 PM > To: Shi, Steven mailto:steven@intel.com>>; > devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Gao, Liming mailto:liming@intel.com>>; Feng, > Bob C mailto:bob.c.f...@intel.com>> > Subject: RE: [PATCH] BaseTools: Cannot store library cache of different arch > together > > The change looks good overall, but I'm concerned about the performance if > there are multiple of the same library used by different modules. In this case > the library will be copied once per arch per module. I'm not sure if it would > make a giant impact, but just something to think about because you would be > doing extra IO accesses for each duplicate library autogen. > > Possible suggestion: > Is it possible to change the hash to include the arch? Or you can store a > unique tuple pair like (lib.arch, lib) in the set to reduce the libraries > being > copied to a unique subset. > > Thanks, > Christian > > >-Original Message- > >From: Shi, Steven > >Sent: Monday, June 10, 2019 10:53 PM > >To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > >Cc: Gao, Liming mailto:liming@intel.com>>; Feng, > >Bob C > >mailto:bob.c.f...@intel.com>>; Rodriguez, Christian > >mailto:christian.rodrig...@intel.com>> > >Subject: [PATCH] BaseTools: Cannot store library cache of different arch > >together > > > >https://bugzilla.tianocore.org/show_bug.cgi?id=1895 > > > >Build cache cannot store cache for the same library modules in different arch > >together. E.g. Both the below IA32 and X64 arch BaseLib caches should exist > >after build Ovmf3264, but now only the one in X64 arch exist. > >The reason is the current Basetool use a set() to same all library AutoGen > >objects, but the different arch lib AutoGen objects have same __hash_ value > >which comes from the lib MetaFile(The path of module file): > >def __hash__(self): > >return hash(self.MetaFile) > > > >So the different arch lib AutoGen objects are duplicated one to the set() and > >only one can exist. This is why the Basetool can only store one arch cache > >for > >library. > >This patch avoid to use the set() as the container to save the library and > >module objects because the objects might have same __hash__ value. > > > >Cc: Liming Gao mailto:liming@intel.com>> > >Cc: Bob Feng mailto:bob.c.f...@intel.com>> > >Cc: Christian Rodriguez > >mailto:christian.rodrig...@intel.com>> > >Signed-off-by: Steven Shi mailto:steven@intel.com>> > >--- > > BaseTools/Source/Python/build/build.py | 13 +++-- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > >diff --git a/BaseTools/Source/Python/build/build.py > >b/BaseTools/Source/Python/build/build.py > >index 0855d4561c..f7a79cbbab 100644 > >--- a/BaseTools/Source/Python/build/build.py > >+++ b/BaseTools/Source/Python/build/build.py > >@@ -2203,21 +2203,14 @@ class Build(): > > RemoveDirectory(os.path.dirname(GlobalData.gDatabasePath),
Re: [edk2-devel] [Patch V3] OpensslLib: Missing local header files in [Sources] section of .INFs
Hi Xiaoyu, For your comment (1), I don't see a way to rule out headers using configdata. I was already using configdata to get the include directories, but configdata doesn't list the specific header files that will be used for compilation. We can ignore things with a common name or specific files, but ruling out all of them doesn't seem possible using what is given. The rest of the comments can be addressed. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42017): https://edk2.groups.io/g/devel/message/42017 Mute This Topic: https://groups.io/mt/31713211/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Edk2 BaseTools Patches.
See below.
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@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 +, 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 ; 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=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:leif.lindh...@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
Re: [edk2-devel] 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:leif.lindh...@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 +, 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/3183#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 (#41675): https://edk2.groups.io/g/devel/message/41675 Mute This Topic: https://groups.io/mt/31866190/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
Add a check to see if [Sources] section lists all the header 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/GenMake.py | 38
1 file changed, 38 insertions(+)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0e0f9fd9b0..5c992d7c26 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -905,6 +905,44 @@ 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 = 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)
+
+# Check if a module dependency header file is missing from the
module's MetaFile
+for aFile in headerFileDependencySet:
+if aFile in headerFilesInMetaFileSet:
+continue
+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:
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41641): https://edk2.groups.io/g/devel/message/41641
Mute This Topic: https://groups.io/mt/3183/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V4 0/2] BaseTools: Add a checking for Sources section and update hashing feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804 In V4: Update commit messages. In V3: Seperate checker and hashing into individual patches 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 patch since information is already being fetched for Makefile purposes. All other information is already cached in memory. No extra IO time is needed. Christian Rodriguez (2): BaseTools: Add a checking for Sources section in INF file BaseTools: Refactor hash tracking after checking for Sources section 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(-) -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41640): https://edk2.groups.io/g/devel/message/41640 Mute This Topic: https://groups.io/mt/31835554/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources section
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
After adding a check to see if [Sources] section lists all the header
type files of a module, track module and library hashes for --hash
feature. If above check is not in compilance for a library or module,
force hash invalidation on that library or module.
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 | 6 ++
BaseTools/Source/Python/Common/GlobalData.py | 3 +-
BaseTools/Source/Python/build/build.py | 65
4 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index a5bef4f7c6..a376bc24d6 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 5c992d7c26..212ca0fa7f 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -935,10 +935,16 @@ cleanlib:
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 in headerFilesInMetaFileSet:
continue
+if GlobalData.gUseHashCache:
+
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
)
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 ObjValue: '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 80ceb98310..0855d4561c 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.BuildItem.BuildObject]
!= 'FAIL_METAFILE' and
+ not BuildTask._ErrorFlag.isSet()
+ ):
+
GlobalData.gModuleBuildTracking[self.BuildItem.B
Re: [edk2-devel] [Patch V3 2/2] BaseTools: Add a checking for Sources section in INF file [Part 2/2]
Please review. Bump.
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Christian Rodriguez
>Sent: Friday, May 24, 2019 7:40 AM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong
>Subject: [edk2-devel] [Patch V3 2/2] BaseTools: Add a checking for Sources
>section in INF file [Part 2/2]
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>
>In V3: Seperate checker and hashing into individual patches 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 patch since
>information is already being fetched for Makefile purposes. All other
>information is already cached in memory. No extra IO time is needed. Part 2 is
>hashing update.
>
>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 | 6 ++
> BaseTools/Source/Python/Common/GlobalData.py | 3 +-
> BaseTools/Source/Python/build/build.py | 65
> 4 files changed, 53 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) == 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 5c992d7c26..212ca0fa7f 100644
>--- a/BaseTools/Source/Python/AutoGen/GenMake.py
>+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>@@ -935,10 +935,16 @@ cleanlib:
> 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 in headerFilesInMetaFileSet:
> continue
>+if GlobalData.gUseHashCache:
>+
>GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch][self._AutoGen
>Object] = 'FAIL_METAFILE'
> EdkLogger.warn("build","Module MetaFile [Sources] is missing local
>header!",
> ExtraData = "Local Header: " + aFile + " not found in
> " +
>self._AutoGenObject.MetaFile.Path
> )
>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 ObjValue: '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 80ceb98310..0855d4561c 100644
>--- a/BaseTools/Source/Python/build/build.py
>+++ b/BaseTools/Source/Python/build/build.py
>@@ -625,8 +625
Re: [edk2-devel] [Patch V3 1/2] BaseTools: Add a checking for Sources section in INF file [Part 1/2]
Please review. Bump.
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Christian Rodriguez
>Sent: Friday, May 24, 2019 7:40 AM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong
>Subject: [edk2-devel] [Patch V3 1/2] BaseTools: Add a checking for Sources
>section in INF file [Part 1/2]
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>
>In V3: Seperate checker and hashing into individual patches 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 patch since
>information is already being fetched for Makefile purposes. All other
>information is already cached in memory. No extra IO time is needed. Part 1 is
>checker only.
>
>Signed-off-by: Christian Rodriguez
>Cc: Bob Feng
>Cc: Liming Gao
>Cc: Yonghong Zhu
>---
> BaseTools/Source/Python/AutoGen/GenMake.py | 38
>
> 1 file changed, 38 insertions(+)
>
>diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
>b/BaseTools/Source/Python/AutoGen/GenMake.py
>index 0e0f9fd9b0..5c992d7c26 100644
>--- a/BaseTools/Source/Python/AutoGen/GenMake.py
>+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
>@@ -905,6 +905,44 @@ 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 = 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)
>+
>+# Check if a module dependency header file is missing from the
>module's
>MetaFile
>+for aFile in headerFileDependencySet:
>+if aFile in headerFilesInMetaFileSet:
>+continue
>+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:
>--
>2.19.1.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41616): https://edk2.groups.io/g/devel/message/41616
Mute This Topic: https://groups.io/mt/31833976/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support library cache
Hi Steven,
After testing the bug reported, it seems that the dependency problem isn't the
root cause. We don't need to change the way it skips having to rebuild
libraries. The cache is missing the library build artifacts as you mentioned
before and this is the root cause of the rebuild failure. This patch should fix
that, but you can simplify it by not adding a new tracking system and keeping
the dependency reduction as before. That will save the performance because it
doesn't have to do extra checking.
Please see attached patch for minimal change needed.
Thanks,
Christian
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Steven Shi
>Sent: Tuesday, May 28, 2019 1:52 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming ; Feng, Bob C
>; Rodriguez, Christian
>; Fan, ZhijuX
>Subject: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support
>library cache
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=1797
>
>Current binary cache doesn't support to save and restore
>the library module. If a driver module cache miss happen,
>all its dependency library modules need rebuild which
>is very time-consuming. This patch is to entend the binary
>cache to support library.
>
>Cc: Liming Gao
>Cc: Bob Feng
>Cc: Christian Rodriguez
>Signed-off-by: Steven Shi
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py | 17 ++---
> BaseTools/Source/Python/build/build.py | 3 ++-
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index a5bef4f7c6..aeb63f52c5 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -2578,7 +2578,7 @@ class ModuleAutoGen(AutoGen):
> self.AutoGenDepSet = set()
> self.ReferenceModules = []
> self.ConstPcd = {}
>-
>+self.CacheRestored = False
>
> def __repr__(self):
> return "%s [%s]" % (self.MetaFile, self.Arch)
>@@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
> ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
> if os.path.exists(ModuleFile):
> shutil.copy2(ModuleFile, FileDir)
>+else:
>+OutputDir = self.OutputDir.replace('\\', '/').strip('/')
>+DebugDir = self.DebugDir.replace('\\', '/').strip('/')
>+for Item in self.CodaTargetList:
>+File = Item.Target.Path.replace('\\',
>'/').strip('/').replace(DebugDir,
>'').replace(OutputDir, '').strip('/')
>+self.OutputFile.add(File)
> if not self.OutputFile:
> Ma = self.BuildDatabase[self.MetaFile, self.Arch,
> self.BuildTarget,
>self.ToolChain]
> self.OutputFile = Ma.Binaries
>@@ -3949,6 +3955,7 @@ class ModuleAutoGen(AutoGen):
> destination_dir =
> os.path.dirname(destination_file)
> CreateDirectory(destination_dir)
> shutil.copy2(File, destination_dir)
>+self.CacheRestored = True
> if self.Name == "PcdPeim" or self.Name == "PcdDxe":
> CreatePcdDatabaseCode(self, TemplateString(),
>TemplateString())
> return True
>@@ -3987,7 +3994,9 @@ class ModuleAutoGen(AutoGen):
> self.GenFfsList = GenFfsList
> if not self.IsLibrary and CreateLibraryMakeFile:
> for LibraryAutoGen in self.LibraryAutoGenList:
>-LibraryAutoGen.CreateMakeFile()
>+# Only create makefile for libraries which have not been
>restored
>+if not LibraryAutoGen.CacheRestored:
>+LibraryAutoGen.CreateMakeFile()
>
> if self.CanSkip():
> return
>@@ -4030,7 +4039,9 @@ class ModuleAutoGen(AutoGen):
>
> if not self.IsLibrary and CreateLibraryCodeFile:
> for LibraryAutoGen in self.LibraryAutoGenList:
>-LibraryAutoGen.CreateCodeFile()
>+# Only create autogen code for libraries which have not been
>restored
>+if not LibraryAutoGen.CacheRestored:
>+LibraryAutoGen.CreateCodeFile()
>
> if self.CanSkip():
> return
>diff --git a/BaseTools/Source/Python/build/build.py
>b/BaseTools/Source/Python/build/build.py
>index 80ceb98310..f1f4c07980 100644
>--- a/BaseTools/Source/Python/build/build.py
>+++ b/BaseTools/Source/Python/build/build.py
>@@ -341,7 +341,8 @@ class ModuleMakeUnit(BuildUnit):
> # @para
Re: [edk2-devel] [PATCH] BaseTools:Fix the library dependency missing in Binary Cache
Hi Steven, The problem isn't that the library dependency is missing. We are missing library artifacts and therefore cannot build the library. This can be fixed with a snippet of your next patch that adds the libraries artifacts, but without the extra tracking information. Thanks, Christian >-Original Message- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Steven Shi >Sent: Tuesday, May 28, 2019 1:14 AM >To: devel@edk2.groups.io >Cc: Gao, Liming ; Feng, Bob C >; Rodriguez, Christian > >Subject: [edk2-devel] [PATCH] BaseTools:Fix the library dependency missing >in Binary Cache > >BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1788 > >The library dependency of a module is wrongly filtered out by binary cache >implementation which cause all dependent libraries will not been built prior to >the module in the build scheduler and the module build fails if cache miss >happen. > >Cc: Liming Gao >Cc: Bob Feng >Cc: Christian Rodriguez >Signed-off-by: Steven Shi >--- > BaseTools/Source/Python/build/build.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/BaseTools/Source/Python/build/build.py >b/BaseTools/Source/Python/build/build.py >index 80ceb98310..673a9379ba 100644 >--- a/BaseTools/Source/Python/build/build.py >+++ b/BaseTools/Source/Python/build/build.py >@@ -593,7 +593,7 @@ class BuildTask: > # > def AddDependency(self, Dependency): > for Dep in Dependency: >-if not Dep.BuildObject.IsBinaryModule and not >Dep.BuildObject.CanSkipbyHash(): >+if not Dep.BuildObject.IsBinaryModule: > self.DependencyList.append(BuildTask.New(Dep))# BuildTask > list > > ## The thread wrapper of LaunchCommand function >-- >2.17.1.windows.2 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41512): https://edk2.groups.io/g/devel/message/41512 Mute This Topic: https://groups.io/mt/31819545/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V3 1/2] BaseTools: Add a checking for Sources section in INF file [Part 1/2]
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
In V3: Seperate checker and hashing into individual patches
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 patch since information is already
being fetched for Makefile purposes. All other information is already
cached in memory. No extra IO time is needed. Part 1 is checker only.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/GenMake.py | 38
1 file changed, 38 insertions(+)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0e0f9fd9b0..5c992d7c26 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -905,6 +905,44 @@ 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 = 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)
+
+# Check if a module dependency header file is missing from the
module's MetaFile
+for aFile in headerFileDependencySet:
+if aFile in headerFilesInMetaFileSet:
+continue
+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:
--
2.19.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41337): https://edk2.groups.io/g/devel/message/41337
Mute This Topic: https://groups.io/mt/31745161/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V3 2/2] BaseTools: Add a checking for Sources section in INF file [Part 2/2]
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
In V3: Seperate checker and hashing into individual patches
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 patch since information is already
being fetched for Makefile purposes. All other information is already
cached in memory. No extra IO time is needed. Part 2 is hashing update.
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 | 6 ++
BaseTools/Source/Python/Common/GlobalData.py | 3 +-
BaseTools/Source/Python/build/build.py | 65
4 files changed, 53 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) == 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 5c992d7c26..212ca0fa7f 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -935,10 +935,16 @@ cleanlib:
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 in headerFilesInMetaFileSet:
continue
+if GlobalData.gUseHashCache:
+
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
)
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 ObjValue: '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 80ceb98310..0855d4561c 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 i
[edk2-devel] [Patch V3 0/2] BaseTools: Add a checking for Sources section in INF file [Part 0/2]
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804 In V3: Seperate checker and hashing into individual patches Part 1/2: Contains checker. Part 2/2: Contains hash feature alignment. 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 patch since information is already being fetched for Makefile purposes. All other information is already cached in memory. No extra IO time is needed. Christian Rodriguez (2): BaseTools: Add a checking for Sources section in INF file [Part 1/2] BaseTools: Add a checking for Sources section in INF file [Part 2/2] 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(-) -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41336): https://edk2.groups.io/g/devel/message/41336 Mute This Topic: https://groups.io/mt/31745160/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources section in INF file
The performance impact seems to be irrelevant. After a few runs both on master
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=1804
>
>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 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 | 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) == 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..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 = 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.gModu
[edk2-devel] [Patch V2] BaseTools: Add a checking for Sources section in INF file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
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 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 | 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) == 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..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 = 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 in headerFilesInMetaFileSet:
+continue
+if GlobalData.gUseHashCache:
+
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():
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/S
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 aFile in self._AutoGenObject.AutoGenFi
Re: [edk2-devel] [Patch V3] OpensslLib: Missing local header files in [Sources] section of .INFs
Thanks for the feedback. I will move it forward into the next patch.
I also agree with Laszlo's comment on the first patch: " I'd request that we
please hold this patch for now. (Comments in disagreement are welcome of
course.) If we agree, I'd suggest marking TianoCore#1821 dependent on
TianoCore#1089".
Both due to the soft feature freeze and the OpenSSL update.
Thanks,
Christian
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Laszlo Ersek
>Sent: Wednesday, May 22, 2019 2:50 AM
>To: devel@edk2.groups.io; Lu, XiaoyuX ; Rodriguez,
>Christian
>Cc: Wang, Jian J ; Ye, Ting ; Zhu,
>Yonghong
>Subject: Re: [edk2-devel] [Patch V3] OpensslLib: Missing local header files in
>[Sources] section of .INFs
>
>On 05/22/19 11:40, Xiaoyu Lu wrote:
>> Hi Christian,
>>
>> (1) We use OpenSSL configure script disabled some OpenSSL feature.
>>But I saw you include all .h files from OpenSSL(only excluded some).
>>Even some header files we don't need (In openssl/crypto/).
>>Can you rule them out?
>>I found OpenSSl use configdata.pm to store configure result.
>>
>>use configdata qw/%config/;
>>foreach my $enabledcryptomodule (@{config{"%sdirs"}})
>>}
>>
>>Is it possible? If this is difficult, then I have no problem.
>>
>> (2) see blow.
>>
>> (3) I think it's better to separate this patch into two.
>>Patch1. process_files.pl
>>Patch2. OpensslLib[Crypto].conf
>>
>>> -Original Message-
>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>> Christian Rodriguez
>>> Sent: Wednesday, May 22, 2019 5:12 AM
>>> To: devel@edk2.groups.io
>>> Cc: Wang, Jian J ; Ye, Ting
>>> ; Zhu, Yonghong
>>> Subject: [edk2-devel] [Patch V3] OpensslLib: Missing local header
>>> files in [Sources] section of .INFs
>>>
>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1821
>>>
>>> In V2: Remove opensslconf.h because it is a script generated header.
>>> Update OpenSSL autogeneration script for .INFs because some OpenSSL
>>> local headers are missing from [Sources] section of OpensslLib.inf
>>> and OpensslLibCrypto.inf. Update OpensslLib.inf and
>>> OpensslLibCrypto.inf using the updated script. Enforce compilance of
>>> Edk2 INF Spec 3.9, which states, All HII Unicode format files must be
>>> listed in [Sources] section. Not functional issue, just compilance.
>>>
>>> Signed-off-by: Christian Rodriguez
>>> Cc: Jian Wang
>>> Cc: Ting Ye
>>> Cc: Yonghong Zhu
>>> ---
>>> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 173
>>>
>+++
>>>
>+++
>>> +++
>>> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 167
>>>
>+++
>>>
>+++
>>> +
>>> CryptoPkg/Library/OpensslLib/process_files.pl | 50
>>> --
>>> 3 files changed, 388 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl
>>> b/CryptoPkg/Library/OpensslLib/process_files.pl
>>> index f6e1f43641..9cad6d3ebd 100755
>>> --- a/CryptoPkg/Library/OpensslLib/process_files.pl
>>> +++ b/CryptoPkg/Library/OpensslLib/process_files.pl
>>> @@ -115,12 +115,18 @@ BEGIN {
>>> # Retrieve file lists from OpenSSL configdata # use configdata
>>> qw/%unified_info/;
>>> +use Cwd qw(cwd getcwd);
>>> +use Cwd;
>>>
>>> my @cryptofilelist = ();
>>> my @sslfilelist = ();
>>> +my %includedirset = ();
>>> foreach my $product ((@{$unified_info{libraries}},
>>>@{$unified_info{engines}})) {
>>> foreach my $o (@{$unified_info{sources}->{$product}}) {
>>> +foreach my $inc (@{%{$unified_info{includes}}{$o}}) {
>>> +$includedirset{$inc} = 1;
>>> +}
>>> foreach my $s (@{$unified_info{sources}->{$o}}) {
>>> next if ($unified_info{generate}->{$s});
>>> next if $s =~ "crypto/bio/b_print.c"; @@ -133,6 +139,46
>>> @@ foreach my $product ((@{$unified_info
[edk2-devel] [Patch V3] OpensslLib: Missing local header files in [Sources] section of .INFs
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1821 In V2: Remove opensslconf.h because it is a script generated header. Update OpenSSL autogeneration script for .INFs because some OpenSSL local headers are missing from [Sources] section of OpensslLib.inf and OpensslLibCrypto.inf. Update OpensslLib.inf and OpensslLibCrypto.inf using the updated script. Enforce compilance of Edk2 INF Spec 3.9, which states, All HII Unicode format files must be listed in [Sources] section. Not functional issue, just compilance. Signed-off-by: Christian Rodriguez Cc: Jian Wang Cc: Ting Ye Cc: Yonghong Zhu --- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 173 + CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 167 +++ CryptoPkg/Library/OpensslLib/process_files.pl | 50 -- 3 files changed, 388 insertions(+), 2 deletions(-) diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index 530ac5f110..c24289d353 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf @@ -513,6 +513,179 @@ $(OPENSSL_PATH)/ssl/t1_reneg.c $(OPENSSL_PATH)/ssl/t1_trce.c $(OPENSSL_PATH)/ssl/tls_srp.c + buildinf.h + $(OPENSSL_PATH)/include/internal/asn1t.h + $(OPENSSL_PATH)/include/internal/bio.h + $(OPENSSL_PATH)/include/internal/comp.h + $(OPENSSL_PATH)/include/internal/conf.h + $(OPENSSL_PATH)/include/internal/constant_time_locl.h + $(OPENSSL_PATH)/include/internal/dane.h + $(OPENSSL_PATH)/include/internal/dso.h + $(OPENSSL_PATH)/include/internal/err.h + $(OPENSSL_PATH)/include/internal/numbers.h + $(OPENSSL_PATH)/include/internal/o_dir.h + $(OPENSSL_PATH)/include/internal/o_str.h + $(OPENSSL_PATH)/include/internal/sslconf.h + $(OPENSSL_PATH)/include/internal/thread_once.h + $(OPENSSL_PATH)/include/openssl/aes.h + $(OPENSSL_PATH)/include/openssl/asn1.h + $(OPENSSL_PATH)/include/openssl/asn1t.h + $(OPENSSL_PATH)/include/openssl/asn1_mac.h + $(OPENSSL_PATH)/include/openssl/async.h + $(OPENSSL_PATH)/include/openssl/bio.h + $(OPENSSL_PATH)/include/openssl/blowfish.h + $(OPENSSL_PATH)/include/openssl/bn.h + $(OPENSSL_PATH)/include/openssl/buffer.h + $(OPENSSL_PATH)/include/openssl/camellia.h + $(OPENSSL_PATH)/include/openssl/cast.h + $(OPENSSL_PATH)/include/openssl/cmac.h + $(OPENSSL_PATH)/include/openssl/cms.h + $(OPENSSL_PATH)/include/openssl/comp.h + $(OPENSSL_PATH)/include/openssl/conf.h + $(OPENSSL_PATH)/include/openssl/conf_api.h + $(OPENSSL_PATH)/include/openssl/crypto.h + $(OPENSSL_PATH)/include/openssl/ct.h + $(OPENSSL_PATH)/include/openssl/des.h + $(OPENSSL_PATH)/include/openssl/dh.h + $(OPENSSL_PATH)/include/openssl/dsa.h + $(OPENSSL_PATH)/include/openssl/dtls1.h + $(OPENSSL_PATH)/include/openssl/ebcdic.h + $(OPENSSL_PATH)/include/openssl/ec.h + $(OPENSSL_PATH)/include/openssl/ecdh.h + $(OPENSSL_PATH)/include/openssl/ecdsa.h + $(OPENSSL_PATH)/include/openssl/engine.h + $(OPENSSL_PATH)/include/openssl/err.h + $(OPENSSL_PATH)/include/openssl/evp.h + $(OPENSSL_PATH)/include/openssl/e_os2.h + $(OPENSSL_PATH)/include/openssl/hmac.h + $(OPENSSL_PATH)/include/openssl/idea.h + $(OPENSSL_PATH)/include/openssl/kdf.h + $(OPENSSL_PATH)/include/openssl/lhash.h + $(OPENSSL_PATH)/include/openssl/md2.h + $(OPENSSL_PATH)/include/openssl/md4.h + $(OPENSSL_PATH)/include/openssl/md5.h + $(OPENSSL_PATH)/include/openssl/mdc2.h + $(OPENSSL_PATH)/include/openssl/modes.h + $(OPENSSL_PATH)/include/openssl/objects.h + $(OPENSSL_PATH)/include/openssl/obj_mac.h + $(OPENSSL_PATH)/include/openssl/ocsp.h + $(OPENSSL_PATH)/include/openssl/opensslv.h + $(OPENSSL_PATH)/include/openssl/ossl_typ.h + $(OPENSSL_PATH)/include/openssl/pem.h + $(OPENSSL_PATH)/include/openssl/pem2.h + $(OPENSSL_PATH)/include/openssl/pkcs12.h + $(OPENSSL_PATH)/include/openssl/pkcs7.h + $(OPENSSL_PATH)/include/openssl/rand.h + $(OPENSSL_PATH)/include/openssl/rc2.h + $(OPENSSL_PATH)/include/openssl/rc4.h + $(OPENSSL_PATH)/include/openssl/rc5.h + $(OPENSSL_PATH)/include/openssl/ripemd.h + $(OPENSSL_PATH)/include/openssl/rsa.h + $(OPENSSL_PATH)/include/openssl/safestack.h + $(OPENSSL_PATH)/include/openssl/seed.h + $(OPENSSL_PATH)/include/openssl/sha.h + $(OPENSSL_PATH)/include/openssl/srp.h + $(OPENSSL_PATH)/include/openssl/srtp.h + $(OPENSSL_PATH)/include/openssl/ssl.h + $(OPENSSL_PATH)/include/openssl/ssl2.h + $(OPENSSL_PATH)/include/openssl/ssl3.h + $(OPENSSL_PATH)/include/openssl/stack.h + $(OPENSSL_PATH)/include/openssl/symhacks.h + $(OPENSSL_PATH)/include/openssl/tls1.h + $(OPENSSL_PATH)/include/openssl/ts.h
[edk2-devel] [PATCH] OpensslLib: Missing local header files in [Sources] section of .INFs
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1821 Update OpenSSL autogeneration script for .INFs because some OpenSSL local headers are missing from [Sources] section of OpensslLib.inf and OpensslLibCrypto.inf. Update OpensslLib.inf and OpensslLibCrypto.inf using the updated script. Enforce compilance of Edk2 INF Spec 3.9, which states, All HII Unicode format files must be listed in [Sources] section. Not functional issue, just compilance. Signed-off-by: Christian Rodriguez Cc: Jian Wang Cc: Ting Ye Cc: Yonghong Zhu --- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 174 ++ CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 168 CryptoPkg/Library/OpensslLib/process_files.pl | 49 +++-- 3 files changed, 389 insertions(+), 2 deletions(-) diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index 530ac5f110..359c22a09f 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf @@ -513,6 +513,180 @@ $(OPENSSL_PATH)/ssl/t1_reneg.c $(OPENSSL_PATH)/ssl/t1_trce.c $(OPENSSL_PATH)/ssl/tls_srp.c + buildinf.h + $(OPENSSL_PATH)/include/internal/asn1t.h + $(OPENSSL_PATH)/include/internal/bio.h + $(OPENSSL_PATH)/include/internal/comp.h + $(OPENSSL_PATH)/include/internal/conf.h + $(OPENSSL_PATH)/include/internal/constant_time_locl.h + $(OPENSSL_PATH)/include/internal/dane.h + $(OPENSSL_PATH)/include/internal/dso.h + $(OPENSSL_PATH)/include/internal/err.h + $(OPENSSL_PATH)/include/internal/numbers.h + $(OPENSSL_PATH)/include/internal/o_dir.h + $(OPENSSL_PATH)/include/internal/o_str.h + $(OPENSSL_PATH)/include/internal/sslconf.h + $(OPENSSL_PATH)/include/internal/thread_once.h + $(OPENSSL_PATH)/include/openssl/aes.h + $(OPENSSL_PATH)/include/openssl/asn1.h + $(OPENSSL_PATH)/include/openssl/asn1t.h + $(OPENSSL_PATH)/include/openssl/asn1_mac.h + $(OPENSSL_PATH)/include/openssl/async.h + $(OPENSSL_PATH)/include/openssl/bio.h + $(OPENSSL_PATH)/include/openssl/blowfish.h + $(OPENSSL_PATH)/include/openssl/bn.h + $(OPENSSL_PATH)/include/openssl/buffer.h + $(OPENSSL_PATH)/include/openssl/camellia.h + $(OPENSSL_PATH)/include/openssl/cast.h + $(OPENSSL_PATH)/include/openssl/cmac.h + $(OPENSSL_PATH)/include/openssl/cms.h + $(OPENSSL_PATH)/include/openssl/comp.h + $(OPENSSL_PATH)/include/openssl/conf.h + $(OPENSSL_PATH)/include/openssl/conf_api.h + $(OPENSSL_PATH)/include/openssl/crypto.h + $(OPENSSL_PATH)/include/openssl/ct.h + $(OPENSSL_PATH)/include/openssl/des.h + $(OPENSSL_PATH)/include/openssl/dh.h + $(OPENSSL_PATH)/include/openssl/dsa.h + $(OPENSSL_PATH)/include/openssl/dtls1.h + $(OPENSSL_PATH)/include/openssl/ebcdic.h + $(OPENSSL_PATH)/include/openssl/ec.h + $(OPENSSL_PATH)/include/openssl/ecdh.h + $(OPENSSL_PATH)/include/openssl/ecdsa.h + $(OPENSSL_PATH)/include/openssl/engine.h + $(OPENSSL_PATH)/include/openssl/err.h + $(OPENSSL_PATH)/include/openssl/evp.h + $(OPENSSL_PATH)/include/openssl/e_os2.h + $(OPENSSL_PATH)/include/openssl/hmac.h + $(OPENSSL_PATH)/include/openssl/idea.h + $(OPENSSL_PATH)/include/openssl/kdf.h + $(OPENSSL_PATH)/include/openssl/lhash.h + $(OPENSSL_PATH)/include/openssl/md2.h + $(OPENSSL_PATH)/include/openssl/md4.h + $(OPENSSL_PATH)/include/openssl/md5.h + $(OPENSSL_PATH)/include/openssl/mdc2.h + $(OPENSSL_PATH)/include/openssl/modes.h + $(OPENSSL_PATH)/include/openssl/objects.h + $(OPENSSL_PATH)/include/openssl/obj_mac.h + $(OPENSSL_PATH)/include/openssl/ocsp.h + $(OPENSSL_PATH)/include/openssl/opensslconf.h + $(OPENSSL_PATH)/include/openssl/opensslv.h + $(OPENSSL_PATH)/include/openssl/ossl_typ.h + $(OPENSSL_PATH)/include/openssl/pem.h + $(OPENSSL_PATH)/include/openssl/pem2.h + $(OPENSSL_PATH)/include/openssl/pkcs12.h + $(OPENSSL_PATH)/include/openssl/pkcs7.h + $(OPENSSL_PATH)/include/openssl/rand.h + $(OPENSSL_PATH)/include/openssl/rc2.h + $(OPENSSL_PATH)/include/openssl/rc4.h + $(OPENSSL_PATH)/include/openssl/rc5.h + $(OPENSSL_PATH)/include/openssl/ripemd.h + $(OPENSSL_PATH)/include/openssl/rsa.h + $(OPENSSL_PATH)/include/openssl/safestack.h + $(OPENSSL_PATH)/include/openssl/seed.h + $(OPENSSL_PATH)/include/openssl/sha.h + $(OPENSSL_PATH)/include/openssl/srp.h + $(OPENSSL_PATH)/include/openssl/srtp.h + $(OPENSSL_PATH)/include/openssl/ssl.h + $(OPENSSL_PATH)/include/openssl/ssl2.h + $(OPENSSL_PATH)/include/openssl/ssl3.h + $(OPENSSL_PATH)/include/openssl/stack.h + $(OPENSSL_PATH)/include/openssl/symhacks.h + $(OPENSSL_PATH)/include/openssl/tls1.h + $(OPENSSL_PATH)/include/openssl/ts.h + $(OPENSSL_PATH
[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
[edk2-devel] [Patch V3] BaseTools: Library hashing fix and optimization for --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1788
In V3: Must generate hashes before attempting to copy from cache for
hash verifcation
In V2: Build failure caused by passing incorrect boolean parameter to
SaveFileOnChange(). Fixed for patch instances.
Library hashing is now supported by the --hash feature. The --hash
feature implementation assumed that the hashing could be done in
place once per module, but that isn't true for libraries due to the
fact that they are built as dependencies. So on a clean build, we now
generate the .hash after the library dependencies are complete.
Added early escape as optimization, if hash already exists in memory.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 53
-
BaseTools/Source/Python/Common/GlobalData.py | 6 ++
BaseTools/Source/Python/build/build.py | 7 ++-
3 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..81efc4f06c 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3935,6 +3935,7 @@ class ModuleAutoGen(AutoGen):
f = open(HashFile, 'r')
CacheHash = f.read()
f.close()
+self.GenModuleHash()
if GlobalData.gModuleHash[self.Arch][self.Name]:
if CacheHash == GlobalData.gModuleHash[self.Arch][self.Name]:
for root, dir, files in os.walk(FileDir):
@@ -4093,13 +4094,20 @@ class ModuleAutoGen(AutoGen):
return RetVal
def GenModuleHash(self):
+# Initialize a dictionary for each arch type
if self.Arch not in GlobalData.gModuleHash:
GlobalData.gModuleHash[self.Arch] = {}
-if self.Name in GlobalData.gModuleHash[self.Arch] and
GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
+
+# Early exit if module or library has been hashed and is in memory
+if self.Name in GlobalData.gModuleHash[self.Arch]:
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
+
+# Initialze hash object
m = hashlib.md5()
+
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x:
x.PackageName):
@@ -4118,6 +4126,7 @@ class ModuleAutoGen(AutoGen):
Content = f.read()
f.close()
m.update(Content)
+
# Add Module's source files
if self.SourceFileList:
for File in sorted(self.SourceFileList, key=lambda x: str(x)):
@@ -4126,27 +4135,45 @@ class ModuleAutoGen(AutoGen):
f.close()
m.update(Content)
-ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
-if self.Name not in GlobalData.gModuleHash[self.Arch]:
-GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
-if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
-return SaveFileOnChange(ModuleHashFile, m.hexdigest(), False)
+GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
+
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
## Decide whether we can skip the ModuleAutoGen process
def CanSkipbyHash(self):
+# Hashing feature is off
+if not GlobalData.gUseHashCache:
+return False
+
+# Initialize a dictionary for each arch type
+if self.Arch not in GlobalData.gBuildHashSkipTracking:
+GlobalData.gBuildHashSkipTracking[self.Arch] = dict()
+
# If library or Module is binary do not skip by hash
if self.IsBinaryModule:
return False
+
# .inc is contains binary information so do not skip by hash as well
for f_ext in self.SourceFileList:
if '.inc' in str(f_ext):
return False
-if GlobalData.gUseHashCache:
-# If there is a valid hash or function generated a valid hash;
function will return False
-# and the statement below will return True
-return not self.GenModuleHash()
-return False
+
+# Use Cache, if exists and if Module has a copy in cache
+if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+return True
+
+# Early exit for libraries that haven't yet finished building
+HashFile = path.join(self.BuildDir, self.Name + ".hash")
+if self.IsLibrary and not os.path.exists(HashFile):
+return False
+
+# Return a Boolean based on if can skip by hash, either fro
[edk2-devel] [Patch V2] BaseTools: Library hashing fix and optimization for --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1788
In V2: Build failure caused by passing incorrect boolean parameter to
SaveFileOnChange(). Fixed for patch instances.
Library hashing is now supported by the --hash feature. The --hash
feature implementation assumed that the hashing could be done in
place once per module, but that isn't true for libraries due to the
fact that they are built as dependencies. So on a clean build, we now
generate the .hash after the library dependencies are complete.
Added early escape as optimization, if hash already exists in memory.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 52
+++-
BaseTools/Source/Python/Common/GlobalData.py | 6 ++
BaseTools/Source/Python/build/build.py | 7 ++-
3 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..455415da45 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4093,13 +4093,20 @@ class ModuleAutoGen(AutoGen):
return RetVal
def GenModuleHash(self):
+# Initialize a dictionary for each arch type
if self.Arch not in GlobalData.gModuleHash:
GlobalData.gModuleHash[self.Arch] = {}
-if self.Name in GlobalData.gModuleHash[self.Arch] and
GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
+
+# Early exit if module or library has been hashed and is in memory
+if self.Name in GlobalData.gModuleHash[self.Arch]:
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
+
+# Initialze hash object
m = hashlib.md5()
+
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x:
x.PackageName):
@@ -4118,6 +4125,7 @@ class ModuleAutoGen(AutoGen):
Content = f.read()
f.close()
m.update(Content)
+
# Add Module's source files
if self.SourceFileList:
for File in sorted(self.SourceFileList, key=lambda x: str(x)):
@@ -4126,27 +4134,45 @@ class ModuleAutoGen(AutoGen):
f.close()
m.update(Content)
-ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
-if self.Name not in GlobalData.gModuleHash[self.Arch]:
-GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
-if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
-return SaveFileOnChange(ModuleHashFile, m.hexdigest(), False)
+GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
+
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
## Decide whether we can skip the ModuleAutoGen process
def CanSkipbyHash(self):
+# Hashing feature is off
+if not GlobalData.gUseHashCache:
+return False
+
+# Initialize a dictionary for each arch type
+if self.Arch not in GlobalData.gBuildHashSkipTracking:
+GlobalData.gBuildHashSkipTracking[self.Arch] = dict()
+
# If library or Module is binary do not skip by hash
if self.IsBinaryModule:
return False
+
# .inc is contains binary information so do not skip by hash as well
for f_ext in self.SourceFileList:
if '.inc' in str(f_ext):
return False
-if GlobalData.gUseHashCache:
-# If there is a valid hash or function generated a valid hash;
function will return False
-# and the statement below will return True
-return not self.GenModuleHash()
-return False
+
+# Use Cache, if exists and if Module has a copy in cache
+if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+return True
+
+# Early exit for libraries that haven't yet finished building
+HashFile = path.join(self.BuildDir, self.Name + ".hash")
+if self.IsLibrary and not os.path.exists(HashFile):
+return False
+
+# Return a Boolean based on if can skip by hash, either from memory or
from IO.
+if self.Name not in GlobalData.gBuildHashSkipTracking[self.Arch]:
+# If hashes are the same, SaveFileOnChange() will return False.
+GlobalData.gBuildHashSkipTracking[self.Arch][self.Name] = not
SaveFileOnChange(HashFile, self.GenModuleHash(), True)
+return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
+else:
+return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
## Decide
[edk2-devel] [PATCH] BaseTools: Library hashing fix and optimization for --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1788
Library hashing is now supported by the --hash feature. The --hash
feature implementation assumed that the hashing could be done in
place once per module, but that isn't true for libraries due to the
fact that they are built as dependencies. So on a clean build, we now
generate the .hash after the library dependencies are complete.
Added early escape as optimization, if hash already exists in memory.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 52
+++-
BaseTools/Source/Python/Common/GlobalData.py | 6 ++
BaseTools/Source/Python/build/build.py | 7 ++-
3 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..cf92d07be1 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4093,13 +4093,20 @@ class ModuleAutoGen(AutoGen):
return RetVal
def GenModuleHash(self):
+# Initialize a dictionary for each arch type
if self.Arch not in GlobalData.gModuleHash:
GlobalData.gModuleHash[self.Arch] = {}
-if self.Name in GlobalData.gModuleHash[self.Arch] and
GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
+
+# Early exit if module or library has been hashed and is in memory
+if self.Name in GlobalData.gModuleHash[self.Arch]:
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
+
+# Initialze hash object
m = hashlib.md5()
+
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x:
x.PackageName):
@@ -4118,6 +4125,7 @@ class ModuleAutoGen(AutoGen):
Content = f.read()
f.close()
m.update(Content)
+
# Add Module's source files
if self.SourceFileList:
for File in sorted(self.SourceFileList, key=lambda x: str(x)):
@@ -4126,27 +4134,45 @@ class ModuleAutoGen(AutoGen):
f.close()
m.update(Content)
-ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
-if self.Name not in GlobalData.gModuleHash[self.Arch]:
-GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
-if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
-return False
-return SaveFileOnChange(ModuleHashFile, m.hexdigest(), False)
+GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
+
+return GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
## Decide whether we can skip the ModuleAutoGen process
def CanSkipbyHash(self):
+# Hashing feature is off
+if not GlobalData.gUseHashCache:
+return False
+
+# Initialize a dictionary for each arch type
+if self.Arch not in GlobalData.gBuildHashSkipTracking:
+GlobalData.gBuildHashSkipTracking[self.Arch] = dict()
+
# If library or Module is binary do not skip by hash
if self.IsBinaryModule:
return False
+
# .inc is contains binary information so do not skip by hash as well
for f_ext in self.SourceFileList:
if '.inc' in str(f_ext):
return False
-if GlobalData.gUseHashCache:
-# If there is a valid hash or function generated a valid hash;
function will return False
-# and the statement below will return True
-return not self.GenModuleHash()
-return False
+
+# Use Cache, if exists and if Module has a copy in cache
+if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+return True
+
+# Early exit for libraries that haven't yet finished building
+HashFile = path.join(self.BuildDir, self.Name + ".hash")
+if self.IsLibrary and not os.path.exists(HashFile):
+return False
+
+# Return a Boolean based on if can skip by hash, either from memory or
from IO.
+if self.Name not in GlobalData.gBuildHashSkipTracking[self.Arch]:
+# If hashes are the same, SaveFileOnChange() will return False.
+GlobalData.gBuildHashSkipTracking[self.Arch][self.Name] = not
SaveFileOnChange(HashFile, self.GenModuleHash(), False)
+return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
+else:
+return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
## Decide whether we can skip the ModuleAutoGen process
# If any source file is newer than the module than we cannot skip
di
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
Oh sorry about that, I misspoke. I meant to say Edk2 INF spec. Thanks, Christian >-Original Message- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Monday, May 13, 2019 1:20 PM >To: devel@edk2.groups.io; Rodriguez, Christian >; fel...@ami.com >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >On 05/13/19 20:53, Christian Rodriguez wrote: >> I think a warning would be reasonable. >> >> I only mention the spec because it requires all headers to be in the >> sources section of the inf, > >That could be required by the edk2 INF spec, yes. It's totally irrelevant for >the >UEFI spec however. (Originally you wrote, "[...] This would force users of the >hash feature to be UEFI spec complaint [...]".) > >Laszlo > >> but it's not enforced strictly by BaseTools. Though the hashing feature >> relies >on this requirement. It not a big deal, I just wanted to make sure false >positive >build successes were addressed. >> >> Thanks, >> Christian >> >>> -Original Message- >>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>> Sent: Monday, May 13, 2019 4:39 AM >>> To: Rodriguez, Christian ; >>> devel@edk2.groups.io; fel...@ami.com >>> Cc: Feng, Bob C ; Gao, Liming >>> ; Zhu, Yonghong >>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>> mentioned in inf are not hashed >>> >>> On 05/10/19 21:45, Rodriguez, Christian wrote: >>>> Hashing is not changing file format requirements as Basetools has no >>> requirement on this even though the spec does have file requirements. >>> That's why the initial patch was a workaround of sorts because it is >>> allowed by Basetools to have local headers not in the sources section of >the meta file. >>>> >>>> Always breaking the build is outside of the scope of this BZ and my >>>> project >>> priorities. I agree it should be done, but it's out of my scope. >>>> >>>> I am specifically targeting the hashing feature, which relies on >>>> UEFI Spec >>> requirements. >>> >>> I think breaking the build immediately (and unconditionally) could >>> catch platforms by surprise. Can we make this a warning vs. an error? >>> And, I'm totally OK if it's available only with --hash, for now. >>> >>> BTW -- I'm not sure why the UEFI spec is relevant here. >>> >>> Thanks >>> Laszlo >>> >>>>> -Original Message- >>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>>> Of Felix Polyudov >>>>> Sent: Friday, May 10, 2019 12:32 PM >>>>> To: Rodriguez, Christian ; >>>>> devel@edk2.groups.io; 'ler...@redhat.com' >>>>> Cc: Feng, Bob C ; Gao, Liming >>>>> ; Zhu, Yonghong >>>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>> mentioned in inf are not hashed >>>>> >>>>> My suggestion would be to always break a build (no matter what the >>>>> hashing settings are). >>>>> Hashing is just an optimization technique, usage of which should >>>>> not be changing source file formatting requirements. >>>>> >>>>>> -Original Message- >>>>>> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >>>>>> Sent: Friday, May 10, 2019 3:14 PM >>>>>> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >>>>>> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >>>>>> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>>> mentioned in inf are not hashed >>>>>> >>>>>> After talking to my colleagues about this, the direction seems to >>>>>> be to fundamentally change this BZ. Instead of building this sort >>>>>> of workaround feature, we should use the information gathered from >>>>>> this >>>>> feature to cause the build to break when the hash feature is enabled. >>>>> This would force users of the hash feature to be UEFI spec complaint. >>>>>> >>>>>> What do you guys think; Laszlo and Felix? >>>>>> >>>>>> I'll update the BZ when I get your input. >>>>>> >>>>>> Thanks, >>
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
So the direction put forward by my team is that we should raise an error instead of a warning because a false positive successful build would be detrimental to a continuous integration environment. And it would also enforce the Spec requirements. The point they are making is that a warning does not emphasizes that there is a false successful build as much as breaking the build. And for now it will be at a limited scope only with the hashing feature enabled so it won't catch people doing a normal build by surprise. Thanks, Christian >-Original Message- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Christian Rodriguez >Sent: Monday, May 13, 2019 11:54 AM >To: Laszlo Ersek ; devel@edk2.groups.io; >fel...@ami.com >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >I think a warning would be reasonable. > >I only mention the spec because it requires all headers to be in the sources >section of the inf, but it's not enforced strictly by BaseTools. Though the >hashing feature relies on this requirement. It not a big deal, I just wanted to >make sure false positive build successes were addressed. > >Thanks, >Christian > >>-Original Message- >>From: Laszlo Ersek [mailto:ler...@redhat.com] >>Sent: Monday, May 13, 2019 4:39 AM >>To: Rodriguez, Christian ; >>devel@edk2.groups.io; fel...@ami.com >>Cc: Feng, Bob C ; Gao, Liming >>; Zhu, Yonghong >>Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>mentioned in inf are not hashed >> >>On 05/10/19 21:45, Rodriguez, Christian wrote: >>> Hashing is not changing file format requirements as Basetools has no >>requirement on this even though the spec does have file requirements. >>That's why the initial patch was a workaround of sorts because it is >>allowed by Basetools to have local headers not in the sources section of the >meta file. >>> >>> Always breaking the build is outside of the scope of this BZ and my >>> project >>priorities. I agree it should be done, but it's out of my scope. >>> >>> I am specifically targeting the hashing feature, which relies on UEFI >>> Spec >>requirements. >> >>I think breaking the build immediately (and unconditionally) could >>catch platforms by surprise. Can we make this a warning vs. an error? >>And, I'm totally OK if it's available only with --hash, for now. >> >>BTW -- I'm not sure why the UEFI spec is relevant here. >> >>Thanks >>Laszlo >> >>>> -Original Message- >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>> Of Felix Polyudov >>>> Sent: Friday, May 10, 2019 12:32 PM >>>> To: Rodriguez, Christian ; >>>> devel@edk2.groups.io; 'ler...@redhat.com' >>>> Cc: Feng, Bob C ; Gao, Liming >>>> ; Zhu, Yonghong >>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>> mentioned in inf are not hashed >>>> >>>> My suggestion would be to always break a build (no matter what the >>>> hashing settings are). >>>> Hashing is just an optimization technique, usage of which should not >>>> be changing source file formatting requirements. >>>> >>>>> -Original Message- >>>>> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >>>>> Sent: Friday, May 10, 2019 3:14 PM >>>>> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >>>>> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >>>>> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>> mentioned in inf are not hashed >>>>> >>>>> After talking to my colleagues about this, the direction seems to >>>>> be to fundamentally change this BZ. Instead of building this sort >>>>> of workaround feature, we should use the information gathered from >>>>> this >>>> feature to cause the build to break when the hash feature is enabled. >>>> This would force users of the hash feature to be UEFI spec complaint. >>>>> >>>>> What do you guys think; Laszlo and Felix? >>>>> >>>>> I'll update the BZ when I get your input. >>>>> >>>>> Thanks, >>>>> Christian Rodriguez >>>>> >>>>>> -Original Message- >>>>>> From: devel@edk2.groups.io [m
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
I think a warning would be reasonable. I only mention the spec because it requires all headers to be in the sources section of the inf, but it's not enforced strictly by BaseTools. Though the hashing feature relies on this requirement. It not a big deal, I just wanted to make sure false positive build successes were addressed. Thanks, Christian >-Original Message- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Monday, May 13, 2019 4:39 AM >To: Rodriguez, Christian ; >devel@edk2.groups.io; fel...@ami.com >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >On 05/10/19 21:45, Rodriguez, Christian wrote: >> Hashing is not changing file format requirements as Basetools has no >requirement on this even though the spec does have file requirements. That's >why the initial patch was a workaround of sorts because it is allowed by >Basetools to have local headers not in the sources section of the meta file. >> >> Always breaking the build is outside of the scope of this BZ and my project >priorities. I agree it should be done, but it's out of my scope. >> >> I am specifically targeting the hashing feature, which relies on UEFI Spec >requirements. > >I think breaking the build immediately (and unconditionally) could catch >platforms by surprise. Can we make this a warning vs. an error? And, I'm >totally OK if it's available only with --hash, for now. > >BTW -- I'm not sure why the UEFI spec is relevant here. > >Thanks >Laszlo > >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >>> Felix Polyudov >>> Sent: Friday, May 10, 2019 12:32 PM >>> To: Rodriguez, Christian ; >>> devel@edk2.groups.io; 'ler...@redhat.com' >>> Cc: Feng, Bob C ; Gao, Liming >>> ; Zhu, Yonghong >>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>> mentioned in inf are not hashed >>> >>> My suggestion would be to always break a build (no matter what the >>> hashing settings are). >>> Hashing is just an optimization technique, usage of which should not >>> be changing source file formatting requirements. >>> >>>> -Original Message- >>>> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >>>> Sent: Friday, May 10, 2019 3:14 PM >>>> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >>>> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >>>> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >>>> mentioned in inf are not hashed >>>> >>>> After talking to my colleagues about this, the direction seems to be >>>> to fundamentally change this BZ. Instead of building this sort of >>>> workaround feature, we should use the information gathered from this >>> feature to cause the build to break when the hash feature is enabled. >>> This would force users of the hash feature to be UEFI spec complaint. >>>> >>>> What do you guys think; Laszlo and Felix? >>>> >>>> I'll update the BZ when I get your input. >>>> >>>> Thanks, >>>> Christian Rodriguez >>>> >>>>> -Original Message- >>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>>> Of Felix Polyudov >>>>> Sent: Friday, May 10, 2019 6:41 AM >>>>> To: devel@edk2.groups.io; 'ler...@redhat.com' ; >>>>> Rodriguez, Christian >>>>> Cc: Feng, Bob C ; Gao, Liming >>>>> ; Zhu, Yonghong >>>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>> mentioned in inf are not hashed >>>>> >>>>>> -Original Message- >>>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>>>> Of Laszlo Ersek >>>>>> Sent: Thursday, May 09, 2019 7:53 PM >>>>>> >>>>>> Hello Christian, >>>>>> >>>>>> On 05/09/19 23:27, Christian Rodriguez wrote: >>>>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787 >>>>>>> >>>>>>> Get a list of local header files that are not present in the >>>>>>> MetaFile for this module. Add those local header files into the >>>>>>> hashing algorithm for a module. If a local header file is not >>>>>>
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
Yes there could be a performance hit. It might be better to make it a script. But this is only going to happen when the hash feature is enabled, not ever build. Also we can minimize the performance hit by saving the data from ModuleMakefile.GetFileDependency() to memory and reuse it later in GenMake to save on IO and search. Thanks, Christian >-Original Message- >From: Feng, Bob C >Sent: Monday, May 13, 2019 5:23 AM >To: Laszlo Ersek ; Rodriguez, Christian >; devel@edk2.groups.io; fel...@ami.com >Cc: Gao, Liming ; Zhu, Yonghong > >Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >I think checking if [Source] includes all the "source" file under module's >folder >for each module during build would make build slow down, we have been >doing a lot to improve the build performance. >Maybe we could create a new python script under BaseTools/Scripts folder to >do this check. > >Thanks, >Bob > >-Original Message- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Monday, May 13, 2019 7:39 PM >To: Rodriguez, Christian ; >devel@edk2.groups.io; fel...@ami.com >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >On 05/10/19 21:45, Rodriguez, Christian wrote: >> Hashing is not changing file format requirements as Basetools has no >requirement on this even though the spec does have file requirements. That's >why the initial patch was a workaround of sorts because it is allowed by >Basetools to have local headers not in the sources section of the meta file. >> >> Always breaking the build is outside of the scope of this BZ and my project >priorities. I agree it should be done, but it's out of my scope. >> >> I am specifically targeting the hashing feature, which relies on UEFI Spec >requirements. > >I think breaking the build immediately (and unconditionally) could catch >platforms by surprise. Can we make this a warning vs. an error? And, I'm >totally OK if it's available only with --hash, for now. > >BTW -- I'm not sure why the UEFI spec is relevant here. > >Thanks >Laszlo > >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >>> Felix Polyudov >>> Sent: Friday, May 10, 2019 12:32 PM >>> To: Rodriguez, Christian ; >>> devel@edk2.groups.io; 'ler...@redhat.com' >>> Cc: Feng, Bob C ; Gao, Liming >>> ; Zhu, Yonghong >>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>> mentioned in inf are not hashed >>> >>> My suggestion would be to always break a build (no matter what the >>> hashing settings are). >>> Hashing is just an optimization technique, usage of which should not >>> be changing source file formatting requirements. >>> >>>> -Original Message- >>>> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >>>> Sent: Friday, May 10, 2019 3:14 PM >>>> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >>>> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >>>> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >>>> mentioned in inf are not hashed >>>> >>>> After talking to my colleagues about this, the direction seems to be >>>> to fundamentally change this BZ. Instead of building this sort of >>>> workaround feature, we should use the information gathered from this >>> feature to cause the build to break when the hash feature is enabled. >>> This would force users of the hash feature to be UEFI spec complaint. >>>> >>>> What do you guys think; Laszlo and Felix? >>>> >>>> I'll update the BZ when I get your input. >>>> >>>> Thanks, >>>> Christian Rodriguez >>>> >>>>> -Original Message- >>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>>> Of Felix Polyudov >>>>> Sent: Friday, May 10, 2019 6:41 AM >>>>> To: devel@edk2.groups.io; 'ler...@redhat.com' ; >>>>> Rodriguez, Christian >>>>> Cc: Feng, Bob C ; Gao, Liming >>>>> ; Zhu, Yonghong >>>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>> mentioned in inf are not hashed >>>>> >>>>>> -Original Message- >>>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
Hashing is not changing file format requirements as Basetools has no requirement on this even though the spec does have file requirements. That's why the initial patch was a workaround of sorts because it is allowed by Basetools to have local headers not in the sources section of the meta file. Always breaking the build is outside of the scope of this BZ and my project priorities. I agree it should be done, but it's out of my scope. I am specifically targeting the hashing feature, which relies on UEFI Spec requirements. >-Original Message- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Felix Polyudov >Sent: Friday, May 10, 2019 12:32 PM >To: Rodriguez, Christian ; >devel@edk2.groups.io; 'ler...@redhat.com' >Cc: Feng, Bob C ; Gao, Liming >; Zhu, Yonghong >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >My suggestion would be to always break a build (no matter what the hashing >settings are). >Hashing is just an optimization technique, usage of which should not be >changing source file formatting requirements. > >> -Original Message- >> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >> Sent: Friday, May 10, 2019 3:14 PM >> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >> mentioned in inf are not hashed >> >> After talking to my colleagues about this, the direction seems to be >> to fundamentally change this BZ. Instead of building this sort of >> workaround feature, we should use the information gathered from this >feature to cause the build to break when the hash feature is enabled. This >would force users of the hash feature to be UEFI spec complaint. >> >> What do you guys think; Laszlo and Felix? >> >> I'll update the BZ when I get your input. >> >> Thanks, >> Christian Rodriguez >> >> >-Original Message- >> >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> >Felix Polyudov >> >Sent: Friday, May 10, 2019 6:41 AM >> >To: devel@edk2.groups.io; 'ler...@redhat.com' ; >> >Rodriguez, Christian >> >Cc: Feng, Bob C ; Gao, Liming >> >; Zhu, Yonghong >> >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >> >mentioned in inf are not hashed >> > >> >> -Original Message- >> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >> >> Of Laszlo Ersek >> >> Sent: Thursday, May 09, 2019 7:53 PM >> >> >> >> Hello Christian, >> >> >> >> On 05/09/19 23:27, Christian Rodriguez wrote: >> >> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787 >> >> > >> >> > Get a list of local header files that are not present in the >> >> > MetaFile for this module. Add those local header files into the >> >> > hashing algorithm for a module. If a local header file is not >> >> > present in the MetaFile, the module will still build correctly >> >> > though the hashing system didn't know about it before. >> >> > >> >> > Signed-off-by: Christian Rodriguez >> >> > >> >> > Cc: Bob Feng >> >> > Cc: Liming Gao >> >> > Cc: Yonghong Zhu >> >> > --- >> >> > BaseTools/Source/Python/AutoGen/AutoGen.py | 24 >> >> > >> >> > 1 file changed, 24 insertions(+) >> >> >> >> I saw the BZ soon after it was reported. I almost commented, but >> >> ultimately I couldn't decide what the real use case was. >> >> >> >> With this particular use case (i.e. INF file is missing some >> >> module-specific header files that it could easily list), I think I >> >> disagree, mildly (not too strongly). E.g., we fixed such omissions >> >> in a bunch of INF files, last March, in the series >> > >> >I agree with Lazlo. >> >According to section 3.9 of the INF specification (https://edk2- >> >docs.gitbooks.io/edk-ii-inf- >> >specification/3_edk_ii_inf_file_format/39_[sources]_sections.html), >> >all source files (including module header files) must be listed in >> >the [Sources] section. >> >Here is the quote: >> >"All HII Unicode format files must be listed in this section as well >> >as any other "source" type file,
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
After talking to my colleagues about this, the direction seems to be to
fundamentally change this BZ. Instead of building this sort of workaround
feature, we should use the information gathered from this feature to cause the
build to break when the hash feature is enabled. This would force users of the
hash feature to be UEFI spec complaint.
What do you guys think; Laszlo and Felix?
I'll update the BZ when I get your input.
Thanks,
Christian Rodriguez
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Felix Polyudov
>Sent: Friday, May 10, 2019 6:41 AM
>To: devel@edk2.groups.io; 'ler...@redhat.com' ;
>Rodriguez, Christian
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong
>Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
>in inf are not hashed
>
>> -Original Message-
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Thursday, May 09, 2019 7:53 PM
>>
>> Hello Christian,
>>
>> On 05/09/19 23:27, Christian Rodriguez wrote:
>> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787
>> >
>> > Get a list of local header files that are not present in the
>> > MetaFile for this module. Add those local header files into the
>> > hashing algorithm for a module. If a local header file is not
>> > present in the MetaFile, the module will still build correctly
>> > though the hashing system didn't know about it before.
>> >
>> > Signed-off-by: Christian Rodriguez
>> > Cc: Bob Feng
>> > Cc: Liming Gao
>> > Cc: Yonghong Zhu
>> > ---
>> > BaseTools/Source/Python/AutoGen/AutoGen.py | 24
>> >
>> > 1 file changed, 24 insertions(+)
>>
>> I saw the BZ soon after it was reported. I almost commented, but
>> ultimately I couldn't decide what the real use case was.
>>
>> With this particular use case (i.e. INF file is missing some
>> module-specific header files that it could easily list), I think I
>> disagree, mildly (not too strongly). E.g., we fixed such omissions in
>> a bunch of INF files, last March, in the series
>
>I agree with Lazlo.
>According to section 3.9 of the INF specification (https://edk2-
>docs.gitbooks.io/edk-ii-inf-
>specification/3_edk_ii_inf_file_format/39_[sources]_sections.html),
>all source files (including module header files) must be listed in the
>[Sources]
>section.
>Here is the quote:
>"All HII Unicode format files must be listed in this section as well as any
>other
>"source" type file, such as local module header files, Vfr files, etc. "
>
>So, if file X is used by module Y, but is not listed in Y.inf, it's a
>violation of the
>INF spec., which makes it a bug that has to be fixed.
>
>
>Please consider the environment before printing this email.
>
>The information contained in this message may be confidential and
>proprietary to American Megatrends, Inc. This communication is intended to
>be read only by the individual or entity to whom it is addressed or by their
>designee. If the reader of this message is not the intended recipient, you are
>on notice that any distribution of this message, in any form, is strictly
>prohibited. Please promptly notify the sender by reply e-mail or by telephone
>at 770-246-8600, and then delete or destroy all copies of the transmission.
> l K q y e ,j a + U ?E e w Ӎ i vM *? ^ ,j N6
>˭y8b :) m ?
>躛"}y M5 { j躓z'z h+ l ' r zm y 6 . Ȩ z칷! +-
>糊{^ &
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40455): https://edk2.groups.io/g/devel/message/40455
Mute This Topic: https://groups.io/mt/31570019/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
Replies inline.
>-Original Message-
>From: Carsey, Jaben
>Sent: Thursday, May 9, 2019 4:39 PM
>To: devel@edk2.groups.io; Rodriguez, Christian
>
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong
>Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
>in inf are not hashed
>
>Some questions inline.
>
>> -Original Message-
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Christian Rodriguez
>> Sent: Thursday, May 09, 2019 2:27 PM
>> To: devel@edk2.groups.io
>> Cc: Feng, Bob C ; Gao, Liming
>> ; Zhu, Yonghong
>> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
>> in inf are not hashed
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787
>>
>> Get a list of local header files that are not present in the MetaFile
>> for this module. Add those local header files into the hashing
>> algorithm for a module. If a local header file is not present in the
>> MetaFile, the module will still build correctly though the hashing
>> system didn't know about it before.
>>
>> Signed-off-by: Christian Rodriguez
>> Cc: Bob Feng
>> Cc: Liming Gao
>> Cc: Yonghong Zhu
>> ---
>> BaseTools/Source/Python/AutoGen/AutoGen.py | 24
>>
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>> b/BaseTools/Source/Python/AutoGen/AutoGen.py
>> index 31721a6f9f..bd282d3ec1 100644
>> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
>> if self.Name in GlobalData.gModuleHash[self.Arch] and
>> GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
>> return False
>> m = hashlib.md5()
>> +
>> # Add Platform level hash
>> m.update(GlobalData.gPlatformHash.encode('utf-8'))
>> +
>> # Add Package level hash
>> if self.DependentPackageList:
>> for Pkg in sorted(self.DependentPackageList, key=lambda x:
>> x.PackageName):
>> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
>> Content = f.read()
>> f.close()
>> m.update(Content)
>> +
>> # Add Module's source files
>> +localSourceFileList = set()
>> if self.SourceFileList:
>> for File in sorted(self.SourceFileList, key=lambda x: str(x)):
>> +localSourceFileList.add(str(File))
>> f = open(str(File), 'rb')
>> Content = f.read()
>> f.close()
>> m.update(Content)
>>
>> +# Get a list of Module's local header files not included in metaFile
>> +localHeaderList = set()
>> +if self.SourceDir:
>> +for root, dirs, files in os.walk(self.SourceDir):
>> +for aFile in files:
>> +filePath = os.path.join(self.WorkspaceDir,
>> + os.path.join(root,
>> aFile))
>> +if not filePath.endswith(('.h', '.H')):
>> +continue
>> +if filePath not in localSourceFileList:
>
>Confused about localSourceFileList.
>Why is it a set and named list?
>Why not just use self.SourceFileList?
>
The naming convention could be better and I can address that in a different
patch, if we decide to go forward with this idea overall.
It should probably be named a set.
The reason to using this new set is for a few reasons:
1. self.SourceFileList contains source file paths of class PathClass and not
type str
2. If we want to use self.SourceFileList you must convert PathClass to a str
for string comparison
The set just allows for a unique list of objects and theoretically faster to
check existence.
>> +localHeaderList.add(filePath)
>> +
>> +# Add Module's local header files
>> +localHeaderList = list(localHeaderList)
>> +for File in sorted(localHeaderList):
>> +f = open(str(File), 'rb')
>> +Content = f.read()
>
>Can you use 'with open(...) as...:' syntax to make sure the file is always
>closed?
I just used the same implementation as the above existing code. I can
definitely change it to use 'with open(...)'.
Though explicitly calling f.close() as below should be making sure the file is
always closed.
>
>> +f.close()
>> +m.update(Content)
>> +
>> ModuleHash
[edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787
Get a list of local header files that are not present in the
MetaFile for this module. Add those local header files into
the hashing algorithm for a module. If a local header file is
not present in the MetaFile, the module will still build correctly
though the hashing system didn't know about it before.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 24
1 file changed, 24 insertions(+)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..bd282d3ec1 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
if self.Name in GlobalData.gModuleHash[self.Arch] and
GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
return False
m = hashlib.md5()
+
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x:
x.PackageName):
@@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
Content = f.read()
f.close()
m.update(Content)
+
# Add Module's source files
+localSourceFileList = set()
if self.SourceFileList:
for File in sorted(self.SourceFileList, key=lambda x: str(x)):
+localSourceFileList.add(str(File))
f = open(str(File), 'rb')
Content = f.read()
f.close()
m.update(Content)
+# Get a list of Module's local header files not included in metaFile
+localHeaderList = set()
+if self.SourceDir:
+for root, dirs, files in os.walk(self.SourceDir):
+for aFile in files:
+filePath = os.path.join(self.WorkspaceDir,
os.path.join(root, aFile))
+if not filePath.endswith(('.h', '.H')):
+continue
+if filePath not in localSourceFileList:
+localHeaderList.add(filePath)
+
+# Add Module's local header files
+localHeaderList = list(localHeaderList)
+for File in sorted(localHeaderList):
+f = open(str(File), 'rb')
+Content = f.read()
+f.close()
+m.update(Content)
+
ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
if self.Name not in GlobalData.gModuleHash[self.Arch]:
GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
--
2.19.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40377): https://edk2.groups.io/g/devel/message/40377
Mute This Topic: https://groups.io/mt/31570019/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] BaseTools: Hash false success.. minor change in hash invalidation
Please don't forget to review this patch. It corrects a small error in my last patch. We should only invalidate hashes in the gBinCacheDest not gBinCacheSource because we want to error handle the hashes being written. -Original Message- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Christian Rodriguez Sent: Tuesday, April 16, 2019 12:41 PM To: devel@edk2.groups.io Cc: Feng, Bob C ; Gao, Liming ; Zhu, Yonghong Subject: [edk2-devel] [PATCH] BaseTools: Hash false success.. minor change in hash invalidation BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1692 Change part of the hash error handling to invalidate hashes in the cache destination not the cache source. Signed-off-by: Christian Rodriguez Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu --- BaseTools/Source/Python/build/build.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index 71478b7268..7271570d29 100644 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -1165,8 +1165,8 @@ class Build(): os.remove(ModuleHashFile) # Remove .hash file from cache -if GlobalData.gBinCacheSource: -FileDir = path.join(GlobalData.gBinCacheSource, moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, moduleAutoGenObj.MetaFile.BaseName) +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) -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39302): https://edk2.groups.io/g/devel/message/39302 Mute This Topic: https://groups.io/mt/31204490/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V2] BaseTools: Enhance Bin Cache database to support save the cache
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1726 V2: Added the platform name to the path Add more level sub-directories in the database to support save the cache for multiple platforms with multiple tool-chains and targets, just like edk2 Build output. Signed-off-by: Christian Rodriguez Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu --- BaseTools/Source/Python/AutoGen/AutoGen.py | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 8648fc40ed..31721a6f9f 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -3897,18 +3897,17 @@ class ModuleAutoGen(AutoGen): self.CopyModuleToCache() def CopyModuleToCache(self): -FileDir = path.join(GlobalData.gBinCacheDest, self.Arch, self.SourceDir, self.MetaFile.BaseName) +FileDir = path.join(GlobalData.gBinCacheDest, self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch, self.SourceDir, self.MetaFile.BaseName) CreateDirectory (FileDir) HashFile = path.join(self.BuildDir, self.Name + '.hash') if os.path.exists(HashFile): shutil.copy2(HashFile, FileDir) -if self.IsLibrary: -return -ModuleFile = path.join(self.OutputDir, self.Name + '.inf') -if os.path.exists(ModuleFile): -shutil.copy2(ModuleFile, FileDir) +if not self.IsLibrary: +ModuleFile = path.join(self.OutputDir, self.Name + '.inf') +if os.path.exists(ModuleFile): +shutil.copy2(ModuleFile, FileDir) if not self.OutputFile: -Ma = self.BuildDatabase[PathClass(ModuleFile), self.Arch, self.BuildTarget, self.ToolChain] +Ma = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain] self.OutputFile = Ma.Binaries if self.OutputFile: for File in self.OutputFile: @@ -3930,7 +3929,7 @@ class ModuleAutoGen(AutoGen): for f_ext in self.SourceFileList: if '.inc' in str(f_ext): return False -FileDir = path.join(GlobalData.gBinCacheSource, self.Arch, self.SourceDir, self.MetaFile.BaseName) +FileDir = path.join(GlobalData.gBinCacheSource, self.PlatformInfo.Name, self.BuildTarget + "_" + self.ToolChain, self.Arch, self.SourceDir, self.MetaFile.BaseName) HashFile = path.join(FileDir, self.Name + '.hash') if os.path.exists(HashFile): f = open(HashFile, 'r') -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39300): https://edk2.groups.io/g/devel/message/39300 Mute This Topic: https://groups.io/mt/31223880/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] BaseTools: Hash false success.. minor change in hash invalidation
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1692 Change part of the hash error handling to invalidate hashes in the cache destination not the cache source. Signed-off-by: Christian Rodriguez Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu --- BaseTools/Source/Python/build/build.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index 71478b7268..7271570d29 100644 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -1165,8 +1165,8 @@ class Build(): os.remove(ModuleHashFile) # Remove .hash file from cache -if GlobalData.gBinCacheSource: -FileDir = path.join(GlobalData.gBinCacheSource, moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, moduleAutoGenObj.MetaFile.BaseName) +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) -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39189): https://edk2.groups.io/g/devel/message/39189 Mute This Topic: https://groups.io/mt/31204490/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] BaseTools: Hash false success with back to back builds
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1692
Add error handling to the --hash feature so that hash files
are invalidated when a build error occurs.
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/Common/GlobalData.py | 4
BaseTools/Source/Python/build/build.py | 45
+
2 files changed, 49 insertions(+)
diff --git a/BaseTools/Source/Python/Common/GlobalData.py
b/BaseTools/Source/Python/Common/GlobalData.py
index 1853f1d2f6..79f23c892d 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -108,3 +108,7 @@ gPackageHash = {}
gModuleHash = {}
gEnableGenfdsMultiThread = False
gSikpAutoGenCache = set()
+
+# Dictionary for tracking Module build status as success or failure
+# False -> Fail : True -> Success
+gModuleBuildTracking = dict()
diff --git a/BaseTools/Source/Python/build/build.py
b/BaseTools/Source/Python/build/build.py
index af8bba47b1..71478b7268 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -620,6 +620,8 @@ 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
# indicate there's a thread is available for another build task
BuildTask._RunningQueueLock.acquire()
BuildTask._RunningQueue.pop(self.BuildItem)
@@ -1138,6 +1140,37 @@ class Build():
if Process.returncode != 0 :
EdkLogger.error("Postbuild", POSTBUILD_ERROR, 'Postbuild
process is not success!')
EdkLogger.info("\n- Postbuild Done -\n")
+
+## Error handling for hash feature
+#
+# On BuildTask error, iterate through the Module Build tracking
+# dictionary to determine wheather a module failed to build. Invalidate
+# the hash associated with that module by removing it from storage.
+#
+#
+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
+
+# The module failed to build or failed to start building, 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 file from cache
+if GlobalData.gBinCacheSource:
+FileDir = path.join(GlobalData.gBinCacheSource,
moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir,
moduleAutoGenObj.MetaFile.BaseName)
+HashFile = path.join(FileDir, moduleAutoGenObj.Name + '.hash')
+if os.path.exists(HashFile):
+os.remove(HashFile)
+
## Build a module or platform
#
# Create autogen code and makefile for a module or platform, and the launch
@@ -1795,6 +1828,9 @@ 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
self.AutoGenTime += int(round((time.time() -
AutoGenStart)))
MakeStart = time.time()
for Ma in self.BuildModules:
@@ -1805,6 +1841,7 @@ class Build():
# we need a full version of makefile for platform
ExitFlag.set()
BuildTask.WaitForComplete()
+self.invalidateHash()
Pa.CreateMakeFile(False)
EdkLogger.error("build", BUILD_ERROR, "Failed to
build module", ExtraData=GlobalData.gBuildingModule)
# Start task scheduler
@@ -1814,6 +1851,7 @@ class Build():
# in case there's an interruption. we need a full version
of makefile for platform
[edk2-devel] [Patch V2 0/4] BaseTools: Fix corner-cases of --hash feature
Patch 1: Consider modules with .inc source files as Binary Modules and do not Skip by hash. Patch 2: Re-order hashing operations so we don't do redundant hashes. Patch 3: Respect artifact location within directory structure. Patch 4: Re-use libraries, since they have already been hashed. Christian Rodriguez (4): BaseTools: Fix corner-cases of --hash feature BaseTools: Fix corner-cases of --hash feature BaseTools: Fix corner-cases of --hash feature BaseTools: Fix corner-cases of --hash feature BaseTools/Source/Python/AutoGen/AutoGen.py | 47 +- BaseTools/Source/Python/AutoGen/GenMake.py | 6 +-- BaseTools/Source/Python/build/build.py | 14 ++- 3 files changed, 52 insertions(+), 15 deletions(-) -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72): https://edk2.groups.io/g/devel/message/72 Mute This Topic: https://groups.io/mt/30897932/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V2 2/4] BaseTools: Fix corner-cases of --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1680
Re-order hashing operations so we don't do redundant hashes.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Christian Rodriguez
Cc: Bob Feng
Cc: Liming Gao
Cc: Yonghong Zhu
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 792bc99f54..b516404696 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -661,7 +661,7 @@ class WorkspaceAutoGen(AutoGen):
#
# Generate Package level hash value
#
-GlobalData.gPackageHash[Arch] = {}
+GlobalData.gPackageHash = {}
if GlobalData.gUseHashCache:
for Pkg in Pkgs:
self._GenPkgLevelHash(Pkg)
@@ -747,7 +747,7 @@ class WorkspaceAutoGen(AutoGen):
return True
def _GenPkgLevelHash(self, Pkg):
-if Pkg.PackageName in GlobalData.gPackageHash[Pkg.Arch]:
+if Pkg.PackageName in GlobalData.gPackageHash:
return
PkgDir = os.path.join(self.BuildDir, Pkg.Arch, Pkg.PackageName)
@@ -770,7 +770,7 @@ class WorkspaceAutoGen(AutoGen):
f.close()
m.update(Content)
SaveFileOnChange(HashFile, m.hexdigest(), False)
-GlobalData.gPackageHash[Pkg.Arch][Pkg.PackageName] = m.hexdigest()
+GlobalData.gPackageHash[Pkg.PackageName] = m.hexdigest()
def _GetMetaFiles(self, Target, Toolchain, Arch):
AllWorkSpaceMetaFiles = set()
@@ -4092,14 +4092,16 @@ class ModuleAutoGen(AutoGen):
def GenModuleHash(self):
if self.Arch not in GlobalData.gModuleHash:
GlobalData.gModuleHash[self.Arch] = {}
+if self.Name in GlobalData.gModuleHash[self.Arch] and
GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+return False
m = hashlib.md5()
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x:
x.PackageName):
-if Pkg.PackageName in GlobalData.gPackageHash[self.Arch]:
-
m.update(GlobalData.gPackageHash[self.Arch][Pkg.PackageName].encode('utf-8'))
+if Pkg.PackageName in GlobalData.gPackageHash:
+
m.update(GlobalData.gPackageHash[Pkg.PackageName].encode('utf-8'))
# Add Library hash
if self.LibraryAutoGenList:
@@ -4124,9 +4126,8 @@ class ModuleAutoGen(AutoGen):
ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
if self.Name not in GlobalData.gModuleHash[self.Arch]:
GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
-if GlobalData.gBinCacheSource:
-if self.AttemptModuleCacheCopy():
-return False
+if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+return False
return SaveFileOnChange(ModuleHashFile, m.hexdigest(), False)
## Decide whether we can skip the ModuleAutoGen process
--
2.19.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68): https://edk2.groups.io/g/devel/message/68
Mute This Topic: https://groups.io/mt/30897895/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V2 3/4] BaseTools: Fix corner-cases of --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1680 Respect artifact location within directory structure. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Christian Rodriguez Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu --- BaseTools/Source/Python/AutoGen/AutoGen.py | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index b516404696..d087ca5e0e 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -3906,9 +3906,11 @@ class ModuleAutoGen(AutoGen): FileDir = path.join(GlobalData.gBinCacheDest, self.Arch, self.SourceDir, self.MetaFile.BaseName) CreateDirectory (FileDir) HashFile = path.join(self.BuildDir, self.Name + '.hash') -ModuleFile = path.join(self.OutputDir, self.Name + '.inf') if os.path.exists(HashFile): shutil.copy2(HashFile, FileDir) +if self.IsLibrary: +return +ModuleFile = path.join(self.OutputDir, self.Name + '.inf') if os.path.exists(ModuleFile): shutil.copy2(ModuleFile, FileDir) if not self.OutputFile: @@ -3920,7 +3922,11 @@ class ModuleAutoGen(AutoGen): if not os.path.isabs(File): File = os.path.join(self.OutputDir, File) if os.path.exists(File): -shutil.copy2(File, FileDir) +sub_dir = os.path.relpath(File, self.OutputDir) +destination_file = os.path.join(FileDir, sub_dir) +destination_dir = os.path.dirname(destination_file) +CreateDirectory(destination_dir) +shutil.copy2(File, destination_dir) def AttemptModuleCacheCopy(self): # If library or Module is binary do not skip by hash @@ -3944,7 +3950,11 @@ class ModuleAutoGen(AutoGen): shutil.copy2(HashFile, self.BuildDir) else: File = path.join(root, f) -shutil.copy2(File, self.OutputDir) +sub_dir = os.path.relpath(File, FileDir) +destination_file = os.path.join(self.OutputDir, sub_dir) +destination_dir = os.path.dirname(destination_file) +CreateDirectory(destination_dir) +shutil.copy2(File, destination_dir) if self.Name == "PcdPeim" or self.Name == "PcdDxe": CreatePcdDatabaseCode(self, TemplateString(), TemplateString()) return True -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69): https://edk2.groups.io/g/devel/message/69 Mute This Topic: https://groups.io/mt/30897896/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V2 1/4] BaseTools: Fix corner-cases of --hash feature
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1680 Consider modules with .inc source files as Binary Modules and do not Skip by hash. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Christian Rodriguez Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu --- BaseTools/Source/Python/AutoGen/AutoGen.py | 14 ++ 1 file changed, 14 insertions(+) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 8c7c20a386..792bc99f54 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -3923,8 +3923,13 @@ class ModuleAutoGen(AutoGen): shutil.copy2(File, FileDir) def AttemptModuleCacheCopy(self): +# If library or Module is binary do not skip by hash if self.IsBinaryModule: return False +# .inc is contains binary information so do not skip by hash as well +for f_ext in self.SourceFileList: +if '.inc' in str(f_ext): +return False FileDir = path.join(GlobalData.gBinCacheSource, self.Arch, self.SourceDir, self.MetaFile.BaseName) HashFile = path.join(FileDir, self.Name + '.hash') if os.path.exists(HashFile): @@ -4126,7 +4131,16 @@ class ModuleAutoGen(AutoGen): ## Decide whether we can skip the ModuleAutoGen process def CanSkipbyHash(self): +# If library or Module is binary do not skip by hash +if self.IsBinaryModule: +return False +# .inc is contains binary information so do not skip by hash as well +for f_ext in self.SourceFileList: +if '.inc' in str(f_ext): +return False if GlobalData.gUseHashCache: +# If there is a valid hash or function generated a valid hash; function will return False +# and the statement below will return True return not self.GenModuleHash() return False -- 2.19.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#67): https://edk2.groups.io/g/devel/message/67 Mute This Topic: https://groups.io/mt/30897894/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
