Refactoring of highlevel workspace code

Move all workspace related code out of Pipeline() and into the
frontend App() object.

Some changes in transition here include:

  o Workspaces() object methods for looking up and deleting workspaces
    now take an element name instead of an element.

  o Share code for partial App() initialization between the
    `workspace close` and `workspace list` commands

  o No longer require that an element exist in the project
    in order to close a workspace

This fixes issue #249

5 files changed, 150 insertions, 194 deletions

diff --git a/buildstream/_frontend/app.py b/buildstream/_frontend/app.py
index 954865b72..c9c006b7d 100644
--- a/buildstream/_frontend/app.py
+++ b/buildstream/_frontend/app.py
@@ -20,6 +20,7 @@
 
 import os
 import sys
+import shutil
 import resource
 import datetime
 from contextlib import contextmanager
@@ -29,12 +30,12 @@ import click
 from click import UsageError
 
 # Import buildstream public symbols
-from .. import Scope
+from .. import Scope, Consistency
 
 # Import various buildstream internals
 from .._context import Context
 from .._project import Project
-from .._exceptions import BstError, PipelineError
+from .._exceptions import BstError, PipelineError, AppError
 from .._message import Message, MessageType, unconditional_messages
 from .._pipeline import Pipeline
 from .._scheduler import Scheduler
@@ -297,6 +298,121 @@ class App():
                     self.message(MessageType.SUCCESS, session_name, elapsed=self.scheduler.elapsed_time())
                     self.print_summary()
 
+    ############################################################
+    #                   Workspace Commands                     #
+    ############################################################
+
+    # open_workspace
+    #
+    # Open a project workspace - this requires full initialization
+    #
+    # Args:
+    #    directory (str): The directory to stage the source in
+    #    no_checkout (bool): Whether to skip checking out the source
+    #    track_first (bool): Whether to track and fetch first
+    #    force (bool): Whether to ignore contents in an existing directory
+    #
+    def open_workspace(self, directory, no_checkout, track_first, force):
+
+        # When working on workspaces we only have one target
+        target = self.pipeline.targets[0]
+        workdir = os.path.abspath(directory)
+
+        if not list(target.sources()):
+            build_depends = [x.name for x in target.dependencies(Scope.BUILD, recurse=False)]
+            if not build_depends:
+                raise AppError("The given element has no sources")
+            detail = "Try opening a workspace on one of its dependencies instead:\n"
+            detail += "  \n".join(build_depends)
+            raise AppError("The given element has no sources", detail=detail)
+
+        # Check for workspace config
+        if self.project._workspaces.get_workspace(target):
+            raise AppError("Workspace '{}' is already defined.".format(target.name))
+
+        # 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.
+        if not no_checkout or track_first:
+            self.pipeline.fetch(self.scheduler, [target], track_first)
+
+        if not no_checkout and target._consistency() != Consistency.CACHED:
+            raise PipelineError("Could not stage uncached source. " +
+                                "Use `--track` to track and " +
+                                "fetch the latest version of the " +
+                                "source.")
+
+        try:
+            os.makedirs(directory, exist_ok=True)
+        except OSError as e:
+            raise AppError("Failed to create workspace directory: {}".format(e)) from e
+
+        workspace = self.project._workspaces.create_workspace(target, workdir)
+
+        if not no_checkout:
+            if not force and os.listdir(directory):
+                raise AppError("Checkout directory is not empty: {}".format(directory))
+
+            with target.timed_activity("Staging sources to {}".format(directory)):
+                workspace.open()
+
+        self.project._workspaces.save_config()
+        self.message(MessageType.INFO, "Saved workspace configuration")
+
+    # close_workspace
+    #
+    # Close a project workspace - this requires only partial initialization
+    #
+    # Args:
+    #    element_name (str): The element name to close the workspace for
+    #    remove_dir (bool): Whether to remove the associated directory
+    #
+    def close_workspace(self, element_name, remove_dir):
+
+        workspace = self.project._workspaces.get_workspace(element_name)
+
+        if workspace is None:
+            raise AppError("Workspace '{}' does not exist".format(element_name))
+
+        # Remove workspace directory if prompted
+        if remove_dir:
+            with self.context._timed_activity("Removing workspace directory {}"
+                                              .format(workspace.path)):
+                try:
+                    shutil.rmtree(workspace.path)
+                except OSError as e:
+                    raise AppError("Could not remove  '{}': {}"
+                                   .format(workspace.path, e)) from e
+
+        # Delete the workspace and save the configuration
+        self.project._workspaces.delete_workspace(element_name)
+        self.project._workspaces.save_config()
+        self.message(MessageType.INFO, "Saved workspace configuration")
+
+    # reset_workspace
+    #
+    # Reset a workspace to its original state, discarding any user
+    # changes.
+    #
+    # Args:
+    #    track (bool): Whether to also track the source
+    #    no_checkout (bool): Whether to check out the source (at all)
+    #
+    def reset_workspace(self, track, no_checkout):
+        # When working on workspaces we only have one target
+        target = self.pipeline.targets[0]
+        workspace = self.project._workspaces.get_workspace(target.name)
+
+        if workspace is None:
+            raise AppError("Workspace '{}' is currently not defined"
+                           .format(target.name))
+
+        self.close_workspace(target.name, True)
+        self.open_workspace(workspace.path, no_checkout, track, False)
+
+    ############################################################
+    #                      Local Functions                     #
+    ############################################################
+
     # Local message propagator
     #
     def message(self, message_type, message, **kwargs):
diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index dd9a62eff..bb40a45bf 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -573,7 +573,7 @@ def workspace_open(app, no_checkout, force, track_, element, directory):
     """Open a workspace for manual source modification"""
 
     with app.initialized((element,), rewritable=track_, track_elements=[element] if track_ else None):
-        app.pipeline.open_workspace(app.scheduler, directory, no_checkout, track_, force)
+        app.open_workspace(directory, no_checkout, track_, force)
 
 
 ##################################################################
@@ -588,18 +588,13 @@ def workspace_open(app, no_checkout, force, track_, element, directory):
 def workspace_close(app, remove_dir, element):
     """Close a workspace"""
 
-    with app.initialized((element,)):
-
-        if app.pipeline.project._workspaces.get_workspace(app.pipeline.targets[0]) is None:
-            click.echo("ERROR: Workspace '{}' does not exist".format(element), err=True)
+    if app.interactive and remove_dir:
+        if not click.confirm('This will remove all your changes, are you sure?'):
+            click.echo('Aborting', err=True)
             sys.exit(-1)
 
-        if app.interactive and remove_dir:
-            if not click.confirm('This will remove all your changes, are you sure?'):
-                click.echo('Aborting', err=True)
-                sys.exit(-1)
-
-        app.pipeline.close_workspace(remove_dir)
+    with app.partially_initialized():
+        app.close_workspace(element, remove_dir)
 
 
 ##################################################################
@@ -615,13 +610,13 @@ def workspace_close(app, remove_dir, element):
 @click.pass_obj
 def workspace_reset(app, track_, no_checkout, element):
     """Reset a workspace to its original state"""
-    with app.initialized((element,)):
-        if app.interactive:
-            if not click.confirm('This will remove all your changes, are you sure?'):
-                click.echo('Aborting', err=True)
-                sys.exit(-1)
+    if app.interactive:
+        if not click.confirm('This will remove all your changes, are you sure?'):
+            click.echo('Aborting', err=True)
+            sys.exit(-1)
 
-        app.pipeline.reset_workspace(app.scheduler, track_, no_checkout)
+    with app.initialized((element,)):
+        app.reset_workspace(track_, no_checkout)
 
 
 ##################################################################
@@ -632,34 +627,15 @@ def workspace_reset(app, track_, no_checkout, element):
 def workspace_list(app):
     """List open workspaces"""
 
-    from .. import _yaml
-    from .._context import Context
-    from .._project import Project
-
-    directory = app.main_options['directory']
-    config = app.main_options['config']
-
-    try:
-        context = Context()
-        context.load(config)
-    except BstError as e:
-        click.echo("Error loading user configuration: {}".format(e), err=True)
-        sys.exit(-1)
-
-    try:
-        project = Project(directory, context, cli_options=app.main_options['option'])
-    except BstError as e:
-        click.echo("Error loading project: {}".format(e), err=True)
-        sys.exit(-1)
-
-    workspaces = []
-    for element_name, workspace_ in project._workspaces.list():
-        workspace_detail = {
-            'element': element_name,
-            'directory': workspace_.path,
-        }
-        workspaces.append(workspace_detail)
-
-    _yaml.dump({
-        'workspaces': workspaces
-    })
+    with app.partially_initialized():
+        workspaces = []
+        for element_name, workspace_ in app.project._workspaces.list():
+            workspace_detail = {
+                'element': element_name,
+                'directory': workspace_.path,
+            }
+            workspaces.append(workspace_detail)
+
+        _yaml.dump({
+            'workspaces': workspaces
+        })
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 59219ab11..676b068a7 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -22,7 +22,6 @@
 import os
 import stat
 import shlex
-import shutil
 import tarfile
 import itertools
 from contextlib import contextmanager
@@ -574,141 +573,6 @@ class Pipeline():
         else:
             utils.link_files(sandbox_root, directory)
 
-    # open_workspace
-    #
-    # Open a project workspace.
-    #
-    # Args:
-    #    directory (str): The directory to stage the source in
-    #    no_checkout (bool): Whether to skip checking out the source
-    #    track_first (bool): Whether to track and fetch first
-    #    force (bool): Whether to ignore contents in an existing directory
-    #
-    def open_workspace(self, scheduler, directory, no_checkout, track_first, force):
-        # When working on workspaces we only have one target
-        target = self.targets[0]
-        workdir = os.path.abspath(directory)
-
-        if not list(target.sources()):
-            build_depends = [x.name for x in target.dependencies(Scope.BUILD, recurse=False)]
-            if not build_depends:
-                raise PipelineError("The given element has no sources or build-dependencies")
-            detail = "Try opening a workspace on one of its dependencies instead:\n"
-            detail += "  \n".join(build_depends)
-            raise PipelineError("The given element has no sources", detail=detail)
-
-        # Check for workspace config
-        if self.project._workspaces.get_workspace(target):
-            raise PipelineError("Workspace '{}' is already defined."
-                                .format(target.name))
-
-        plan = [target]
-
-        # Track/fetch if required
-        queues = []
-        track = None
-
-        if track_first:
-            track = TrackQueue()
-            queues.append(track)
-        if not no_checkout or track_first:
-            fetch = FetchQueue(skip_cached=True)
-            queues.append(fetch)
-
-        if queues:
-            queues[0].enqueue(plan)
-
-            _, status = scheduler.run(queues)
-            if status == SchedStatus.ERROR:
-                raise PipelineError()
-            elif status == SchedStatus.TERMINATED:
-                raise PipelineError(terminated=True)
-
-        if not no_checkout and target._consistency() != Consistency.CACHED:
-            raise PipelineError("Could not stage uncached source. " +
-                                "Use `--track` to track and " +
-                                "fetch the latest version of the " +
-                                "source.")
-
-        # Check directory
-        try:
-            os.makedirs(directory, exist_ok=True)
-        except OSError as e:
-            raise PipelineError("Failed to create workspace directory: {}".format(e)) from e
-
-        workspace = self.project._workspaces.create_workspace(target, workdir)
-
-        if not no_checkout:
-            if not force and os.listdir(directory):
-                raise PipelineError("Checkout directory is not empty: {}".format(directory))
-
-            with target.timed_activity("Staging sources to {}".format(directory)):
-                workspace.open()
-
-        with target.timed_activity("Saving workspace configuration"):
-            self.project._workspaces.save_config()
-
-    # close_workspace
-    #
-    # Close a project workspace
-    #
-    # Args:
-    #    remove_dir (bool) - Whether to remove the associated directory
-    #
-    def close_workspace(self, remove_dir):
-        # When working on workspaces we only have one target
-        target = self.targets[0]
-
-        # Remove workspace directory if prompted
-        if remove_dir:
-            workspace = self.project._workspaces.get_workspace(target)
-            if workspace is not None:
-                with target.timed_activity("Removing workspace directory {}"
-                                           .format(workspace.path)):
-                    try:
-                        shutil.rmtree(workspace.path)
-                    except OSError as e:
-                        raise PipelineError("Could not remove  '{}': {}"
-                                            .format(workspace.path, e)) from e
-
-        # Delete the workspace config entry
-        with target.timed_activity("Removing workspace"):
-            try:
-                self.project._workspaces.delete_workspace(target)
-            except KeyError:
-                raise PipelineError("Workspace '{}' is currently not defined"
-                                    .format(target.name))
-
-        # Update workspace config
-        self.project._workspaces.save_config()
-
-        # Reset source to avoid checking out the (now empty) workspace
-        for source in target.sources():
-            source._del_workspace()
-
-    # reset_workspace
-    #
-    # Reset a workspace to its original state, discarding any user
-    # changes.
-    #
-    # Args:
-    #    scheduler: The app scheduler
-    #    track (bool): Whether to also track the source
-    #    no_checkout (bool): Whether to check out the source (at all)
-    #
-    def reset_workspace(self, scheduler, track, no_checkout):
-        # When working on workspaces we only have one target
-        target = self.targets[0]
-        workspace = self.project._workspaces.get_workspace(target)
-
-        if workspace is None:
-            raise PipelineError("Workspace '{}' is currently not defined"
-                                .format(target.name))
-
-        self.close_workspace(True)
-
-        self.open_workspace(scheduler, workspace.path, no_checkout, track, False)
-
     # pull()
     #
     # Pulls elements from the pipeline
diff --git a/buildstream/_workspaces.py b/buildstream/_workspaces.py
index e221ca7a9..bdbd73acb 100644
--- a/buildstream/_workspaces.py
+++ b/buildstream/_workspaces.py
@@ -237,15 +237,15 @@ class Workspaces():
     # element's source at the given index
     #
     # Args:
-    #    element (Element) - The element whose workspace to return
+    #    element_name (str) - The element name whose workspace to return
     #
     # Returns:
     #    (None|Workspace)
     #
-    def get_workspace(self, element):
-        if element.name not in self._workspaces:
+    def get_workspace(self, element_name):
+        if element_name not in self._workspaces:
             return None
-        return self._workspaces[element.name]
+        return self._workspaces[element_name]
 
     # delete_workspace()
     #
@@ -254,10 +254,10 @@ class Workspaces():
     # configuration, call save_config() afterwards.
     #
     # Args:
-    #    element (Element) - The element whose workspace to delete
+    #    element_name (str) - The element name whose workspace to delete
     #
-    def delete_workspace(self, element):
-        del self._workspaces[element.name]
+    def delete_workspace(self, element_name):
+        del self._workspaces[element_name]
 
     # save_config()
     #
diff --git a/buildstream/element.py b/buildstream/element.py
index 2fca2bd9d..418096c58 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -775,7 +775,7 @@ class Element(Plugin):
     #    (Workspace|None): A workspace associated with this element
     #
     def _get_workspace(self):
-        return self._get_project()._workspaces.get_workspace(self)
+        return self._get_project()._workspaces.get_workspace(self.name)
 
     # _get_artifact_metadata():
     #