Merge branch 'danielsilverstone-ct/missing-sh-fix' into 'master'

Cache failed builds caused by missing-command

Closes #1101

See merge request BuildStream/buildstream!1543

8 files changed, 63 insertions, 18 deletions

diff --git a/src/buildstream/sandbox/_sandboxbuildbox.py b/src/buildstream/sandbox/_sandboxbuildbox.py
index 417d2224d..99c7322a3 100644
--- a/src/buildstream/sandbox/_sandboxbuildbox.py
+++ b/src/buildstream/sandbox/_sandboxbuildbox.py
@@ -23,7 +23,7 @@ from contextlib import ExitStack
 import psutil
 
 from .. import utils, _signals, ProgramNotFoundError
-from . import Sandbox, SandboxFlags
+from . import Sandbox, SandboxFlags, SandboxCommandError
 from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2
 from ..storage._casbaseddirectory import CasBasedDirectory
 from .._exceptions import SandboxError
@@ -73,9 +73,9 @@ class SandboxBuildBox(Sandbox):
         scratch_directory = self._get_scratch_directory()
 
         if not self._has_command(command[0], env):
-            raise SandboxError("Staged artifacts do not provide command "
-                               "'{}'".format(command[0]),
-                               reason='missing-command')
+            raise SandboxCommandError("Staged artifacts do not provide command "
+                                      "'{}'".format(command[0]),
+                                      reason='missing-command')
 
         # Grab the full path of the buildbox binary
         try:
diff --git a/src/buildstream/sandbox/_sandboxbwrap.py b/src/buildstream/sandbox/_sandboxbwrap.py
index 81e9f34de..d8541354c 100644
--- a/src/buildstream/sandbox/_sandboxbwrap.py
+++ b/src/buildstream/sandbox/_sandboxbwrap.py
@@ -36,7 +36,7 @@ import psutil
 
 from .._exceptions import SandboxError
 from .. import utils, _signals
-from . import Sandbox, SandboxFlags
+from . import Sandbox, SandboxFlags, SandboxCommandError
 from .. import _site
 
 
@@ -142,9 +142,9 @@ class SandboxBwrap(Sandbox):
         root_directory = self.get_virtual_directory()._get_underlying_directory()
 
         if not self._has_command(command[0], env):
-            raise SandboxError("Staged artifacts do not provide command "
-                               "'{}'".format(command[0]),
-                               reason='missing-command')
+            raise SandboxCommandError("Staged artifacts do not provide command "
+                                      "'{}'".format(command[0]),
+                                      reason='missing-command')
 
         # NOTE: MountMap transitively imports `_fuse/fuse.py` which raises an
         # EnvironmentError when fuse is not found. Since this module is
diff --git a/src/buildstream/sandbox/_sandboxchroot.py b/src/buildstream/sandbox/_sandboxchroot.py
index 084ed5b6c..eb8f53308 100644
--- a/src/buildstream/sandbox/_sandboxchroot.py
+++ b/src/buildstream/sandbox/_sandboxchroot.py
@@ -31,7 +31,7 @@ from .. import utils
 from .. import _signals
 from ._mounter import Mounter
 from ._mount import MountMap
-from . import Sandbox, SandboxFlags
+from . import Sandbox, SandboxFlags, SandboxCommandError
 
 
 class SandboxChroot(Sandbox):
@@ -78,9 +78,9 @@ class SandboxChroot(Sandbox):
     def _run(self, command, flags, *, cwd, env):
 
         if not self._has_command(command[0], env):
-            raise SandboxError("Staged artifacts do not provide command "
-                               "'{}'".format(command[0]),
-                               reason='missing-command')
+            raise SandboxCommandError("Staged artifacts do not provide command "
+                                      "'{}'".format(command[0]),
+                                      reason='missing-command')
 
         stdout, stderr = self._get_output()
 
diff --git a/src/buildstream/sandbox/_sandboxdummy.py b/src/buildstream/sandbox/_sandboxdummy.py
index 750ddb05d..ae3d5e512 100644
--- a/src/buildstream/sandbox/_sandboxdummy.py
+++ b/src/buildstream/sandbox/_sandboxdummy.py
@@ -17,7 +17,7 @@
 #  Authors:
 
 from .._exceptions import SandboxError
-from .sandbox import Sandbox
+from .sandbox import Sandbox, SandboxCommandError
 
 
 class SandboxDummy(Sandbox):
@@ -28,9 +28,9 @@ class SandboxDummy(Sandbox):
     def _run(self, command, flags, *, cwd, env):
 
         if not self._has_command(command[0], env):
-            raise SandboxError("Staged artifacts do not provide command "
-                               "'{}'".format(command[0]),
-                               reason='missing-command')
+            raise SandboxCommandError("Staged artifacts do not provide command "
+                                      "'{}'".format(command[0]),
+                                      reason='missing-command')
 
         raise SandboxError("This platform does not support local builds: {}".format(self._reason),
                            reason="unavailable-local-sandbox")
diff --git a/src/buildstream/sandbox/sandbox.py b/src/buildstream/sandbox/sandbox.py
index 6956a7d59..879587748 100644
--- a/src/buildstream/sandbox/sandbox.py
+++ b/src/buildstream/sandbox/sandbox.py
@@ -88,9 +88,10 @@ class SandboxCommandError(SandboxError):
        message (str): The error message to report to the user
        detail (str): The detailed error string
        collect (str): An optional directory containing partial install contents
+       reason (str): An optional reason string (defaults to 'command-failed')
     """
-    def __init__(self, message, *, detail=None, collect=None):
-        super().__init__(message, detail=detail, reason='command-failed')
+    def __init__(self, message, *, detail=None, collect=None, reason='command-failed'):
+        super().__init__(message, detail=detail, reason=reason)
 
         self.collect = collect
 
diff --git a/tests/frontend/project/elements/manual.bst b/tests/frontend/project/elements/manual.bst
new file mode 100644
index 000000000..142409a08
--- /dev/null
+++ b/tests/frontend/project/elements/manual.bst
@@ -0,0 +1,9 @@
+kind: manual
+
+config:
+  build-commands:
+    - echo "hello"
+
+sources:
+  - kind: local
+    path: elements/manual.bst
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index ad0fc7371..a21538325 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -1333,3 +1333,37 @@ def test_build_all(cli, tmpdir, datafiles, case, strict, non_workspaced_elements
     # Assert that the target is built
     assert cli.get_element_states(project, all_elements) == \
         {elem: "cached" for elem in all_elements}
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize('strict', ['strict', 'non-strict'])
+def test_show_workspace_logs(cli, tmpdir, datafiles, strict):
+    project = str(datafiles)
+    workspace = os.path.join(str(tmpdir), 'workspace')
+    target = 'manual.bst'
+
+    # Configure strict mode
+    strict_mode = True
+    if strict != 'strict':
+        strict_mode = False
+    cli.configure({
+        'projects': {
+            'test': {
+                'strict': strict_mode
+            }
+        }
+    })
+
+    # First open the workspace
+    result = cli.run(project=project, args=['workspace', 'open', '--directory', workspace, target])
+    result.assert_success()
+
+    # Build the element
+    result = cli.run(project=project, args=['build', target])
+    result.assert_task_error(ErrorDomain.SANDBOX, 'missing-command')
+
+    result = cli.run(project=project, args=['artifact', 'log', target])
+    result.assert_success()
+
+    # Assert that the log is not empty
+    assert result.output != ""
diff --git a/tests/sandboxes/missing-command.py b/tests/sandboxes/missing-command.py
index 0277389a9..171e855f7 100644
--- a/tests/sandboxes/missing-command.py
+++ b/tests/sandboxes/missing-command.py
@@ -20,3 +20,4 @@ def test_missing_command(cli, datafiles):
     project = str(datafiles)
     result = cli.run(project=project, args=['build', 'no-runtime.bst'])
     result.assert_task_error(ErrorDomain.SANDBOX, 'missing-command')
+    assert cli.get_element_state(project, 'no-runtime.bst') == 'failed'