Merge branch 'bschubert/fix-empty-buildtree' into 'master'

_artifactcache.py: Don't push artifact blobs when no files are present

Closes #1145

See merge request BuildStream/buildstream!1630

3 files changed, 47 insertions, 2 deletions

diff --git a/src/buildstream/_artifactcache.py b/src/buildstream/_artifactcache.py
index f8d856be7..b4d4efe00 100644
--- a/src/buildstream/_artifactcache.py
+++ b/src/buildstream/_artifactcache.py
@@ -560,7 +560,8 @@ class ArtifactCache(BaseCache):
         artifact_proto = artifact._get_proto()
 
         try:
-            self.cas._send_directory(remote, artifact_proto.files)
+            if str(artifact_proto.files):
+                self.cas._send_directory(remote, artifact_proto.files)
 
             if str(artifact_proto.buildtree):
                 try:
diff --git a/src/buildstream/_cas/casserver.py b/src/buildstream/_cas/casserver.py
index 716178bb4..d4241435a 100644
--- a/src/buildstream/_cas/casserver.py
+++ b/src/buildstream/_cas/casserver.py
@@ -502,7 +502,8 @@ class _ArtifactServicer(artifact_pb2_grpc.ArtifactServiceServicer):
 
         if self.update_cas:
             # Check that the files specified are in the CAS
-            self._check_directory("files", artifact.files, context)
+            if str(artifact.files):
+                self._check_directory("files", artifact.files, context)
 
             # Unset protocol buffers don't evaluated to False but do return empty
             # strings, hence str()
diff --git a/tests/integration/cachedfail.py b/tests/integration/cachedfail.py
index 08733ab71..e3c05e691 100644
--- a/tests/integration/cachedfail.py
+++ b/tests/integration/cachedfail.py
@@ -183,6 +183,49 @@ def test_push_cached_fail(cli, tmpdir, datafiles, on_error):
         assert share.get_artifact(cli.get_artifact_name(project, 'test', 'element.bst'))
 
 
+@pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("on_error", ("continue", "quit"))
+def test_push_failed_missing_shell(cli, tmpdir, datafiles, on_error):
+    """Test that we can upload a built artifact that didn't have a valid shell inside.
+
+    When we don't have a valid shell, the artifact will be empty, not even the root directory.
+    This ensures we handle the case of an entirely empty artifact correctly.
+    """
+    if on_error == 'quit':
+        pytest.xfail('https://gitlab.com/BuildStream/buildstream/issues/534')
+
+    project = str(datafiles)
+    element_path = os.path.join(project, 'elements', 'element.bst')
+
+    # Write out our test target
+    element = {
+        'kind': 'script',
+        'config': {
+            'commands': [
+                'false',
+                # Ensure unique cache key for different test variants
+                'TEST="{}"'.format(os.environ.get('PYTEST_CURRENT_TEST')),
+            ],
+        },
+    }
+    _yaml.roundtrip_dump(element, element_path)
+
+    with create_artifact_share(os.path.join(str(tmpdir), 'remote')) as share:
+        cli.configure({
+            'artifacts': {'url': share.repo, 'push': True},
+        })
+
+        # Build the element, continuing to finish active jobs on error.
+        result = cli.run(project=project, args=['--on-error={}'.format(on_error), 'build', 'element.bst'])
+        result.assert_main_error(ErrorDomain.STREAM, None)
+
+        # This element should have failed
+        assert cli.get_element_state(project, 'element.bst') == 'failed'
+        # This element should have been pushed to the remote
+        assert share.get_artifact(cli.get_artifact_name(project, 'test', 'element.bst'))
+
+
 @pytest.mark.skipif(HAVE_SANDBOX != 'bwrap', reason='Only available with bubblewrap on Linux')
 @pytest.mark.datafiles(DATA_DIR)
 def test_host_tools_errors_are_not_cached(cli, datafiles, tmp_path):