_yaml: remove node_sanitizebschubert/node-api-nosanitize

Some call places do not need calls to 'node_sanitize' anymore, therefore
removing the call entirely.

Other still use it for convenience, but that doesn't seem the right way
to do it for consistency. Those places have been replaced by calls to
'Node.strip_node_info()'.

7 files changed, 20 insertions, 71 deletions

diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py
index dce63e14f..e2ab3c5a5 100644
--- a/src/buildstream/_frontend/widget.py
+++ b/src/buildstream/_frontend/widget.py
@@ -28,7 +28,6 @@ import click
 
 from .profile import Profile
 from .. import Element, Consistency, Scope
-from .. import _yaml
 from .. import __version__ as bst_version
 from .._exceptions import ImplError
 from .._message import MessageType
@@ -393,28 +392,27 @@ class LogLine(Widget):
 
             # Element configuration
             if "%{config" in format_:
-                config = _yaml.node_sanitize(element._Element__config)
                 line = p.fmt_subst(
                     line, 'config',
-                    yaml.round_trip_dump(config, default_flow_style=False, allow_unicode=True))
+                    yaml.round_trip_dump(element._Element__config, default_flow_style=False, allow_unicode=True))
 
             # Variables
             if "%{vars" in format_:
-                variables = _yaml.node_sanitize(element._Element__variables.flat)
+                variables = element._Element__variables.flat
                 line = p.fmt_subst(
                     line, 'vars',
                     yaml.round_trip_dump(variables, default_flow_style=False, allow_unicode=True))
 
             # Environment
             if "%{env" in format_:
-                environment = _yaml.node_sanitize(element._Element__environment)
+                environment = element._Element__environment
                 line = p.fmt_subst(
                     line, 'env',
                     yaml.round_trip_dump(environment, default_flow_style=False, allow_unicode=True))
 
             # Public
             if "%{public" in format_:
-                environment = _yaml.node_sanitize(element._Element__public)
+                environment = element._Element__public
                 line = p.fmt_subst(
                     line, 'public',
                     yaml.round_trip_dump(environment, default_flow_style=False, allow_unicode=True))
diff --git a/src/buildstream/_workspaces.py b/src/buildstream/_workspaces.py
index ed50828a3..d3a28b879 100644
--- a/src/buildstream/_workspaces.py
+++ b/src/buildstream/_workspaces.py
@@ -114,7 +114,8 @@ class WorkspaceProject():
     def load(cls, directory):
         workspace_file = os.path.join(directory, WORKSPACE_PROJECT_FILE)
         if os.path.exists(workspace_file):
-            data_dict = _yaml.node_sanitize(_yaml.roundtrip_load(workspace_file), dict_type=dict)
+            data_dict = _yaml.roundtrip_load(workspace_file)
+
             return cls.from_dict(directory, data_dict)
         else:
             return None
@@ -627,13 +628,15 @@ class Workspaces():
     #    (Workspace): A newly instantiated Workspace
     #
     def _load_workspace(self, node):
+        running_files = node.get_mapping('running_files', default=None)
+        if running_files:
+            running_files = running_files.strip_node_info()
+
         dictionary = {
             'prepared': node.get_bool('prepared', default=False),
             'path': node.get_str('path'),
             'last_successful': node.get_str('last_successful', default=None),
-            'running_files': _yaml.node_sanitize(
-                node.get_mapping('running_files', default=None),
-                dict_type=dict),
+            'running_files': running_files,
         }
         return Workspace.from_dict(self._toplevel_project, dictionary)
 
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index f169e1af6..99e3160b2 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -1215,60 +1215,6 @@ def composite_and_move(MappingNode target, MappingNode source):
         del target.value[key]
 
 
-# Types we can short-circuit in node_sanitize for speed.
-__SANITIZE_SHORT_CIRCUIT_TYPES = (int, float, str, bool)
-
-
-# node_sanitize()
-#
-# Returns an alphabetically ordered recursive copy
-# of the source node with internal provenance information stripped.
-#
-# Only dicts are ordered, list elements are left in order.
-#
-cpdef object node_sanitize(object node, object dict_type=OrderedDict):
-    node_type = type(node)
-
-    # If we have an unwrappable node, unwrap it
-    # FIXME: we should only ever have Nodes here
-    if node_type in [MappingNode, SequenceNode]:
-        node = node.value
-        node_type = type(node)
-
-    if node_type is ScalarNode:
-        return node.value
-
-    # Short-circuit None which occurs ca. twice per element
-    if node is None:
-        return node
-
-    # Next short-circuit integers, floats, strings, booleans, and tuples
-    if node_type in __SANITIZE_SHORT_CIRCUIT_TYPES:
-        return node
-
-    # Now short-circuit lists.
-    elif node_type is list:
-        return [node_sanitize(elt, dict_type=dict_type) for elt in node]
-
-    # Finally dict, and other Mappings need special handling
-    elif node_type is dict:
-        result = dict_type()
-
-        key_list = [key for key, _ in node.items()]
-        for key in sorted(key_list):
-            result[key] = node_sanitize(node[key], dict_type=dict_type)
-
-        return result
-
-    # Sometimes we're handed tuples and we can't be sure what they contain
-    # so we have to sanitize into them
-    elif node_type is tuple:
-        return tuple([node_sanitize(v, dict_type=dict_type) for v in node])
-
-    # Everything else just gets returned as-is.
-    return node
-
-
 # node_validate()
 #
 # Validate the node so as to ensure the user has not specified
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 581fe725f..c2e4b9105 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -883,7 +883,7 @@ class Element(Plugin):
            (dict): A dictionary of string key/values suitable for passing
            to :func:`Sandbox.run() <buildstream.sandbox.Sandbox.run>`
         """
-        return _yaml.node_sanitize(self.__environment)
+        return self.__environment
 
     def get_variable(self, varname):
         """Fetch the value of a variable resolved for this element.
diff --git a/src/buildstream/sandbox/_sandboxremote.py b/src/buildstream/sandbox/_sandboxremote.py
index 38dfe3b09..a544331de 100644
--- a/src/buildstream/sandbox/_sandboxremote.py
+++ b/src/buildstream/sandbox/_sandboxremote.py
@@ -175,7 +175,8 @@ class SandboxRemote(Sandbox):
                 if tls_key in config:
                     config[tls_key] = resolve_path(config.get_str(tls_key))
 
-        return RemoteExecutionSpec(*[_yaml.node_sanitize(conf) for conf in service_configs])
+        # TODO: we should probably not be stripping node info and rather load files the safe way
+        return RemoteExecutionSpec(*[conf.strip_node_info() for conf in service_configs])
 
     def run_remote_command(self, channel, action_digest):
         # Sends an execution request to the remote execution server.
diff --git a/src/buildstream/source.py b/src/buildstream/source.py
index c87d77020..c09e318d2 100644
--- a/src/buildstream/source.py
+++ b/src/buildstream/source.py
@@ -893,8 +893,9 @@ class Source(Plugin):
         #
         # Step 2 - Set the ref in memory, and determine changed state
         #
-        clean = _yaml.node_sanitize(node, dict_type=dict)
-        to_modify = _yaml.node_sanitize(node, dict_type=dict)
+        # TODO: we are working on dictionaries here, would be nicer to just work on the nodes themselves
+        clean = node.strip_node_info()
+        to_modify = node.strip_node_info()
 
         current_ref = self.get_ref()  # pylint: disable=assignment-from-no-return
 
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index 4951a74a4..9f8398d14 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -908,7 +908,7 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
     def parse_dict_as_yaml(node):
         tempfile = os.path.join(str(tmpdir), 'yaml_dump')
         _yaml.roundtrip_dump(node, tempfile)
-        return _yaml.node_sanitize(_yaml.load(tempfile))
+        return _yaml.load(tempfile).strip_node_info()
 
     project = str(datafiles)
     os.makedirs(os.path.join(project, '.bst'))
@@ -920,7 +920,7 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
     result = cli.run(project=project, args=['workspace', 'list'])
     result.assert_success()
 
-    loaded_config = _yaml.node_sanitize(_yaml.load(workspace_config_path))
+    loaded_config = _yaml.load(workspace_config_path).strip_node_info()
 
     # Check that workspace config remains the same if no modifications
     # to workspaces were made
@@ -955,7 +955,7 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
     result.assert_success()
 
     # Check that workspace config is converted correctly if necessary
-    loaded_config = _yaml.node_sanitize(_yaml.load(workspace_config_path))
+    loaded_config = _yaml.load(workspace_config_path).strip_node_info()
     assert loaded_config == parse_dict_as_yaml(expected)