Merge branch 'bschubert/stricter-job-scheduler-separation' into 'master'

Ensure we are not calling methods with side effects on the element graph in jobs

See merge request BuildStream/buildstream!2079

1 files changed, 24 insertions, 2 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 81e55a01b..91b809fca 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1562,6 +1562,8 @@ class Element(Plugin):
     # This unblocks pull/fetch/build.
     #
     def _set_required(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         if self.__required:
             # Already done
             return
@@ -1597,6 +1599,8 @@ class Element(Plugin):
     # required in the local cache.
     #
     def _set_artifact_files_required(self, scope=_Scope.RUN):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         if self.__artifact_files_required:
             # Already done
             return
@@ -1645,6 +1649,8 @@ class Element(Plugin):
     # in a subprocess.
     #
     def __schedule_assembly_when_necessary(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         # FIXME: We could reduce the number of function calls a bit by
         # factoring this out of this method (and checking whether we
         # should schedule at the calling end).
@@ -1676,6 +1682,7 @@ class Element(Plugin):
     #
     def _assemble_done(self, successful):
         assert self.__assemble_scheduled
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
 
         self.__assemble_done = True
 
@@ -1850,8 +1857,9 @@ class Element(Plugin):
             except VirtualDirectoryError:
                 pass
 
-        # ensure we have cache keys
-        self.__update_cache_key_non_strict()
+        # We should always have cache keys already set when caching an artifact
+        assert self.__cache_key is not None
+        assert self.__artifact._cache_key is not None
 
         with self.timed_activity("Caching artifact"):
             artifact_size = self.__artifact.cache(sandbox_build_dir, collectvdir, sourcesvdir, buildresult, publicdata)
@@ -1872,6 +1880,8 @@ class Element(Plugin):
     #   fetched_original (bool): Whether the original sources had been asked (and fetched) or not
     #
     def _fetch_done(self, fetched_original):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         self.__sources.fetch_done(fetched_original)
 
     # _pull_pending()
@@ -1916,6 +1926,8 @@ class Element(Plugin):
     # This will result in updating the element state.
     #
     def _pull_done(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         self.__pull_done = True
 
         # Artifact may become cached after pulling, so let it query the
@@ -2092,6 +2104,8 @@ class Element(Plugin):
     # the workspaces metadata first.
     #
     def _open_workspace(self):
+        assert utils._is_main_process(), "This writes to a global file and therefore must be run in the main process"
+
         context = self._get_context()
         workspace = self._get_workspace()
         assert workspace is not None
@@ -2392,6 +2406,8 @@ class Element(Plugin):
     # the appropriate counters.
     #
     def _update_ready_for_runtime_and_cached(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         if not self.__ready_for_runtime_and_cached:
             if self.__runtime_deps_uncached == 0 and self.__cache_key and self._cached_success():
                 self.__ready_for_runtime_and_cached = True
@@ -2410,6 +2426,7 @@ class Element(Plugin):
                     assert not rdep.__build_deps_uncached < 0
 
                     if rdep.__buildable_callback is not None and rdep._buildable():
+                        rdep.__update_cache_key_non_strict()
                         rdep.__buildable_callback(rdep)
                         rdep.__buildable_callback = None
 
@@ -3135,6 +3152,8 @@ class Element(Plugin):
     # in Scope.BUILD has changed in any way.
     #
     def __update_cache_keys(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         if self.__strict_cache_key is not None:
             # Cache keys already calculated
             assert self.__weak_cache_key is not None
@@ -3202,6 +3221,7 @@ class Element(Plugin):
     # it can check whether an artifact exists for that cache key.
     #
     def __update_artifact_state(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
         assert self.__artifact is None
 
         context = self._get_context()
@@ -3236,6 +3256,8 @@ class Element(Plugin):
     # a remote cache).
     #
     def __update_cache_key_non_strict(self):
+        assert utils._is_main_process(), "This has an impact on all elements and must be run in the main process"
+
         # The final cache key can be None here only in non-strict mode
         if self.__cache_key is None:
             if self._pull_pending():