Re: [edk2-devel] [PATCH] BaseTools: Hash false success with back to back builds

2019-04-10 Thread BobCF
The patch looks good for me.

Reviewed-by: Bob Feng 

-Bob

-Original Message-
From: Rodriguez, Christian 
Sent: Wednesday, April 10, 2019 11:27 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming ; 
Zhu, Yonghong 
Subject: [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)
   

[edk2-devel] [PATCH] BaseTools: Hash false success with back to back builds

2019-04-10 Thread Christian Rodriguez
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
 Pa.CreateMakeFile(False)
 if BuildTask.HasError():
+