[edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling

Name posted 1 patch 3 years, 11 months ago
Failed in applying to current master (apply log)
BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++
BaseTools/Source/Python/build/build.py           | 5 ++++-
2 files changed, 10 insertions(+), 1 deletion(-)
[edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling
Posted by Name 3 years, 11 months ago
From: Irene Park <ipark@nvidia.com>

AutoGenWorker doesn't handle the exception from FatalError therefore
the build fails to return the proper error code at the exit.

Signed-off-by: Irene Park <ipark@nvidia.com>
---
 BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++
 BaseTools/Source/Python/build/build.py           | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
index 563d91b..2395964 100755
--- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
@@ -24,6 +24,7 @@ import traceback
 import sys
 from AutoGen.DataPipe import MemoryDataPipe
 import logging
+from Common.BuildToolError import FatalError
 
 def clearQ(q):
     try:
@@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread):
         self.autogen_workers = autogen_workers
         self.feedback_q = feedback_q
         self.Status = True
+        self.Error = 0
         self.error_event = error_event
     def run(self):
         try:
@@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread):
                     fin_num += 1
                 else:
                     self.Status = False
+                    self.Error = self.feedback_q.get()
                     self.TerminateWorkers()
                 if fin_num == len(self.autogen_workers):
                     self.clearQueue()
@@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process):
 
         except Empty:
             pass
+        except FatalError as e:
+            self.feedback_q.put(taskname)
+            self.feedback_q.put(e.args[0])
         except:
             self.feedback_q.put(taskname)
         finally:
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index ed3a3b9..d6e3d84 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -880,7 +880,10 @@ class Build():
 
             self.AutoGenMgr.join()
             rt = self.AutoGenMgr.Status
-            return rt, 0
+            err = 0
+            if not rt:
+                err = self.AutoGenMgr.Error
+            return rt, err
         except FatalError as e:
             return False, e.args[0]
         except:
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60371): https://edk2.groups.io/g/devel/message/60371
Mute This Topic: https://groups.io/mt/74507933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling
Posted by Irene Park 3 years, 11 months ago
A gentle reminder and adding Bob Feng and Liming Gao to CC.
Thank you,
Irene

-----Original Message-----
From: Irene Park <ipark@nvidia.com> 
Sent: Wednesday, May 27, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Irene Park <ipark@nvidia.com>
Subject: [PATCH] BaseTools/Python: Add missing FatalError handling

From: Irene Park <ipark@nvidia.com>

AutoGenWorker doesn't handle the exception from FatalError therefore the build fails to return the proper error code at the exit.

Signed-off-by: Irene Park <ipark@nvidia.com>
---
 BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++
 BaseTools/Source/Python/build/build.py           | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
index 563d91b..2395964 100755
--- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
@@ -24,6 +24,7 @@ import traceback
 import sys
 from AutoGen.DataPipe import MemoryDataPipe  import logging
+from Common.BuildToolError import FatalError
 
 def clearQ(q):
     try:
@@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread):
         self.autogen_workers = autogen_workers
         self.feedback_q = feedback_q
         self.Status = True
+        self.Error = 0
         self.error_event = error_event
     def run(self):
         try:
@@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread):
                     fin_num += 1
                 else:
                     self.Status = False
+                    self.Error = self.feedback_q.get()
                     self.TerminateWorkers()
                 if fin_num == len(self.autogen_workers):
                     self.clearQueue()
@@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process):
 
         except Empty:
             pass
+        except FatalError as e:
+            self.feedback_q.put(taskname)
+            self.feedback_q.put(e.args[0])
         except:
             self.feedback_q.put(taskname)
         finally:
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index ed3a3b9..d6e3d84 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -880,7 +880,10 @@ class Build():
 
             self.AutoGenMgr.join()
             rt = self.AutoGenMgr.Status
-            return rt, 0
+            err = 0
+            if not rt:
+                err = self.AutoGenMgr.Error
+            return rt, err
         except FatalError as e:
             return False, e.args[0]
         except:
--
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60408): https://edk2.groups.io/g/devel/message/60408
Mute This Topic: https://groups.io/mt/74507933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling
Posted by Bob Feng 3 years, 11 months ago
Hi Irene,

For this piece of code
+            self.feedback_q.put(taskname)
+            self.feedback_q.put(e.args[0])

I think there will be the case that another autogenworker process insert its taskname before the current autogenworker process do self.feedback_q.put(e.args[0]). If that case happens, the build tool will hang.

Thanks,
Bob

-----Original Message-----
From: Irene Park <ipark@nvidia.com> 
Sent: Friday, May 29, 2020 3:24 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] BaseTools/Python: Add missing FatalError handling

A gentle reminder and adding Bob Feng and Liming Gao to CC.
Thank you,
Irene

-----Original Message-----
From: Irene Park <ipark@nvidia.com> 
Sent: Wednesday, May 27, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Irene Park <ipark@nvidia.com>
Subject: [PATCH] BaseTools/Python: Add missing FatalError handling

From: Irene Park <ipark@nvidia.com>

AutoGenWorker doesn't handle the exception from FatalError therefore the build fails to return the proper error code at the exit.

Signed-off-by: Irene Park <ipark@nvidia.com>
---
 BaseTools/Source/Python/AutoGen/AutoGenWorker.py | 6 ++++++
 BaseTools/Source/Python/build/build.py           | 5 ++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
index 563d91b..2395964 100755
--- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py
@@ -24,6 +24,7 @@ import traceback
 import sys
 from AutoGen.DataPipe import MemoryDataPipe  import logging
+from Common.BuildToolError import FatalError
 
 def clearQ(q):
     try:
@@ -101,6 +102,7 @@ class AutoGenManager(threading.Thread):
         self.autogen_workers = autogen_workers
         self.feedback_q = feedback_q
         self.Status = True
+        self.Error = 0
         self.error_event = error_event
     def run(self):
         try:
@@ -113,6 +115,7 @@ class AutoGenManager(threading.Thread):
                     fin_num += 1
                 else:
                     self.Status = False
+                    self.Error = self.feedback_q.get()
                     self.TerminateWorkers()
                 if fin_num == len(self.autogen_workers):
                     self.clearQueue()
@@ -282,6 +285,9 @@ class AutoGenWorkerInProcess(mp.Process):
 
         except Empty:
             pass
+        except FatalError as e:
+            self.feedback_q.put(taskname)
+            self.feedback_q.put(e.args[0])
         except:
             self.feedback_q.put(taskname)
         finally:
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index ed3a3b9..d6e3d84 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -880,7 +880,10 @@ class Build():
 
             self.AutoGenMgr.join()
             rt = self.AutoGenMgr.Status
-            return rt, 0
+            err = 0
+            if not rt:
+                err = self.AutoGenMgr.Error
+            return rt, err
         except FatalError as e:
             return False, e.args[0]
         except:
--
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60420): https://edk2.groups.io/g/devel/message/60420
Mute This Topic: https://groups.io/mt/74507933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Python: Add missing FatalError handling
Posted by Irene Park 3 years, 11 months ago
Hi Bob,

Thanks for the review.
Here is the original description of the issue.
There has been a build failure from SCT, related to https://edk2.groups.io/g/devel/message/60407?p=,,,20,0,0,0::relevance,,gEfiFormBrowserExProtocolGuid,20,2,0,74529350. ( https://edk2.groups.io/g/devel/message/60407?p=,,,20,0,0,0::relevance,,gEfiFormBrowserExProtocolGuid,20,2,0,74529350 )
The build log printed the error code obviously but returned with sys.exit(0).
Unfortunately this behavior confused the build system and made me notice the failure so late.
If there is a concern of the conflict among multiple autogenworkers, I wonder how you think this way.

1. No change in BaseTools/Source/Python/AutoGen/AutoGenWorker.py
2. BaseTools/Source/Python/build/build.py

--- a/BaseTools/Source/Python/build/build.py

+++ b/BaseTools/Source/Python/build/build.py

@@ -880,7 +880,10 @@ class Build():

self.AutoGenMgr.join()

rt = self.AutoGenMgr.Status

-            return rt, 0

+            err = 0

+            if not rt:

+                err = UNKNOWN_ERROR << Use a fixed error type of FatalError instead of getting the exact error type from the autogenworkers.

+            return rt, err

except FatalError as e:

return False, e.args[0]

except:

In this way, no change in the multi-thread operation and build.py is able to exit with a standard default error. It's not needed to catch the exact error type.
Please share your opinion.

Thanks,
Irene

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60440): https://edk2.groups.io/g/devel/message/60440
Mute This Topic: https://groups.io/mt/74507933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-