Merge branch 'traveltissues/mr3' into 'master'

Support RE workspaces (non-incremental)

Closes #933

See merge request BuildStream/buildstream!1682

7 files changed, 338 insertions, 19 deletions

diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index b2e58b9e7..3b18af691 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -452,7 +452,7 @@ class Loader:
         if workspace:
             workspace_node = {"kind": "workspace"}
             workspace_node["path"] = workspace.get_absolute_path()
-            workspace_node["ref"] = str(workspace.to_dict().get("last_successful", "ignored"))
+            workspace_node["last_successful"] = str(workspace.to_dict().get("last_successful", ""))
             node[Symbol.SOURCES] = [workspace_node]
             skip_workspace = False
 
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 86664de96..4b8409323 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -926,6 +926,7 @@ class Stream:
 
             if soft:
                 workspace.prepared = False
+                workspace.last_successful = None
                 self._message(
                     MessageType.INFO, "Reset workspace state for {} at: {}".format(element.name, workspace_path)
                 )
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index e40dc3c8c..ffce257bf 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2397,23 +2397,12 @@ class Element(Plugin):
     # Internal method for calling public abstract prepare() method.
     #
     def __prepare(self, sandbox):
-        workspace = self._get_workspace()
-
+        # FIXME:
         # We need to ensure that the prepare() method is only called
         # once in workspaces, because the changes will persist across
         # incremental builds - not desirable, for example, in the case
         # of autotools' `./configure`.
-        if not (workspace and workspace.prepared):
-            self.prepare(sandbox)
-
-            if workspace:
-
-                def mark_workspace_prepared():
-                    workspace.prepared = True
-
-                # Defer workspace.prepared setting until pending batch commands
-                # have been executed.
-                sandbox._callback(mark_workspace_prepared)
+        self.prepare(sandbox)
 
     # __preflight():
     #
diff --git a/src/buildstream/plugins/sources/workspace.py b/src/buildstream/plugins/sources/workspace.py
index c7d16f685..3d4c93b5c 100644
--- a/src/buildstream/plugins/sources/workspace.py
+++ b/src/buildstream/plugins/sources/workspace.py
@@ -55,14 +55,16 @@ class WorkspaceSource(Source):
         self.__unique_key = None
         # the digest of the Directory following the import of the workspace
         self.__digest = None
+        # the cache key of the last successful workspace
+        self.__last_successful = None
 
     def track(self) -> SourceRef:  # pylint: disable=arguments-differ
         return None
 
     def configure(self, node: MappingNode) -> None:
-        node.validate_keys(["path", "ref", "kind"])
+        node.validate_keys(["path", "last_successful", "kind"])
         self.path = node.get_str("path")
-        self.__digest = node.get_str("ref")
+        self.__last_successful = node.get_str("last_successful")
 
     def preflight(self) -> None:
         pass  # pragma: nocover
diff --git a/src/buildstream/testing/integration.py b/src/buildstream/testing/integration.py
index 9a1a48816..269b3d317 100644
--- a/src/buildstream/testing/integration.py
+++ b/src/buildstream/testing/integration.py
@@ -49,11 +49,25 @@ def walk_dir(root):
 
 
 # Ensure that a directory contains the given filenames.
-def assert_contains(directory, expected):
+# If `strict` is `True` then no additional filenames are allowed.
+def assert_contains(directory, expected, strict=False):
+    expected = set(expected)
     missing = set(expected)
-    missing.difference_update(walk_dir(directory))
+    found = set(walk_dir(directory))
+
+    # elements expected but not found
+    missing.difference_update(found)
+
     if missing:
-        raise AssertionError("Missing {} expected elements from list: {}".format(len(missing), missing))
+        msg = "Missing {} expected elements from list: {}".format(len(missing), missing)
+        raise AssertionError(msg)
+
+    if strict:
+        # elements found but not expected
+        found.difference_update(expected)
+        msg = "{} additional elements were present in the directory: {}".format(len(found), found)
+        if found:
+            raise AssertionError(msg)
 
 
 class IntegrationCache:
diff --git a/tests/remoteexecution/project/elements/autotools/amhello.bst b/tests/remoteexecution/project/elements/autotools/amhello.bst
index ee3a029d8..3af67ef3c 100644
--- a/tests/remoteexecution/project/elements/autotools/amhello.bst
+++ b/tests/remoteexecution/project/elements/autotools/amhello.bst
@@ -8,3 +8,19 @@ sources:
 - kind: tar
   url: project_dir:/files/amhello.tar.gz
   ref: 9ba123fa4e660929e9a0aa99f0c487b7eee59c5e7594f3284d015640b90f5590
+
+config:
+
+  configure-commands:
+    - |
+      %{autogen}
+    - |
+      %{configure}
+    - |
+      date +%s > config-time
+
+  build-commands:
+    - |
+      %{make}
+    - |
+      date +%s > build-time
diff --git a/tests/remoteexecution/workspace.py b/tests/remoteexecution/workspace.py
new file mode 100644
index 000000000..4ac743490
--- /dev/null
+++ b/tests/remoteexecution/workspace.py
@@ -0,0 +1,297 @@
+# Pylint doesn't play well with fixtures and dependency injection from pytest
+# pylint: disable=redefined-outer-name
+
+import os
+import shutil
+import pytest
+
+from buildstream.testing import cli_remote_execution as cli  # pylint: disable=unused-import
+from buildstream.testing.integration import assert_contains
+
+pytestmark = pytest.mark.remoteexecution
+
+
+DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project")
+MKFILEAM = os.path.join("src", "Makefile.am")
+MKFILE = os.path.join("src", "Makefile")
+MAIN = os.path.join("src", "main.o")
+CFGMARK = "config-time"
+BLDMARK = "build-time"
+
+
+def files():
+    _input_files = [
+        ".bstproject.yaml",
+        "aclocal.m4",
+        "missing",
+        "README",
+        "install-sh",
+        "depcomp",
+        "configure.ac",
+        "compile",
+        "src",
+        os.path.join("src", "main.c"),
+        MKFILEAM,
+        "Makefile.am",
+    ]
+    input_files = [os.sep + fname for fname in _input_files]
+
+    _generated_files = [
+        "Makefile",
+        "Makefile.in",
+        "autom4te.cache",
+        os.path.join("autom4te.cache", "traces.1"),
+        os.path.join("autom4te.cache", "traces.0"),
+        os.path.join("autom4te.cache", "requests"),
+        os.path.join("autom4te.cache", "output.0"),
+        os.path.join("autom4te.cache", "output.1"),
+        "config.h",
+        "config.h.in",
+        "config.log",
+        "config.status",
+        "configure",
+        "configure.lineno",
+        os.path.join("src", "hello"),
+        os.path.join("src", ".deps"),
+        os.path.join("src", ".deps", "main.Po"),
+        MKFILE,
+        MAIN,
+        CFGMARK,
+        BLDMARK,
+        os.path.join("src", "Makefile.in"),
+        "stamp-h1",
+    ]
+    generated_files = [os.sep + fname for fname in _generated_files]
+
+    _artifacts = [
+        "usr",
+        os.path.join("usr", "lib"),
+        os.path.join("usr", "bin"),
+        os.path.join("usr", "share"),
+        os.path.join("usr", "bin", "hello"),
+        os.path.join("usr", "share", "doc"),
+        os.path.join("usr", "share", "doc", "amhello"),
+        os.path.join("usr", "share", "doc", "amhello", "README"),
+    ]
+    artifacts = [os.sep + fname for fname in _artifacts]
+    return input_files, generated_files, artifacts
+
+
+def _get_mtimes(root):
+    assert os.path.exists(root)
+    for dirname, dirnames, filenames in os.walk(root):
+        dirnames.sort()
+        filenames.sort()
+        for subdirname in dirnames:
+            fname = os.path.join(dirname, subdirname)
+            yield fname[len(root) :], os.stat(fname).st_mtime
+        for filename in filenames:
+            fname = os.path.join(dirname, filename)
+            yield fname[len(root) :], os.stat(fname).st_mtime
+
+
+def get_mtimes(root):
+    return {k: v for (k, v) in set(_get_mtimes(root))}
+
+
+def check_buildtree(
+    cli, project, element_name, input_files, generated_files, incremental=False,
+):
+    # check modified workspace dir was cached
+    #   - generated files are present
+    #   - generated files are newer than inputs
+    #   - check the date recorded in the marker file
+    #   - check that the touched file mtime is preserved from before
+
+    assert cli and project and element_name and input_files and generated_files
+
+    result = cli.run(
+        project=project,
+        args=[
+            "shell",
+            "--build",
+            element_name,
+            "--use-buildtree",
+            "always",
+            "--",
+            "find",
+            ".",
+            "-mindepth",
+            "1",
+            "-exec",
+            "stat",
+            "-c",
+            "%n::%Y",
+            "{}",
+            ";",
+        ],
+    )
+    result.assert_success()
+
+    buildtree = {}
+    inp_times = []
+    gen_times = []
+    output = result.output.splitlines()
+
+    for line in output:
+        assert "::" in line
+        fname, mtime = line.split("::")
+        # remove the symbolic dir
+        fname = fname[1:]
+        mtime = int(mtime)
+        buildtree[fname] = mtime
+
+        if incremental:
+            if fname in input_files:
+                inp_times.append(mtime)
+            else:
+                gen_times.append(mtime)
+
+    # all expected files should have been found
+    for filename in input_files + generated_files:
+        assert filename in buildtree
+
+    if incremental:
+        # at least inputs should be older than generated files
+        assert not any([inp_time > gen_time for inp_time in inp_times for gen_time in gen_times])
+
+        makefile = os.sep + "Makefile"
+        makefile_am = os.sep + "Makefile.am"
+        mainc = os.sep + os.path.join("src", "main.c")
+        maino = os.sep + os.path.join("src", "hello")
+        testfiles = [makefile, makefile_am, mainc, maino]
+        if all([testfile in buildtree for testfile in testfiles]):
+            assert buildtree[makefile] < buildtree[makefile_am]
+            assert buildtree[mainc] < buildtree[maino]
+
+    return buildtree
+
+
+def get_timemark(cli, project, element_name, marker):
+    result = cli.run(
+        project=project, args=["shell", "--build", element_name, "--use-buildtree", "always", "--", "cat", marker[1:]],
+    )
+    result.assert_success()
+    marker_time = int(result.output)
+    return marker_time
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize(
+    "modification",
+    [
+        pytest.param("none"),
+        pytest.param("content"),
+        pytest.param("time", marks=pytest.mark.xfail(reason="mtimes are set to a magic value and not stored in CAS")),
+    ],
+)
+@pytest.mark.parametrize(
+    "buildtype",
+    [
+        pytest.param("non-incremental"),
+        pytest.param(
+            "incremental", marks=pytest.mark.xfail(reason="incremental workspace builds are not yet supported")
+        ),
+    ],
+)
+def test_workspace_build(cli, tmpdir, datafiles, modification, buildtype):
+    incremental = False
+    if buildtype == "incremental":
+        incremental = True
+
+    project = str(datafiles)
+    checkout = os.path.join(cli.directory, "checkout")
+    workspace = os.path.join(cli.directory, "workspace")
+    element_name = "autotools/amhello.bst"
+
+    # cli args
+    artifact_checkout = ["artifact", "checkout", element_name, "--directory", checkout]
+    build = ["--cache-buildtrees", "always", "build", element_name]
+    input_files, generated_files, artifacts = files()
+
+    services = cli.ensure_services()
+    assert set(services) == set(["action-cache", "execution", "storage"])
+
+    # open a workspace for the element in the workspace directory
+    result = cli.run(project=project, args=["workspace", "open", "--directory", workspace, element_name])
+    result.assert_success()
+
+    # check that the workspace path exists
+    assert os.path.exists(workspace)
+
+    # add a file (asserting later that this is in the buildtree)
+    newfile = "newfile.cfg"
+    newfile_path = os.path.join(workspace, newfile)
+    with open(newfile_path, "w") as fdata:
+        fdata.write("somestring")
+    input_files.append(os.sep + newfile)
+
+    # check that the workspace *only* contains the expected input files
+    assert_contains(workspace, input_files, strict=True)
+    # save the mtimes for later comparison
+    ws_times = get_mtimes(workspace)
+
+    # build the element and cache the buildtree
+    result = cli.run(project=project, args=build)
+    result.assert_success()
+
+    # check that the local workspace is unchanged
+    assert_contains(workspace, input_files, strict=True)
+    assert ws_times == get_mtimes(workspace)
+
+    # check modified workspace dir was cached and save the time
+    # build was run
+    build_mtimes = check_buildtree(cli, project, element_name, input_files, generated_files, incremental=incremental)
+    build_timemark = get_timemark(cli, project, element_name, (os.sep + BLDMARK))
+
+    # check that the artifacts are available
+    result = cli.run(project=project, args=artifact_checkout)
+    result.assert_success()
+    assert_contains(checkout, artifacts)
+    shutil.rmtree(checkout)
+
+    # rebuild the element
+    result = cli.run(project=project, args=build)
+    result.assert_success()
+    # this should all be cached
+    # so the buildmark time should be the same
+    rebuild_mtimes = check_buildtree(cli, project, element_name, input_files, generated_files, incremental=incremental)
+    rebuild_timemark = get_timemark(cli, project, element_name, (os.sep + BLDMARK))
+
+    assert build_timemark == rebuild_timemark
+    assert build_mtimes == rebuild_mtimes
+
+    # modify the open workspace and rebuild
+    if modification != "none":
+        assert os.path.exists(newfile_path)
+
+        if modification == "time":
+            # touch a file in the workspace and save the mtime
+            os.utime(newfile_path)
+
+        elif modification == "content":
+            # change a source file
+            with open(newfile_path, "w") as fdata:
+                fdata.write("anotherstring")
+
+        # refresh input times
+        ws_times = get_mtimes(workspace)
+
+        # rebuild the element
+        result = cli.run(project=project, args=build)
+        result.assert_success()
+
+        rebuild_mtimes = check_buildtree(
+            cli, project, element_name, input_files, generated_files, incremental=incremental
+        )
+        rebuild_timemark = get_timemark(cli, project, element_name, (os.sep + BLDMARK))
+        assert build_timemark != rebuild_timemark
+
+        # check the times of the changed files
+        if incremental:
+            touched_time = os.stat(newfile_path).st_mtime
+            assert rebuild_mtimes[newfile] == touched_time
+
+        # Check workspace is unchanged
+        assert_contains(workspace, input_files, strict=True)
+        assert ws_times == get_mtimes(workspace)