From 97376136a7598601c0927715d6463bff4b20f2af Mon Sep 17 00:00:00 2001 From: Valentin David Date: Tue, 6 Nov 2018 18:54:41 +0100 Subject: Fix cache corruption by scripts when layout and integration commands are used 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 --- buildstream/scriptelement.py | 20 ++++--- .../project/elements/script/corruption-2.bst | 11 ++++ .../project/elements/script/corruption-image.bst | 4 ++ .../elements/script/corruption-integration.bst | 7 +++ .../project/elements/script/corruption.bst | 21 +++++++ .../project/elements/script/marked-tmpdir.bst | 12 ++++ .../project/elements/script/no-tmpdir.bst | 12 ++++ .../integration/project/elements/script/tmpdir.bst | 10 ++++ tests/integration/project/files/canary | 1 + tests/integration/script.py | 67 ++++++++++++++++++++++ 10 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 tests/integration/project/elements/script/corruption-2.bst create mode 100644 tests/integration/project/elements/script/corruption-image.bst create mode 100644 tests/integration/project/elements/script/corruption-integration.bst create mode 100644 tests/integration/project/elements/script/corruption.bst create mode 100644 tests/integration/project/elements/script/marked-tmpdir.bst create mode 100644 tests/integration/project/elements/script/no-tmpdir.bst create mode 100644 tests/integration/project/elements/script/tmpdir.bst create mode 100644 tests/integration/project/files/canary 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' -- cgit v1.2.1