Make workspaces element-wide instead of source-specific

At present, BuildStream supports source-specific workspaces. This patch
makes workspaces element-wide. This will bring workspaces closer to what
the build area looks like inside the sandbox when the build actually
happens.

As part of this change, format of `.bst/workspaces.yaml` file will also
change. Previously, each element used to have a dict mapping each source
to its workspace directory. Now, each element will directly map to its
workspace directory.

If users have existing workspaces open, this patch tries to cope with it
but in some cases, it is not possible to reliably convert workspace
config from old format to new format. When an element has workspace open
for just one source, we assume that to be the workspace directory. But
if there were more than one workspaces associated with an element,
BuildStream will simply error out describing the issue.

Note that the actual contents of the workspace directory are not changed
in any case. So, if an element had multiple sources associated with it
but only had workspace open for source, the other sources will not be
automatically staged in the workspace unless the user runs `workspace
reset` or something equivalent.

Part of #209.

4 files changed, 91 insertions, 85 deletions

diff --git a/buildstream/_frontend/main.py b/buildstream/_frontend/main.py
index 7997b45ee..fc5372048 100644
--- a/buildstream/_frontend/main.py
+++ b/buildstream/_frontend/main.py
@@ -617,20 +617,18 @@ def workspace():
               help="Do not checkout the source, only link to the given directory")
 @click.option('--force', '-f', default=False, is_flag=True,
               help="Overwrite files existing in checkout directory")
-@click.option('--source', '-s', default=None, type=click.INT, metavar='INDEX',
-              help="The source to create a workspace for. Projects with one source may omit this")
 @click.option('--track', default=False, is_flag=True,
               help="Track and fetch new source references before checking out the workspace")
 @click.argument('element',
                 type=click.Path(dir_okay=False, readable=True))
 @click.argument('directory', type=click.Path(file_okay=False))
 @click.pass_obj
-def workspace_open(app, no_checkout, force, source, track, element, directory):
+def workspace_open(app, no_checkout, force, track, element, directory):
     """Open a workspace for manual source modification"""
 
     app.initialize((element,), rewritable=track, track_elements=[element] if track else None)
     try:
-        app.pipeline.open_workspace(app.scheduler, directory, source, no_checkout, track, force)
+        app.pipeline.open_workspace(app.scheduler, directory, no_checkout, track, force)
         click.echo("", err=True)
     except BstError as e:
         click.echo("", err=True)
@@ -642,14 +640,12 @@ def workspace_open(app, no_checkout, force, source, track, element, directory):
 #                     Workspace Close Command                    #
 ##################################################################
 @workspace.command(name='close', short_help="Close a workspace")
-@click.option('--source', '-s', default=None, type=click.INT, metavar='INDEX',
-              help="The source of the workspace to remove. Projects with one source may omit this")
 @click.option('--remove-dir', default=False, is_flag=True,
               help="Remove the path that contains the closed workspace")
 @click.argument('element',
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
-def workspace_close(app, source, remove_dir, element):
+def workspace_close(app, remove_dir, element):
     """Close a workspace"""
 
     app.initialize((element,))
@@ -659,7 +655,7 @@ def workspace_close(app, source, remove_dir, element):
             sys.exit(-1)
 
     try:
-        app.pipeline.close_workspace(source, remove_dir)
+        app.pipeline.close_workspace(remove_dir)
         click.echo("", err=True)
     except BstError as e:
         click.echo("", err=True)
@@ -671,8 +667,6 @@ def workspace_close(app, source, remove_dir, element):
 #                     Workspace Reset Command                    #
 ##################################################################
 @workspace.command(name='reset', short_help="Reset a workspace to its original state")
-@click.option('--source', '-s', default=None, type=click.INT, metavar='INDEX',
-              help="The source of the workspace to reset. Projects with one source may omit this")
 @click.option('--track', default=False, is_flag=True,
               help="Track and fetch the latest source before resetting")
 @click.option('--no-checkout', default=False, is_flag=True,
@@ -680,7 +674,7 @@ def workspace_close(app, source, remove_dir, element):
 @click.argument('element',
                 type=click.Path(dir_okay=False, readable=True))
 @click.pass_obj
-def workspace_reset(app, source, track, no_checkout, element):
+def workspace_reset(app, track, no_checkout, element):
     """Reset a workspace to its original state"""
     app.initialize((element,))
     if app.interactive:
@@ -689,7 +683,7 @@ def workspace_reset(app, source, track, no_checkout, element):
             sys.exit(-1)
 
     try:
-        app.pipeline.reset_workspace(app.scheduler, source, track, no_checkout)
+        app.pipeline.reset_workspace(app.scheduler, track, no_checkout)
         click.echo("", err=True)
     except BstError as e:
         click.echo("", err=True)
@@ -722,13 +716,11 @@ def workspace_list(app):
         sys.exit(-1)
 
     workspaces = []
-    for element_name, source_index, directory in project._list_workspaces():
+    for element_name, directory in project._list_workspaces():
         workspace = {
             'element': element_name,
             'directory': directory,
         }
-        if source_index > 0:
-            workspace['index'] = source_index
 
         workspaces.append(workspace)
 
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 5894bdaed..b516d9f44 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -179,15 +179,15 @@ class Pipeline():
                 raise PipelineError("{}: {}".format(plugin, e), reason=e.reason) from e
 
     def initialize_workspaces(self):
-        for element_name, source, workspace in self.project._list_workspaces():
+        for element_name, workspace in self.project._list_workspaces():
             for target in self.targets:
                 element = target.search(Scope.ALL, element_name)
 
                 if element is None:
-                    self.unused_workspaces.append((element_name, source, workspace))
+                    self.unused_workspaces.append((element_name, workspace))
                     continue
 
-                self.project._set_workspace(element, source, workspace)
+                self.project._set_workspace(element, workspace)
 
     def initialize_remote_caches(self, artifact_cache_specs):
         def remote_failed(url, error):
@@ -422,7 +422,7 @@ class Pipeline():
     def build(self, scheduler, build_all, track_first, save):
         if len(self.unused_workspaces) > 0:
             self.message(MessageType.WARN, "Unused workspaces",
-                         detail="\n".join([el + "-" + str(src) for el, src, _
+                         detail="\n".join([el for el, _
                                            in self.unused_workspaces]))
 
         # We set up two plans; one to track elements, the other to
@@ -558,17 +558,17 @@ class Pipeline():
     #
     # Args:
     #    directory (str): The directory to stage the source in
-    #    source_index (int): The index of the source to stage
     #    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, source_index, no_checkout, track_first, force):
+    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)
-        sources = list(target.sources())
-        source_index = self.validate_workspace_index(source_index)
+
+        if len(list(target.sources())) == 0:
+            raise PipelineError("The given element has no sources")
 
         # Check directory
         try:
@@ -580,9 +580,9 @@ class Pipeline():
             raise PipelineError("Checkout directory is not empty: {}".format(directory))
 
         # Check for workspace config
-        if self.project._get_workspace(target.name, source_index):
+        if self.project._get_workspace(target.name):
             raise PipelineError("Workspace '{}' is already defined."
-                                .format(target.name + " - " + str(source_index)))
+                                .format(target.name))
 
         plan = [target]
 
@@ -616,16 +616,16 @@ class Pipeline():
                              "Fetched {} elements".format(fetched), elapsed=elapsed)
 
         if not no_checkout:
-            source = sources[source_index]
-            with target.timed_activity("Staging source to {}".format(directory)):
-                if source.get_consistency() != Consistency.CACHED:
-                    raise PipelineError("Could not stage uncached source. " +
-                                        "Use `--track` to track and " +
-                                        "fetch the latest version of the " +
-                                        "source.")
-                source._init_workspace(directory)
+            with target.timed_activity("Staging sources to {}".format(directory)):
+                for source in target.sources():
+                    if source.get_consistency() != Consistency.CACHED:
+                        raise PipelineError("Could not stage uncached source. " +
+                                            "Use `--track` to track and " +
+                                            "fetch the latest version of the " +
+                                            "source.")
+                    source._init_workspace(directory)
 
-        self.project._set_workspace(target, source_index, workdir)
+        self.project._set_workspace(target, workdir)
 
         with target.timed_activity("Saving workspace configuration"):
             self.project._save_workspace_config()
@@ -635,17 +635,15 @@ class Pipeline():
     # Close a project workspace
     #
     # Args:
-    #    source_index (int) - The index of the source
     #    remove_dir (bool) - Whether to remove the associated directory
     #
-    def close_workspace(self, source_index, remove_dir):
+    def close_workspace(self, remove_dir):
         # When working on workspaces we only have one target
         target = self.targets[0]
-        source_index = self.validate_workspace_index(source_index)
 
         # Remove workspace directory if prompted
         if remove_dir:
-            path = self.project._get_workspace(target.name, source_index)
+            path = self.project._get_workspace(target.name)
             if path is not None:
                 with target.timed_activity("Removing workspace directory {}"
                                            .format(path)):
@@ -658,17 +656,17 @@ class Pipeline():
         # Delete the workspace config entry
         with target.timed_activity("Removing workspace"):
             try:
-                self.project._delete_workspace(target.name, source_index)
+                self.project._delete_workspace(target.name)
             except KeyError:
                 raise PipelineError("Workspace '{}' is currently not defined"
-                                    .format(target.name + " - " + str(source_index)))
+                                    .format(target.name))
 
         # Update workspace config
         self.project._save_workspace_config()
 
         # Reset source to avoid checking out the (now empty) workspace
-        source = list(target.sources())[source_index]
-        source._del_workspace()
+        for source in target.sources():
+            source._del_workspace()
 
     # reset_workspace
     #
@@ -681,20 +679,18 @@ class Pipeline():
     #    track (bool): Whether to also track the source
     #    no_checkout (bool): Whether to check out the source (at all)
     #
-    def reset_workspace(self, scheduler, source_index, track, no_checkout):
+    def reset_workspace(self, scheduler, track, no_checkout):
         # When working on workspaces we only have one target
         target = self.targets[0]
-        source_index = self.validate_workspace_index(source_index)
-        workspace_dir = self.project._get_workspace(target.name, source_index)
+        workspace_dir = self.project._get_workspace(target.name)
 
         if workspace_dir is None:
             raise PipelineError("Workspace '{}' is currently not defined"
                                 .format(target.name + " - " + str(source_index)))
 
-        self.close_workspace(source_index, True)
+        self.close_workspace(True)
 
-        self.open_workspace(scheduler, workspace_dir, source_index, no_checkout,
-                            track, False)
+        self.open_workspace(scheduler, workspace_dir, no_checkout, track, False)
 
     # pull()
     #
@@ -831,19 +827,6 @@ class Pipeline():
         # in before.
         return [element for element in elements if element in visited]
 
-    def validate_workspace_index(self, source_index):
-        sources = list(self.targets[0].sources())
-
-        # Validate source_index
-        if len(sources) < 1:
-            raise PipelineError("The given element has no sources")
-        if len(sources) == 1 and source_index is None:
-            source_index = 0
-        if source_index is None:
-            raise PipelineError("An index needs to be specified for elements with more than one source")
-
-        return source_index
-
     # Various commands define a --deps option to specify what elements to
     # use in the result, this function reports a list that is appropriate for
     # the selected option.
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 9468ec460..1d109b11d 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -179,6 +179,7 @@ class Project():
 
         # Workspace configurations
         self._workspaces = self._load_workspace_config()
+        self._ensure_workspace_config_format()
 
         # Assert project version
         format_version = _yaml.node_get(config, int, 'format-version', default_value=0)
@@ -293,11 +294,10 @@ class Project():
     # Generator function to enumerate workspaces.
     #
     # Yields:
-    #    A tuple in the following format: (element, source, path).
+    #    A tuple in the following format: (element, path).
     def _list_workspaces(self):
         for element, _ in _yaml.node_items(self._workspaces):
-            for source, _ in _yaml.node_items(self._workspaces[element]):
-                yield (element, int(source), self._workspaces[element][source])
+            yield (element, self._workspaces[element])
 
     # _get_workspace()
     #
@@ -306,15 +306,14 @@ class Project():
     #
     # Args:
     #    element (str) - The element name
-    #    index (int) - The source index
     #
     # Returns:
     #    None if no workspace is open, the path to the workspace
     #    otherwise
     #
-    def _get_workspace(self, element, index):
+    def _get_workspace(self, element):
         try:
-            return self._workspaces[element][index]
+            return self._workspaces[element]
         except KeyError:
             return None
 
@@ -325,15 +324,14 @@ class Project():
     #
     # Args:
     #    element (str) - The element name
-    #    index (int) - The source index
     #    path (str) - The path to set the workspace to
     #
-    def _set_workspace(self, element, index, path):
+    def _set_workspace(self, element, path):
         if element.name not in self._workspaces:
             self._workspaces[element.name] = {}
 
-        self._workspaces[element.name][index] = path
-        element._set_source_workspace(index, path)
+        self._workspaces[element.name] = path
+        element._set_source_workspaces(path)
 
     # _delete_workspace()
     #
@@ -343,14 +341,9 @@ class Project():
     #
     # Args:
     #    element (str) - The element name
-    #    index (int) - The source index
     #
-    def _delete_workspace(self, element, index):
-        del self._workspaces[element][index]
-
-        # Contains a provenance object
-        if len(self._workspaces[element]) == 1:
-            del self._workspaces[element]
+    def _delete_workspace(self, element):
+        del self._workspaces[element]
 
     # _load_workspace_config()
     #
@@ -359,13 +352,11 @@ class Project():
     #
     # Returns:
     #
-    #    A node containing a dict that assigns projects to their
+    #    A node containing a dict that assigns elements to their
     #    workspaces. For example:
     #
-    #        amhello.bst: {
-    #            0: /home/me/automake,
-    #            1: /home/me/amhello
-    #        }
+    #        alpha.bst: /home/me/alpha
+    #        bravo.bst: /home/me/bravo
     #
     def _load_workspace_config(self):
         os.makedirs(os.path.join(self.directory, ".bst"), exist_ok=True)
@@ -378,6 +369,45 @@ class Project():
 
         return _yaml.load(workspace_file)
 
+    # _ensure_workspace_config_format()
+    #
+    # If workspace config is in old-style format, i.e. it is using
+    # source-specific workspaces, try to convert it to element-specific
+    # workspaces.
+    #
+    # This method will rewrite workspace config, if it is in old format.
+    #
+    # Args:
+    #    workspaces (dict): current workspace config, usually output of _load_workspace_config()
+    #
+    # Raises: LoadError if there was a problem with the workspace config
+    #
+    def _ensure_workspace_config_format(self):
+        needs_rewrite = False
+        for element, config in _yaml.node_items(self._workspaces):
+            if isinstance(config, str):
+                pass
+
+            elif isinstance(config, dict):
+                sources = list(_yaml.node_items(config))
+                if len(sources) > 1:
+                    detail = "There are multiple workspaces open for '{}'.\n" + \
+                             "This is not supported anymore.\n" + \
+                             "Please remove this element from '{}'."
+                    raise LoadError(LoadErrorReason.INVALID_DATA,
+                                    detail.format(element,
+                                                  os.path.join(self.directory, ".bst", "workspaces.yml")))
+
+                self._workspaces[element] = sources[0][1]
+                needs_rewrite = True
+
+            else:
+                raise LoadError(LoadErrorReason.INVALID_DATA,
+                                "Workspace config is in unexpected format.")
+
+        if needs_rewrite:
+            self._save_workspace_config()
+
     # _save_workspace_config()
     #
     # Dump the current workspace element to the project configuration
diff --git a/buildstream/element.py b/buildstream/element.py
index 8ae45d3bb..afcfcee95 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -1214,8 +1214,9 @@ class Element(Plugin):
 
     # Set a source's workspace
     #
-    def _set_source_workspace(self, source_index, path):
-        self.__sources[source_index]._set_workspace(path)
+    def _set_source_workspaces(self, path):
+        for source in self.sources():
+            source._set_workspace(path)
 
     # Whether this element has a source that is workspaced.
     #