_scheduler/queues: Don't call update state outside of error handling harness

Commit 3fa79d8da, part of an initiative for caching of the failed builds,
introduced a call to Element._update_state() after a job completes and before
entering the error handling harness intended for handling plugin raised errors.

Element._update_state() can result in triggering plugin code to
run, so this is incorrect, and causes raised errors to crash BuildStream
if they happen here.

After analyzing the code, it appears that this additional call to
Element._update_state() is unneeded, and was only added because the
state needs to be updated for a failure as well as a success.

Instead, we now have the BuildQueue call Element._assemble_done()
unconditionally, regardless of whether the build was successful
or not, which has the same effect and also reads better.

In addition, added a FIXME comment that we are still conditionally
updating the artifact cache size from BuildQueue.done() only
if the build is successful, which is incorrect because failed builds
also increase the local artifact cache size - to fix this we need
to communicate the added artifact size through Element._assemble()
regardless of whether the build succeeded or failed.

1 files changed, 0 insertions, 1 deletions

diff --git a/buildstream/_scheduler/queues/queue.py b/buildstream/_scheduler/queues/queue.py
index 909cebb44..055e2f84b 100644
--- a/buildstream/_scheduler/queues/queue.py
+++ b/buildstream/_scheduler/queues/queue.py
@@ -292,7 +292,6 @@ class Queue():
     # See the Job object for an explanation of the call signature
     #
     def _job_done(self, job, element, success, result):
-        element._update_state()
 
         # Update values that need to be synchronized in the main task
         # before calling any queue implementation