Merge branch 'juerg/push' into 'master'

_artifactcache.py: Skip push only if server has identical artifact

See merge request BuildStream/buildstream!1976

2 files changed, 48 insertions, 7 deletions

diff --git a/src/buildstream/_artifactcache.py b/src/buildstream/_artifactcache.py
index c1e87b6dc..46fc5fbfb 100644
--- a/src/buildstream/_artifactcache.py
+++ b/src/buildstream/_artifactcache.py
@@ -541,26 +541,27 @@ class ArtifactCache(BaseCache):
 
         keys = list(utils._deduplicate([artifact_proto.strong_key, artifact_proto.weak_key]))
 
-        # Check whether the artifact is on the server
+        pushed = False
+
         for key in keys:
             try:
-                remote.get_artifact(element.get_artifact_name(key=key))
+                remote_artifact = remote.get_artifact(element.get_artifact_name(key=key))
+                # Skip push if artifact is already on the server
+                if remote_artifact == artifact_proto:
+                    continue
             except grpc.RpcError as e:
                 if e.code() != grpc.StatusCode.NOT_FOUND:
                     raise ArtifactError(
                         "Error checking artifact cache with status {}: {}".format(e.code().name, e.details())
                     )
-            else:
-                return False
 
-        # If not, we send the artifact proto
-        for key in keys:
             try:
                 remote.update_artifact(element.get_artifact_name(key=key), artifact_proto)
+                pushed = True
             except grpc.RpcError as e:
                 raise ArtifactError("Failed to push artifact with status {}: {}".format(e.code().name, e.details()))
 
-        return True
+        return pushed
 
     # _pull_artifact_storage():
     #
diff --git a/tests/frontend/push.py b/tests/frontend/push.py
index 26dd6cb4f..50e35461f 100644
--- a/tests/frontend/push.py
+++ b/tests/frontend/push.py
@@ -675,3 +675,43 @@ def test_push_after_rebuild(cli, tmpdir, datafiles):
         result.assert_success()
         assert result.get_pushed_elements() == ["random.bst"]
         assert cli.get_element_state(project, "random.bst") == "cached"
+
+
+# Test that push after rebuilding a non-reproducible element updates the
+# artifact on the server.
+@pytest.mark.datafiles(DATA_DIR)
+def test_push_update_after_rebuild(cli, tmpdir, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+
+    generate_project(
+        project,
+        config={
+            "element-path": "elements",
+            "min-version": "2.0",
+            "plugins": [{"origin": "local", "path": "plugins", "elements": ["randomelement"]}],
+        },
+    )
+
+    with create_artifact_share(os.path.join(str(tmpdir), "artifactshare")) as share:
+        cli.configure({"artifacts": {"url": share.repo, "push": True}})
+
+        # Build the element and push the artifact
+        result = cli.run(project=project, args=["build", "random.bst"])
+        result.assert_success()
+        assert result.get_pushed_elements() == ["random.bst"]
+        assert cli.get_element_state(project, "random.bst") == "cached"
+
+        # Now delete the artifact and ensure it is not in the cache
+        result = cli.run(project=project, args=["artifact", "delete", "random.bst"])
+        assert cli.get_element_state(project, "random.bst") != "cached"
+
+        # Now rebuild the element. Reset config to disable pulling.
+        cli.config = None
+        result = cli.run(project=project, args=["build", "random.bst"])
+        result.assert_success()
+        assert cli.get_element_state(project, "random.bst") == "cached"
+
+        # Push the new build
+        cli.configure({"artifacts": {"url": share.repo, "push": True}})
+        result = cli.run(project=project, args=["artifact", "push", "random.bst"])
+        assert result.get_pushed_elements() == ["random.bst"]