Fix cache corruption by scripts when layout and integration commands are usedvalentindavid/script-artifact-corruption-1.2

Root directory was marked as a non-artifact mount, so not using
SafeHardLink.  However integration commands executed with write access
to the root directory.

Fixes #749

10 files changed, 157 insertions, 8 deletions

diff --git a/buildstream/scriptelement.py b/buildstream/scriptelement.py
index 46afda807..10bbbf53f 100644
--- a/buildstream/scriptelement.py
+++ b/buildstream/scriptelement.py
@@ -201,16 +201,20 @@ class ScriptElement(Element):
         # Setup environment
         sandbox.set_environment(self.get_environment())
 
+        # Tell the sandbox to mount the install root
+        directories = {self.__install_root: False}
+
         # Mark the artifact directories in the layout
         for item in self.__layout:
-            if item['destination'] != '/':
-                if item['element']:
-                    sandbox.mark_directory(item['destination'], artifact=True)
-                else:
-                    sandbox.mark_directory(item['destination'])
-
-        # Tell the sandbox to mount the install root
-        sandbox.mark_directory(self.__install_root)
+            destination = item['destination']
+            was_artifact = directories.get(destination, False)
+            directories[destination] = item['element'] or was_artifact
+
+        for directory, artifact in directories.items():
+            # Root does not need to be marked as it is always mounted
+            # with artifact (unless explicitly marked non-artifact)
+            if directory != '/':
+                sandbox.mark_directory(directory, artifact=artifact)
 
     def stage(self, sandbox):
 
diff --git a/tests/integration/project/elements/script/corruption-2.bst b/tests/integration/project/elements/script/corruption-2.bst
new file mode 100644
index 000000000..39c4f2c23
--- /dev/null
+++ b/tests/integration/project/elements/script/corruption-2.bst
@@ -0,0 +1,11 @@
+kind: script
+
+depends:
+- filename: base.bst
+  type: build
+- filename: script/corruption-image.bst
+  type: build
+
+config:
+  commands:
+  - echo smashed >>/canary
diff --git a/tests/integration/project/elements/script/corruption-image.bst b/tests/integration/project/elements/script/corruption-image.bst
new file mode 100644
index 000000000..a1035f929
--- /dev/null
+++ b/tests/integration/project/elements/script/corruption-image.bst
@@ -0,0 +1,4 @@
+kind: import
+sources:
+- kind: local
+  path: files/canary
diff --git a/tests/integration/project/elements/script/corruption-integration.bst b/tests/integration/project/elements/script/corruption-integration.bst
new file mode 100644
index 000000000..c0f1d12df
--- /dev/null
+++ b/tests/integration/project/elements/script/corruption-integration.bst
@@ -0,0 +1,7 @@
+kind: stack
+
+public:
+  bst:
+    integration-commands:
+      - echo smashed >>/canary
+
diff --git a/tests/integration/project/elements/script/corruption.bst b/tests/integration/project/elements/script/corruption.bst
new file mode 100644
index 000000000..037d4daca
--- /dev/null
+++ b/tests/integration/project/elements/script/corruption.bst
@@ -0,0 +1,21 @@
+kind: script
+
+depends:
+- filename: base.bst
+  type: build
+- filename: script/corruption-image.bst
+  type: build
+- filename: script/corruption-integration.bst
+  type: build
+
+variables:
+  install-root: "/"
+
+config:
+  layout:
+  - element: base.bst
+    destination: "/"
+  - element: script/corruption-image.bst
+    destination: "/"
+  - element: script/corruption-integration.bst
+    destination: "/"
diff --git a/tests/integration/project/elements/script/marked-tmpdir.bst b/tests/integration/project/elements/script/marked-tmpdir.bst
new file mode 100644
index 000000000..506cdd5f4
--- /dev/null
+++ b/tests/integration/project/elements/script/marked-tmpdir.bst
@@ -0,0 +1,12 @@
+kind: compose
+
+depends:
+- filename: base.bst
+  type: build
+
+public:
+  bst:
+    split-rules:
+      remove:
+        - "/tmp/**"
+        - "/tmp"
diff --git a/tests/integration/project/elements/script/no-tmpdir.bst b/tests/integration/project/elements/script/no-tmpdir.bst
new file mode 100644
index 000000000..5c24e3cff
--- /dev/null
+++ b/tests/integration/project/elements/script/no-tmpdir.bst
@@ -0,0 +1,12 @@
+kind: filter
+
+depends:
+- filename: script/marked-tmpdir.bst
+  type: build
+
+config:
+  exclude:
+  - remove
+  include-orphans: True
+
+
diff --git a/tests/integration/project/elements/script/tmpdir.bst b/tests/integration/project/elements/script/tmpdir.bst
new file mode 100644
index 000000000..685a694ea
--- /dev/null
+++ b/tests/integration/project/elements/script/tmpdir.bst
@@ -0,0 +1,10 @@
+kind: script
+
+depends:
+- filename: script/no-tmpdir.bst
+  type: build
+
+config:
+  commands:
+  - |
+    mkdir -p /tmp/blah
diff --git a/tests/integration/project/files/canary b/tests/integration/project/files/canary
new file mode 100644
index 000000000..715cb3983
--- /dev/null
+++ b/tests/integration/project/files/canary
@@ -0,0 +1 @@
+alive
diff --git a/tests/integration/script.py b/tests/integration/script.py
index 88226c0b7..fb0c4c6b6 100644
--- a/tests/integration/script.py
+++ b/tests/integration/script.py
@@ -155,3 +155,70 @@ def test_script_layout(cli, tmpdir, datafiles):
         text = f.read()
 
     assert text == "Hi\n"
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_regression_cache_corruption(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    checkout_original = os.path.join(cli.directory, 'checkout-original')
+    checkout_after = os.path.join(cli.directory, 'checkout-after')
+    element_name = 'script/corruption.bst'
+    canary_element_name = 'script/corruption-image.bst'
+
+    res = cli.run(project=project, args=['build', canary_element_name])
+    assert res.exit_code == 0
+
+    res = cli.run(project=project, args=['checkout', canary_element_name,
+                                         checkout_original])
+    assert res.exit_code == 0
+
+    with open(os.path.join(checkout_original, 'canary')) as f:
+        assert f.read() == 'alive\n'
+
+    res = cli.run(project=project, args=['build', element_name])
+    assert res.exit_code == 0
+
+    res = cli.run(project=project, args=['checkout', canary_element_name,
+                                         checkout_after])
+    assert res.exit_code == 0
+
+    with open(os.path.join(checkout_after, 'canary')) as f:
+        assert f.read() == 'alive\n'
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_regression_tmpdir(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    element_name = 'script/tmpdir.bst'
+
+    res = cli.run(project=project, args=['build', element_name])
+    assert res.exit_code == 0
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_regression_cache_corruption_2(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    checkout_original = os.path.join(cli.directory, 'checkout-original')
+    checkout_after = os.path.join(cli.directory, 'checkout-after')
+    element_name = 'script/corruption-2.bst'
+    canary_element_name = 'script/corruption-image.bst'
+
+    res = cli.run(project=project, args=['build', canary_element_name])
+    assert res.exit_code == 0
+
+    res = cli.run(project=project, args=['checkout', canary_element_name,
+                                         checkout_original])
+    assert res.exit_code == 0
+
+    with open(os.path.join(checkout_original, 'canary')) as f:
+        assert f.read() == 'alive\n'
+
+    res = cli.run(project=project, args=['build', element_name])
+    assert res.exit_code == 0
+
+    res = cli.run(project=project, args=['checkout', canary_element_name,
+                                         checkout_after])
+    assert res.exit_code == 0
+
+    with open(os.path.join(checkout_after, 'canary')) as f:
+        assert f.read() == 'alive\n'