WIP: bst fmt: Add basic functionality

buildstream/_frontend/cli.py

@@ -355,6 +355,30 @@ def build(app, elements, all_, track_, track_save, track_all, track_except, trac
                          build_all=all_)
 
 
+##################################################################
+#                         Format Command                         #
+##################################################################
+@cli.command(short_help="Format element files")
+@click.option('--all', 'all_', default=False, is_flag=True,
+              help="Format all dependencies of the targets")
+@click.option('--except', 'except_', multiple=True,
+              type=click.Path(readable=False),
+              help="Except certain files from formatting (has no effect without `--all`)")
+@click.argument('elements', nargs=-1,
+                type=click.Path(readable=False))
+@click.pass_obj
+def fmt(app, elements, all_, except_):
+    """Formats element files into a consistent style. This style is the one
+    defaulted to by ruamel.yaml, so is the recommended format for a
+    BuildStream project.
+
+    By default this will format just the specified element, but you can format
+    all the elements at once by passing the `--all` flag.
+    """
+    with app.initialized(session_name="Format"):
+        app.stream.format(elements, except_targets=except_, format_all=all_)
+
+
 ##################################################################
 #                           Pull Command                         #
 ##################################################################

buildstream/_scheduler/__init__.py

@@ -24,6 +24,7 @@
 from .queues.buildqueue import BuildQueue
 from .queues.pushqueue import PushQueue
 from .queues.pullqueue import PullQueue
+from .queues.formatqueue import FormatQueue
 
 from .scheduler import Scheduler, SchedStatus
 from .jobs import ElementJob

buildstream/_scheduler/queues/formatqueue.py

@@ -0,0 +1,13 @@
+# Local imports
+from . import Queue
+from ..resources import ResourceType
+
+
+class FormatQueue(Queue):
+
+    action_name = "Format"
+    complete_name = "Formatted"
+    resources = [ResourceType.PROCESS]
+
+    def process(self, element):
+        element._format()

buildstream/_stream.py

@@ -30,7 +30,7 @@
 
 from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
 from ._message import Message, MessageType
-from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
+from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue, FormatQueue
 from ._pipeline import Pipeline, PipelineSelection
 from . import utils, _yaml, _site
 from . import Scope, Consistency
@@ -284,6 +284,34 @@ def track(self, targets, *,
         self._enqueue_plan(elements, queue=track_queue)
         self._run()
 
+    # format()
+    #
+    # Formats elements into a "canonical" format.
+    #
+    # Args:
+    #    targets (list of str): Targets to format
+    #    except_targets (list of str): Specifiedtargets to except from formatting
+    #    format_all (bool): Whether to format all elements or just the targets
+    #
+    def format(self, targets, *,
+               except_targets=None,
+               format_all=False):
+        if format_all:
+            modify_selection = PipelineSelection.ALL
+        else:
+            modify_selection = PipelineSelection.REDIRECT
+
+        # Only pass targets to track, in order to load a rewritable pipeline
+        _, elements = \
+            self._load([], targets,
+                       track_selection=modify_selection,
+                       track_except_targets=except_targets,
+                       fetch_subprojects=True)
+        fmt_queue = FormatQueue(self._scheduler)
+        self._add_queue(fmt_queue, track=False)
+        self._enqueue_plan(elements, queue=fmt_queue)
+        self._run()
+
     # pull()
     #
     # Pulls artifacts from remote artifact server(s)

buildstream/_yaml.py

@@ -51,6 +51,31 @@ def __init__(self, name, shortname, project):
         self.project = project
 
 
+# A custom yaml dumper to reorder keys in dicts to a canonical order, defined
+# in each plugin
+class BstFormatter(yaml.RoundTripDumper):
+
+    keyorder = []
+
+    @classmethod
+    def _iter_in_global_order(cls, mapping):
+        for key in cls.keyorder:
+            if key in mapping.keys():
+                yield key, mapping[key]
+        for key in sorted(mapping.keys()):
+            if key not in cls.keyorder:
+                yield key, mapping[key]
+
+    @classmethod
+    def _represent_dict(cls, dumper, mapping):
+        return dumper.represent_mapping('tag:yaml.org,2002:map', cls._iter_in_global_order(mapping))
+
+    def __init__(self, *args, **kwargs):
+        yaml.RoundTripDumper.__init__(self, *args, **kwargs)
+        self.no_newline = False
+        self.add_representer(dict, self._represent_dict)
+
+
 # Provenance tracks the origin of a given node in the parsed dictionary.
 #
 # Args:
@@ -244,15 +269,16 @@ def load_data(data, file=None, copy_tree=False):
 # Args:
 #    node (dict): A node previously loaded with _yaml.load() above
 #    filename (str): The YAML file to load
+#    dumper (yaml.Dumper): The yaml dumper to be used
 #
-def dump(node, filename=None):
+def dump(node, filename=None, dumper=yaml.RoundTripDumper):
     with ExitStack() as stack:
         if filename:
             from . import utils
             f = stack.enter_context(utils.save_file_atomic(filename, 'w'))
         else:
             f = sys.stdout
-        yaml.round_trip_dump(node, f)
+        yaml.round_trip_dump(node, f, Dumper=dumper)
 
 
 # node_decorated_copy()

buildstream/buildelement.py

@@ -172,6 +172,7 @@ def configure(self, node):
         for command_name in _legacy_command_steps:
             if command_name in _command_steps:
                 self.__commands[command_name] = self.__get_commands(node, command_name)
+                self.keyorder.append(command_name)
             else:
                 self.__commands[command_name] = []
 

buildstream/element.py

@@ -269,6 +269,9 @@ def __init__(self, context, project, meta, plugin_conf):
                 # This will taint the artifact, disable pushing.
                 self.__sandbox_config_supported = False
 
+        self.keyorder = ['kind', 'description', 'depends', 'variables',
+                         'environment', 'config', 'public', 'sandbox', 'sources']
+
     def __lt__(self, other):
         return self.name < other.name
 
@@ -1332,6 +1335,24 @@ def _track(self):
 
         return refs
 
+    # _format()
+    #
+    # Dumps an element's yaml file in canonical format
+    #
+    def _format(self):
+        provenance = self._get_provenance()
+
+        _yaml.BstFormatter.keyorder = self.keyorder
+
+        # We need to add the key orders for each source into the
+        # global keyorder, as sources are dumped with the element.
+        for s in self.sources():
+            for key in s.keyorder:
+                if key not in _yaml.BstFormatter.keyorder:
+                    _yaml.BstFormatter.keyorder += s.keyorder
+
+        _yaml.dump(provenance.toplevel, provenance.filename.name, dumper=_yaml.BstFormatter)
+
     # _prepare_sandbox():
     #
     # This stages things for either _shell() (below) or also

buildstream/plugin.py

@@ -188,6 +188,8 @@ def __init__(self, name, context, project, provenance, type_tag):
         self.__kind = modulename.split('.')[-1]
         self.debug("Created: {}".format(self))
 
+        self.keyorder = []
+
     def __del__(self):
         # Dont send anything through the Message() pipeline at destruction time,
         # any subsequent lookup of plugin by unique id would raise KeyError.

buildstream/plugins/elements/compose.py

@@ -70,6 +70,8 @@ def configure(self, node):
         self.exclude = self.node_get_member(node, list, 'exclude')
         self.include_orphans = self.node_get_member(node, bool, 'include-orphans')
 
+        self.keyorder += ['integrate', 'include', 'exclude', 'include-orphans']
+
     def preflight(self):
         pass
 

buildstream/plugins/elements/filter.py

@@ -65,6 +65,8 @@ def configure(self, node):
         self.exclude = self.node_get_member(node, list, 'exclude')
         self.include_orphans = self.node_get_member(node, bool, 'include-orphans')
 
+        self.keyorder += ['include', 'exclude', 'include-orphans']
+
     def preflight(self):
         # Exactly one build-depend is permitted
         build_deps = list(self.dependencies(Scope.BUILD, recurse=False))

buildstream/plugins/elements/junction.py

@@ -140,6 +140,7 @@ class JunctionElement(Element):
     def configure(self, node):
         self.path = self.node_get_member(node, str, 'path', default='')
         self.options = self.node_get_member(node, Mapping, 'options', default={})
+        self.keyorder += ['path', 'options']
 
     def preflight(self):
         pass

buildstream/plugins/elements/script.py

@@ -51,6 +51,7 @@ def configure(self, node):
         self.node_validate(node, [
             'commands', 'root-read-only', 'layout'
         ])
+        self.keyorder += ['layout', 'root-read-only', 'commands']
 
         cmds = self.node_subst_list(node, "commands")
         self.add_commands("commands", cmds)

buildstream/plugins/sources/_downloadablefilesource.py

@@ -82,6 +82,8 @@ def configure(self, node):
         self.url = self.translate_url(self.original_url)
         self._warn_deprecated_etag(node)
 
+        self.keyorder += ['url', 'ref', 'etag']
+
     def preflight(self):
         return
 

buildstream/plugins/sources/bzr.py

@@ -68,6 +68,8 @@ class BzrSource(Source):
     def configure(self, node):
         self.node_validate(node, ['url', 'track', 'ref'] + Source.COMMON_CONFIG_KEYS)
 
+        self.keyorder += ['url', 'track', 'ref']
+
         self.original_url = self.node_get_member(node, str, 'url')
         self.tracking = self.node_get_member(node, str, 'track')
         self.ref = self.node_get_member(node, str, 'ref', None)

buildstream/plugins/sources/git.py

@@ -506,6 +506,8 @@ def configure(self, node):
                        'track-tags', 'tags']
         self.node_validate(node, config_keys + Source.COMMON_CONFIG_KEYS)
 
+        self.keyorder += config_keys
+
         tags_node = self.node_get_member(node, list, 'tags', [])
         for tag_node in tags_node:
             self.node_validate(tag_node, ['tag', 'commit', 'annotated'])

buildstream/plugins/sources/local.py

@@ -53,6 +53,7 @@ def __init__(self, context, project, meta):
 
     def configure(self, node):
         self.node_validate(node, ['path'] + Source.COMMON_CONFIG_KEYS)
+        self.keyorder += ['path']
         self.path = self.node_get_project_path(node, 'path')
         self.fullpath = os.path.join(self.get_project_directory(), self.path)
 

buildstream/plugins/sources/ostree.py

@@ -65,6 +65,7 @@ class OSTreeSource(Source):
     def configure(self, node):
 
         self.node_validate(node, ['url', 'ref', 'track', 'gpg-key'] + Source.COMMON_CONFIG_KEYS)
+        self.keyorder += ['url', 'track', 'ref', 'gpg-key']
 
         self.original_url = self.node_get_member(node, str, 'url')
         self.url = self.translate_url(self.original_url)

buildstream/plugins/sources/patch.py

@@ -53,6 +53,7 @@ class PatchSource(Source):
     # pylint: disable=attribute-defined-outside-init
 
     def configure(self, node):
+        self.keyorder += ['strip-level', 'path']
         self.path = self.node_get_project_path(node, 'path',
                                                check_is_file=True)
         self.strip_level = self.node_get_member(node, int, "strip-level", 1)

buildstream/plugins/sources/pip.py

@@ -111,6 +111,7 @@ class PipSource(Source):
     def configure(self, node):
         self.node_validate(node, ['url', 'packages', 'ref', 'requirements-files'] +
                            Source.COMMON_CONFIG_KEYS)
+        self.keyorder += ['url', 'packages', 'ref', 'requirements-files']
         self.ref = self.node_get_member(node, str, 'ref', None)
         self.original_url = self.node_get_member(node, str, 'url', _PYPI_INDEX_URL)
         self.index_url = self.translate_url(self.original_url)

buildstream/plugins/sources/remote.py

@@ -61,6 +61,7 @@ class RemoteSource(DownloadableFileSource):
 
     def configure(self, node):
         super().configure(node)
+        self.keyorder += ['filename']
 
         self.filename = self.node_get_member(node, str, 'filename', os.path.basename(self.url))
         self.executable = self.node_get_member(node, bool, 'executable', False)

buildstream/plugins/sources/tar.py

@@ -75,6 +75,7 @@ def configure(self, node):
         self.base_dir = self.node_get_member(node, str, 'base-dir', '*') or None
 
         self.node_validate(node, DownloadableFileSource.COMMON_CONFIG_KEYS + ['base-dir'])
+        self.keyorder += ['base-dir']
 
     def preflight(self):
         self.host_lzip = None

buildstream/plugins/sources/zip.py

@@ -75,6 +75,7 @@ def configure(self, node):
         self.base_dir = self.node_get_member(node, str, 'base-dir', '*') or None
 
         self.node_validate(node, DownloadableFileSource.COMMON_CONFIG_KEYS + ['base-dir'])
+        self.keyorder += ['base-dir']
 
     def get_unique_key(self):
         return super().get_unique_key() + [self.base_dir]

buildstream/source.py

@@ -302,6 +302,8 @@ def __init__(self, context, project, meta, *, alias_override=None):
         # FIXME: Reconstruct a MetaSource from a Source instead of storing it.
         self.__meta = meta                              # MetaSource stored so we can copy this source later.
 
+        self.keyorder = ['directory'] + self.keyorder
+
         # Collect the composited element configuration and
         # ask the element to configure itself.
         self.__init_defaults(meta)

tests/completions/completions.py

@@ -9,6 +9,7 @@
     'artifact ',
     'build ',
     'checkout ',
+    'fmt ',
     'help ',
     'init ',
     'pull ',