Merge branch 'chandan/builddir-cachekey' into 'master'
buildelement: Ensure command-subdir is part of the cache key
Closes #1295
See merge request BuildStream/buildstream!1920
diff --git a/NEWS b/NEWS
index b69e3c9..af362eb 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,10 @@
Plugins
-------
+ o Cache keys will change for all elements that have defined the
+ `command-subdir` variable. This is the result of fixing a bug where this
+ variable was not included in the cache key correctly.
+
o The `pip` element has been removed. Please use the one from bst-plugins-experimental
diff --git a/src/buildstream/buildelement.py b/src/buildstream/buildelement.py
index ce6bb33..aa42706 100644
--- a/src/buildstream/buildelement.py
+++ b/src/buildstream/buildelement.py
@@ -171,6 +171,8 @@
# extend the configuration
node.validate_keys(_command_steps)
+ self._command_subdir = self.get_variable("command-subdir") # pylint: disable=attribute-defined-outside-init
+
for command_name in _legacy_command_steps:
self.__commands[command_name] = node.get_str_list(command_name, [])
@@ -183,6 +185,9 @@
for command_name, command_list in self.__commands.items():
dictionary[command_name] = command_list
+ if self._command_subdir:
+ dictionary["command-subdir"] = self._command_subdir
+
# Specifying notparallel for a given element effects the
# cache key, while having the side effect of setting max-jobs to 1,
# which is normally automatically resolved and does not affect
@@ -201,9 +206,8 @@
sandbox.mark_directory(install_root)
# Allow running all commands in a specified subdirectory
- command_subdir = self.get_variable("command-subdir")
- if command_subdir:
- command_dir = os.path.join(build_root, command_subdir)
+ if self._command_subdir:
+ command_dir = os.path.join(build_root, self._command_subdir)
else:
command_dir = build_root
sandbox.set_work_directory(command_dir)
diff --git a/tests/integration/manual.py b/tests/integration/manual.py
index 23fa68e..defc250 100644
--- a/tests/integration/manual.py
+++ b/tests/integration/manual.py
@@ -2,6 +2,7 @@
# pylint: disable=redefined-outer-name
import os
+import shutil
import pytest
from buildstream import _yaml
@@ -16,7 +17,7 @@
DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project")
-def create_manual_element(name, path, config, variables, environment):
+def create_manual_element(name, path, config, variables, environment, sources=None):
element = {
"kind": "manual",
"depends": [{"filename": "base.bst", "type": "build"}],
@@ -24,6 +25,8 @@
"variables": variables,
"environment": environment,
}
+ if sources:
+ element["sources"] = sources
os.makedirs(os.path.dirname(os.path.join(path, name)), exist_ok=True)
_yaml.roundtrip_dump(element, os.path.join(path, name))
@@ -153,3 +156,51 @@
assert "echo build" in res.stderr
assert "echo install" in res.stderr
assert "echo strip" in res.stderr
+
+
+# Regression test for https://gitlab.com/BuildStream/buildstream/-/issues/1295.
+#
+# Test that the command-subdir variable works as expected.
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+def test_manual_command_subdir(cli, datafiles):
+ project = str(datafiles)
+ checkout = os.path.join(cli.directory, "checkout")
+ element_path = os.path.join(project, "elements")
+ element_name = "manual/command-subdir.bst"
+ sources = [{"kind": "local", "path": "files/manual-element/root"}]
+
+ create_manual_element(
+ element_name, element_path, {"install-commands": ["cp hello %{install-root}"]}, {}, {}, sources=sources,
+ )
+
+ # First, verify that element builds, and has the correct expected output.
+ result = cli.run(project=project, args=["build", element_name])
+ result.assert_success()
+ result = cli.run(project=project, args=["artifact", "checkout", element_name, "--directory", checkout])
+ result.assert_success()
+ with open(os.path.join(checkout, "hello")) as f:
+ assert f.read() == "hello from root\n"
+
+ # Now, change element configuration to have a different command-subdir.
+ # This should result in a different cache key.
+ create_manual_element(
+ element_name,
+ element_path,
+ {"install-commands": ["cp hello %{install-root}"]},
+ {"command-subdir": "subdir"},
+ {},
+ sources=sources,
+ )
+
+ # Verify that the element needs to be rebuilt.
+ assert cli.get_element_state(project, element_name) == "buildable"
+
+ # Finally, ensure that the variable actually takes effect.
+ result = cli.run(project=project, args=["build", element_name])
+ result.assert_success()
+ shutil.rmtree(checkout)
+ result = cli.run(project=project, args=["artifact", "checkout", element_name, "--directory", checkout])
+ result.assert_success()
+ with open(os.path.join(checkout, "hello")) as f:
+ assert f.read() == "hello from subdir\n"
diff --git a/tests/integration/project/files/manual-element/root/hello b/tests/integration/project/files/manual-element/root/hello
new file mode 100644
index 0000000..14dee17
--- /dev/null
+++ b/tests/integration/project/files/manual-element/root/hello
@@ -0,0 +1 @@
+hello from root
diff --git a/tests/integration/project/files/manual-element/root/subdir/hello b/tests/integration/project/files/manual-element/root/subdir/hello
new file mode 100644
index 0000000..ae355a8
--- /dev/null
+++ b/tests/integration/project/files/manual-element/root/subdir/hello
@@ -0,0 +1 @@
+hello from subdir
