bst-fmt: Allow greater control over node order
Allow authors of plugins to utilise the Plugin.keyorder attribute to
define an order for the sub-headers in config. Modify `bst fmt` to
modify the yaml into that order. Add canonical keyorder for every plugin
already in core.
In terms of implementation, this adds a custom yaml dumper subclassed
from ruamel.yaml's RoundTripDumper, which will dump a dict in the order
defined in the keyorder attribute. The Plugin class is given a keyorder
attribute, which defines the order in which keys should be dumped. The
element and source classes add the common fields used in them, so as to
minimise work for plugin authors. Plugin authors should add their custom
keys at configure time.
This causes some stripping of comments, which seems to be due to a bug
in the ruamel.yaml RoundTripDumper, as that also strips the same
comments. It also will remove blank spaces, again probably due to
limitations in ruamel.
diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index e9eecd9..c8df181 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -354,6 +354,7 @@
track_cross_junctions=track_cross_junctions,
build_all=all_)
+
##################################################################
# Format Command #
##################################################################
diff --git a/buildstream/_yaml.py b/buildstream/_yaml.py
index 8d7302b..2d659c7 100644
--- a/buildstream/_yaml.py
+++ b/buildstream/_yaml.py
@@ -51,6 +51,31 @@
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 @@
# 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()
diff --git a/buildstream/buildelement.py b/buildstream/buildelement.py
index 6ef060f..0b83ea0 100644
--- a/buildstream/buildelement.py
+++ b/buildstream/buildelement.py
@@ -172,6 +172,7 @@
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] = []
diff --git a/buildstream/element.py b/buildstream/element.py
index 9a9e015..a8ee747 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -269,6 +269,9 @@
# 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
@@ -1338,7 +1341,17 @@
#
def _format(self):
provenance = self._get_provenance()
- _yaml.dump(provenance.toplevel, provenance.filename.name)
+
+ _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():
#
diff --git a/buildstream/plugin.py b/buildstream/plugin.py
index 2f51c88..181a319 100644
--- a/buildstream/plugin.py
+++ b/buildstream/plugin.py
@@ -188,6 +188,8 @@
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.
diff --git a/buildstream/plugins/elements/compose.py b/buildstream/plugins/elements/compose.py
index d61a324..ed4da4d 100644
--- a/buildstream/plugins/elements/compose.py
+++ b/buildstream/plugins/elements/compose.py
@@ -70,6 +70,8 @@
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
diff --git a/buildstream/plugins/elements/filter.py b/buildstream/plugins/elements/filter.py
index 6723253..9f86baa 100644
--- a/buildstream/plugins/elements/filter.py
+++ b/buildstream/plugins/elements/filter.py
@@ -65,6 +65,8 @@
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))
diff --git a/buildstream/plugins/elements/junction.py b/buildstream/plugins/elements/junction.py
index 7f98173..2a5b17f 100644
--- a/buildstream/plugins/elements/junction.py
+++ b/buildstream/plugins/elements/junction.py
@@ -140,6 +140,7 @@
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
diff --git a/buildstream/plugins/elements/script.py b/buildstream/plugins/elements/script.py
index 4e422c5..b695fad 100644
--- a/buildstream/plugins/elements/script.py
+++ b/buildstream/plugins/elements/script.py
@@ -51,6 +51,7 @@
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)
diff --git a/buildstream/plugins/sources/_downloadablefilesource.py b/buildstream/plugins/sources/_downloadablefilesource.py
index f5c5b3d..4cb2d50 100644
--- a/buildstream/plugins/sources/_downloadablefilesource.py
+++ b/buildstream/plugins/sources/_downloadablefilesource.py
@@ -82,6 +82,8 @@
self.url = self.translate_url(self.original_url)
self._warn_deprecated_etag(node)
+ self.keyorder += ['url', 'ref', 'etag']
+
def preflight(self):
return
diff --git a/buildstream/plugins/sources/bzr.py b/buildstream/plugins/sources/bzr.py
index f524729..544bf2d 100644
--- a/buildstream/plugins/sources/bzr.py
+++ b/buildstream/plugins/sources/bzr.py
@@ -68,6 +68,8 @@
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)
diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py
index 74d632b..e93517c 100644
--- a/buildstream/plugins/sources/git.py
+++ b/buildstream/plugins/sources/git.py
@@ -506,6 +506,8 @@
'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'])
diff --git a/buildstream/plugins/sources/local.py b/buildstream/plugins/sources/local.py
index 55cdc14..c6b6963 100644
--- a/buildstream/plugins/sources/local.py
+++ b/buildstream/plugins/sources/local.py
@@ -53,6 +53,7 @@
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)
diff --git a/buildstream/plugins/sources/ostree.py b/buildstream/plugins/sources/ostree.py
index 770cfd5..1bc4ef6 100644
--- a/buildstream/plugins/sources/ostree.py
+++ b/buildstream/plugins/sources/ostree.py
@@ -65,6 +65,7 @@
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)
diff --git a/buildstream/plugins/sources/patch.py b/buildstream/plugins/sources/patch.py
index 8e833b4..48aab13 100644
--- a/buildstream/plugins/sources/patch.py
+++ b/buildstream/plugins/sources/patch.py
@@ -53,6 +53,7 @@
# 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)
diff --git a/buildstream/plugins/sources/pip.py b/buildstream/plugins/sources/pip.py
index abef1fd..04e8bab 100644
--- a/buildstream/plugins/sources/pip.py
+++ b/buildstream/plugins/sources/pip.py
@@ -111,6 +111,7 @@
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)
diff --git a/buildstream/plugins/sources/remote.py b/buildstream/plugins/sources/remote.py
index a6b02fd..9247c12 100644
--- a/buildstream/plugins/sources/remote.py
+++ b/buildstream/plugins/sources/remote.py
@@ -61,6 +61,7 @@
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)
diff --git a/buildstream/plugins/sources/tar.py b/buildstream/plugins/sources/tar.py
index 195c059..a830df8 100644
--- a/buildstream/plugins/sources/tar.py
+++ b/buildstream/plugins/sources/tar.py
@@ -75,6 +75,7 @@
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
diff --git a/buildstream/plugins/sources/zip.py b/buildstream/plugins/sources/zip.py
index f5fac3a..27e5fd9 100644
--- a/buildstream/plugins/sources/zip.py
+++ b/buildstream/plugins/sources/zip.py
@@ -75,6 +75,7 @@
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]
diff --git a/buildstream/source.py b/buildstream/source.py
index bb54110..bc4bae9 100644
--- a/buildstream/source.py
+++ b/buildstream/source.py
@@ -302,6 +302,8 @@
# 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)
