Implementing relative workspacesrelative_workspaces

This fixes #191

A note has been added to NEWS explaining backwards
compatibility issues

6 files changed, 97 insertions, 54 deletions

diff --git a/NEWS b/NEWS
index 05466b8c0..5b2e1afd3 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,10 @@ buildstream 1.1.5
     to not be scheduled and fail during artifact assembly,
     and display the retry prompt during an interactive session.
 
+  o Due to enabling the use of relative workspaces, "Legacy" workspaces
+    may need to be closed and remade before the changes will affect them.
+    Downgrading after using this feature may result in workspaces
+    not functioning correctly
 
 =================
 buildstream 1.1.4
diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py
index 3abc31d40..478f0ff14 100644
--- a/buildstream/_frontend/widget.py
+++ b/buildstream/_frontend/widget.py
@@ -418,7 +418,9 @@ class LogLine(Widget):
             if "%{workspace-dirs" in format_:
                 workspace = element._get_workspace()
                 if workspace is not None:
-                    path = workspace.path.replace(os.getenv('HOME', '/root'), '~')
+                    path = workspace.get_absolute_path()
+                    if path.startswith("~/"):
+                        path = os.path.join(os.getenv('HOME', '/root'), path[2:])
                     line = p.fmt_subst(line, 'workspace-dirs', "Workspace: {}".format(path))
                 else:
                     line = p.fmt_subst(
diff --git a/buildstream/_stream.py b/buildstream/_stream.py
index f17d641de..37636b353 100644
--- a/buildstream/_stream.py
+++ b/buildstream/_stream.py
@@ -460,7 +460,7 @@ class Stream():
                                               selection=PipelineSelection.REDIRECT,
                                               track_selection=PipelineSelection.REDIRECT)
         target = elements[0]
-        workdir = os.path.abspath(directory)
+        directory = os.path.abspath(directory)
 
         if not list(target.sources()):
             build_depends = [x.name for x in target.dependencies(Scope.BUILD, recurse=False)]
@@ -476,7 +476,7 @@ class Stream():
         workspace = workspaces.get_workspace(target._get_full_name())
         if workspace and not force:
             raise StreamError("Workspace '{}' is already defined at: {}"
-                              .format(target.name, workspace.path))
+                              .format(target.name, workspace.get_absolute_path()))
 
         # If we're going to checkout, we need at least a fetch,
         # if we were asked to track first, we're going to fetch anyway.
@@ -502,7 +502,7 @@ class Stream():
         except OSError as e:
             raise StreamError("Failed to create workspace directory: {}".format(e)) from e
 
-        workspaces.create_workspace(target._get_full_name(), workdir)
+        workspaces.create_workspace(target._get_full_name(), directory)
 
         if not no_checkout:
             with target.timed_activity("Staging sources to {}".format(directory)):
@@ -526,12 +526,12 @@ class Stream():
         # Remove workspace directory if prompted
         if remove_dir:
             with self._context.timed_activity("Removing workspace directory {}"
-                                              .format(workspace.path)):
+                                              .format(workspace.get_absolute_path())):
                 try:
-                    shutil.rmtree(workspace.path)
+                    shutil.rmtree(workspace.get_absolute_path())
                 except OSError as e:
                     raise StreamError("Could not remove  '{}': {}"
-                                      .format(workspace.path, e)) from e
+                                      .format(workspace.get_absolute_path(), e)) from e
 
         # Delete the workspace and save the configuration
         workspaces.delete_workspace(element_name)
@@ -574,28 +574,30 @@ class Stream():
 
         for element in elements:
             workspace = workspaces.get_workspace(element._get_full_name())
-
+            workspace_path = workspace.get_absolute_path()
             if soft:
                 workspace.prepared = False
                 self._message(MessageType.INFO, "Reset workspace state for {} at: {}"
-                              .format(element.name, workspace.path))
+                              .format(element.name, workspace_path))
                 continue
 
             with element.timed_activity("Removing workspace directory {}"
-                                        .format(workspace.path)):
+                                        .format(workspace_path)):
                 try:
-                    shutil.rmtree(workspace.path)
+                    shutil.rmtree(workspace_path)
                 except OSError as e:
                     raise StreamError("Could not remove  '{}': {}"
-                                      .format(workspace.path, e)) from e
+                                      .format(workspace_path, e)) from e
 
             workspaces.delete_workspace(element._get_full_name())
-            workspaces.create_workspace(element._get_full_name(), workspace.path)
+            workspaces.create_workspace(element._get_full_name(), workspace_path)
 
-            with element.timed_activity("Staging sources to {}".format(workspace.path)):
+            with element.timed_activity("Staging sources to {}".format(workspace_path)):
                 element._open_workspace()
 
-            self._message(MessageType.INFO, "Reset workspace for {} at: {}".format(element.name, workspace.path))
+            self._message(MessageType.INFO,
+                          "Reset workspace for {} at: {}".format(element.name,
+                                                                 workspace_path))
 
         workspaces.save_config()
 
@@ -632,7 +634,7 @@ class Stream():
         for element_name, workspace_ in self._context.get_workspaces().list():
             workspace_detail = {
                 'element': element_name,
-                'directory': workspace_.path,
+                'directory': workspace_.get_absolute_path(),
             }
             workspaces.append(workspace_detail)
 
diff --git a/buildstream/_workspaces.py b/buildstream/_workspaces.py
index 3f474b8ca..adffaa694 100644
--- a/buildstream/_workspaces.py
+++ b/buildstream/_workspaces.py
@@ -26,14 +26,6 @@ from ._exceptions import LoadError, LoadErrorReason
 
 BST_WORKSPACE_FORMAT_VERSION = 3
 
-# Hold on to a list of members which get serialized
-_WORKSPACE_MEMBERS = [
-    'prepared',
-    'path',
-    'last_successful',
-    'running_files'
-]
-
 
 # Workspace()
 #
@@ -56,7 +48,7 @@ class Workspace():
     def __init__(self, toplevel_project, *, last_successful=None, path=None, prepared=False, running_files=None):
         self.prepared = prepared
         self.last_successful = last_successful
-        self.path = path
+        self._path = path
         self.running_files = running_files if running_files is not None else {}
 
         self._toplevel_project = toplevel_project
@@ -64,14 +56,20 @@ class Workspace():
 
     # to_dict()
     #
-    # Convert this object to a dict for serialization purposes
+    # Convert a list of members which get serialized to a dict for serialization purposes
     #
     # Returns:
     #     (dict) A dict representation of the workspace
     #
     def to_dict(self):
-        return {key: val for key, val in self.__dict__.items()
-                if key in _WORKSPACE_MEMBERS and val is not None}
+        ret = {
+            'prepared': self.prepared,
+            'path': self._path,
+            'running_files': self.running_files
+        }
+        if self.last_successful is not None:
+            ret["last_successful"] = self.last_successful
+        return ret
 
     # from_dict():
     #
@@ -103,15 +101,7 @@ class Workspace():
     #    True if the workspace differs from 'other', otherwise False
     #
     def differs(self, other):
-
-        for member in _WORKSPACE_MEMBERS:
-            member_a = getattr(self, member)
-            member_b = getattr(other, member)
-
-            if member_a != member_b:
-                return True
-
-        return False
+        return self.to_dict() != other.to_dict()
 
     # invalidate_key()
     #
@@ -133,7 +123,7 @@ class Workspace():
         if os.path.isdir(fullpath):
             utils.copy_files(fullpath, directory)
         else:
-            destfile = os.path.join(directory, os.path.basename(self.path))
+            destfile = os.path.join(directory, os.path.basename(self.get_absolute_path()))
             utils.safe_copy(fullpath, destfile)
 
     # add_running_files()
@@ -189,7 +179,7 @@ class Workspace():
                 filelist = utils.list_relative_paths(fullpath)
                 filelist = [(relpath, os.path.join(fullpath, relpath)) for relpath in filelist]
             else:
-                filelist = [(self.path, fullpath)]
+                filelist = [(self.get_absolute_path(), fullpath)]
 
             self._key = [(relpath, unique_key(fullpath)) for relpath, fullpath in filelist]
 
@@ -200,7 +190,7 @@ class Workspace():
     # Returns: The absolute path of the element's workspace.
     #
     def get_absolute_path(self):
-        return os.path.join(self._toplevel_project.directory, self.path)
+        return os.path.join(self._toplevel_project.directory, self._path)
 
 
 # Workspaces()
@@ -236,6 +226,9 @@ class Workspaces():
     #    path (str) - The path in which the workspace should be kept
     #
     def create_workspace(self, element_name, path):
+        if path.startswith(self._toplevel_project.directory):
+            path = os.path.relpath(path, self._toplevel_project.directory)
+
         self._workspaces[element_name] = Workspace(self._toplevel_project, path=path)
 
         return self._workspaces[element_name]
diff --git a/buildstream/element.py b/buildstream/element.py
index a011869cd..a099e934b 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -1403,7 +1403,8 @@ class Element(Plugin):
                     # If mount_workspaces is set and we're doing incremental builds,
                     # the workspace is already mounted into the sandbox.
                     if not (mount_workspaces and self.__can_build_incrementally()):
-                        with self.timed_activity("Staging local files at {}".format(workspace.path)):
+                        with self.timed_activity("Staging local files at {}"
+                                                 .format(workspace.get_absolute_path())):
                             workspace.stage(temp_staging_directory)
                 else:
                     # No workspace, stage directly
@@ -1566,7 +1567,7 @@ class Element(Plugin):
                         path_components = self.__staged_sources_directory.lstrip(os.sep).split(os.sep)
                         sandbox_vpath = sandbox_vroot.descend(path_components)
                         try:
-                            sandbox_vpath.import_files(workspace.path)
+                            sandbox_vpath.import_files(workspace.get_absolute_path())
                         except UtilError as e:
                             self.warn("Failed to preserve workspace state for failed build sysroot: {}"
                                       .format(e))
@@ -1893,7 +1894,7 @@ class Element(Plugin):
                 source._init_workspace(temp)
 
             # Now hardlink the files into the workspace target.
-            utils.link_files(temp, workspace.path)
+            utils.link_files(temp, workspace.get_absolute_path())
 
     # _get_workspace():
     #
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index 566fbb1cd..fff9bf00a 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -18,12 +18,13 @@ DATA_DIR = os.path.join(
 )
 
 
-def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''):
-    project = os.path.join(datafiles.dirname, datafiles.basename)
-    bin_files_path = os.path.join(project, 'files', 'bin-files')
-    element_path = os.path.join(project, 'elements')
+def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None):
+    if not workspace_dir:
+        workspace_dir = os.path.join(str(tmpdir), 'workspace{}'.format(suffix))
+    project_path = os.path.join(datafiles.dirname, datafiles.basename)
+    bin_files_path = os.path.join(project_path, 'files', 'bin-files')
+    element_path = os.path.join(project_path, 'elements')
     element_name = 'workspace-test-{}{}.bst'.format(kind, suffix)
-    workspace = os.path.join(str(tmpdir), 'workspace{}'.format(suffix))
 
     # Create our repo object of the given source type with
     # the bin files, and then collect the initial ref.
@@ -45,7 +46,7 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''):
                             element_name))
 
     # Assert that there is no reference, a track & fetch is needed
-    state = cli.get_element_state(project, element_name)
+    state = cli.get_element_state(project_path, element_name)
     if track:
         assert state == 'no reference'
     else:
@@ -56,20 +57,20 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''):
     args = ['workspace', 'open']
     if track:
         args.append('--track')
-    args.extend([element_name, workspace])
+    args.extend([element_name, workspace_dir])
+    result = cli.run(project=project_path, args=args)
 
-    result = cli.run(project=project, args=args)
     result.assert_success()
 
     # Assert that we are now buildable because the source is
     # now cached.
-    assert cli.get_element_state(project, element_name) == 'buildable'
+    assert cli.get_element_state(project_path, element_name) == 'buildable'
 
     # Check that the executable hello file is found in the workspace
-    filename = os.path.join(workspace, 'usr', 'bin', 'hello')
+    filename = os.path.join(workspace_dir, 'usr', 'bin', 'hello')
     assert os.path.exists(filename)
 
-    return (element_name, project, workspace)
+    return (element_name, project_path, workspace_dir)
 
 
 @pytest.mark.datafiles(DATA_DIR)
@@ -191,6 +192,46 @@ def test_close(cli, tmpdir, datafiles, kind):
 
 
 @pytest.mark.datafiles(DATA_DIR)
+def test_close_external_after_move_project(cli, tmpdir, datafiles):
+    tmp_parent = os.path.dirname(str(tmpdir))
+    workspace_dir = os.path.join(tmp_parent, "workspace")
+    element_name, project_path, _ = open_workspace(cli, tmpdir, datafiles, 'git', False, "", workspace_dir)
+    assert os.path.exists(workspace_dir)
+    tmp_dir = os.path.join(tmp_parent, 'external_project')
+    shutil.move(project_path, tmp_dir)
+    assert os.path.exists(tmp_dir)
+
+    # Close the workspace
+    result = cli.run(configure=False, project=tmp_dir, args=[
+        'workspace', 'close', '--remove-dir', element_name
+    ])
+    result.assert_success()
+
+    # Assert the workspace dir has been deleted
+    assert not os.path.exists(workspace_dir)
+    # Move directory back inside tmp directory so it can be recognised
+    shutil.move(tmp_dir, project_path)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_close_internal_after_move_project(cli, tmpdir, datafiles):
+    element_name, project, _ = open_workspace(cli, tmpdir, datafiles, 'git', False)
+    tmp_dir = os.path.join(os.path.dirname(str(tmpdir)), 'external_project')
+    shutil.move(str(tmpdir), tmp_dir)
+    assert os.path.exists(tmp_dir)
+
+    # Close the workspace
+    result = cli.run(configure=False, project=tmp_dir, args=[
+        'workspace', 'close', '--remove-dir', element_name
+    ])
+    result.assert_success()
+
+    # Assert the workspace dir has been deleted
+    workspace = os.path.join(tmp_dir, 'workspace')
+    assert not os.path.exists(workspace)
+
+
+@pytest.mark.datafiles(DATA_DIR)
 def test_close_removed(cli, tmpdir, datafiles):
     element_name, project, workspace = open_workspace(cli, tmpdir, datafiles, 'git', False)