Merge branch 'Qinusty/634-workspace-failed-builds' into 'master'

Do not save workspace on failed build

Closes #634

See merge request BuildStream/buildstream!812

2 files changed, 29 insertions, 5 deletions

diff --git a/buildstream/element.py b/buildstream/element.py
index 9154f10e8..25ef22ed2 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -212,7 +212,7 @@ class Element(Plugin):
         self.__staged_sources_directory = None  # Location where Element.stage_sources() was called
         self.__tainted = None                   # Whether the artifact is tainted and should not be shared
         self.__required = False                 # Whether the artifact is required in the current session
-        self.__build_result = None              # The result of assembling this Element
+        self.__build_result = None              # The result of assembling this Element (success, description, detail)
         self._build_log_path = None            # The path of the build log for this Element
 
         # hash tables of loaded artifact metadata, hashed by key
@@ -1479,11 +1479,13 @@ class Element(Plugin):
 
         self._update_state()
 
-        if self._get_workspace() and self._cached():
+        if self._get_workspace() and self._cached_success():
+            assert utils._is_main_process(), \
+                "Attempted to save workspace configuration from child process"
             #
             # Note that this block can only happen in the
-            # main process, since `self._cached()` cannot
-            # be true when assembly is completed in the task.
+            # main process, since `self._cached_success()` cannot
+            # be true when assembly is successful in the task.
             #
             # For this reason, it is safe to update and
             # save the workspaces configuration
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index 47d3b6a85..51b7d6088 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -43,7 +43,8 @@ DATA_DIR = os.path.join(
 )
 
 
-def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None, project_path=None):
+def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None,
+                   project_path=None, element_attrs=None):
     if not workspace_dir:
         workspace_dir = os.path.join(str(tmpdir), 'workspace{}'.format(suffix))
     if not project_path:
@@ -69,6 +70,8 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir
             repo.source_config(ref=ref)
         ]
     }
+    if element_attrs:
+        element = {**element, **element_attrs}
     _yaml.dump(element,
                os.path.join(element_path,
                             element_name))
@@ -854,3 +857,22 @@ def test_cache_key_workspace_in_dependencies(cli, tmpdir, datafiles, strict):
 
     # Check that the original /usr/bin/hello is not in the checkout
     assert not os.path.exists(os.path.join(checkout, 'usr', 'bin', 'hello'))
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_multiple_failed_builds(cli, tmpdir, datafiles):
+    element_config = {
+        "kind": "manual",
+        "config": {
+            "configure-commands": [
+                "unknown_command_that_will_fail"
+            ]
+        }
+    }
+    element_name, project, _ = open_workspace(cli, tmpdir, datafiles,
+                                              "git", False, element_attrs=element_config)
+
+    for _ in range(2):
+        result = cli.run(project=project, args=["build", element_name])
+        assert "BUG" not in result.stderr
+        assert cli.get_element_state(project, element_name) != "cached"