[Notes] [Git][BuildStream/buildstream][master] 4 commits:

Tristan Van Berkom pushed to branch master at BuildStream / buildstream

Commits:

e61e6e50

by Tristan Van Berkom at 2018-12-07T07:57:13Z

_yaml.py: Added `allow_none` parameter to _yaml.node_get()

This allows specifying whether or not the code believes a None
value is acceptable if explicitly expressed to be None in the YAML.

A previous patch 3ba544b80f9f268be8ffe62fc8589b30212ec4a2 changed
the behavior to accept None for all values across the board, along
with allowing explicitly setting the `default_value` to `None` which
caused the code to be much more readable (this was the main motivation
of the patch, but it had the side effect of allowing None for everything).

In the majority of cases we load YAML however, it either has a value or
it is not specified in the YAML, and None is hardly ever acceptable
to be explicitly specified (it may be in the case you want to override
something with nothing in the YAML, but it is the edge case).

This had the side effect of causing unexpected crashes where the
YAML specifies an empty dictionary for instance.

Instead of forcing the caller to handle a possible None value, give
the choice to the caller if they really want to accept a None value,
and raise the LoadError otherwise.

This fixes issue #803, which is a crash due to receival of an
unexpected None value through _yaml.node_get()

504ecb3e

by Tristan Van Berkom at 2018-12-07T07:57:56Z

plugin.py: Added new `allow_none` parameter to Plugin.node_get_member(), defaulting to False.

Allow plugins to conveniently leverage the new `allow_none` parameter,
and have Plugin.node_get_member() raise a LoadError in the usual
case where None is not an acceptable value to be explicitly set in
the user provided YAML.

642ae4e8

by Tristan Van Berkom at 2018-12-07T07:57:56Z

tests/format/project.py: Added new regression test for empty dependency dictionaries

This adds a regression test for issue #803

2a0676c3

by Tristan Van Berkom at 2018-12-07T08:33:14Z

Merge branch 'tristan/yaml-optionally-allow-none' into 'master'

Only optionally allow None values in user provided YAML

Closes #803

See merge request BuildStream/buildstream!999

5 changed files:

buildstream/_yaml.py
buildstream/plugin.py
tests/format/project.py
+ tests/format/project/empty-depends/manual.bst
+ tests/format/project/empty-depends/project.conf

Changes:

buildstream/_yaml.py

@@ -352,6 +352,7 @@ _sentinel = object()
  #    key (str): The key to get a value for in node
  #    indices (list of ints): Optionally decend into lists of lists
  #    default_value: Optionally return this value if the key is not found
 +#    allow_none: (bool): Allow None to be a valid value
+ #
  # Returns:
  #    The value if found in node, otherwise default_value is returned
@@ -362,7 +363,7 @@ _sentinel = object()
  # Note:
  #    Returned strings are stripped of leading and trailing whitespace
+ #
 -def node_get(node, expected_type, key, indices=None, default_value=_sentinel):
 +def node_get(node, expected_type, key, indices=None, *, default_value=_sentinel, allow_none=False):
      value = node.get(key, default_value)
      provenance = node_get_provenance(node)
      if value is _sentinel:
@@ -377,8 +378,8 @@ def node_get(node, expected_type, key, indices=None, default_value=_sentinel):
              value = value[index]
              path += '[{:d}]'.format(index)
 -    # We want to allow None as a valid value for any type
 -    if value is None:
 +    # Optionally allow None as a valid value for any type
 +    if value is None and (allow_none or default_value is None):
          return None
      if not isinstance(value, expected_type):

buildstream/plugin.py

@@ -323,7 +323,7 @@ class Plugin():
          provenance = _yaml.node_get_provenance(node, key=member_name)
          return str(provenance)
 -    def node_get_member(self, node, expected_type, member_name, default=_yaml._sentinel):
 +    def node_get_member(self, node, expected_type, member_name, default=_yaml._sentinel, *, allow_none=False):
          """Fetch the value of a node member, raising an error if the value is
          missing or incorrectly typed.
@@ -332,6 +332,7 @@ class Plugin():
             expected_type (type): The expected type of the node member
             member_name (str): The name of the member to fetch
             default (expected_type): A value to return when *member_name* is not specified in *node*
 +           allow_none (bool): Allow explicitly set None values in the YAML (*Since: 1.4*)
          Returns:
             The value of *member_name* in *node*, otherwise *default*
@@ -352,7 +353,7 @@ class Plugin():
            # Fetch an optional integer
            level = self.node_get_member(node, int, 'level', -1)
          """
 -        return _yaml.node_get(node, expected_type, member_name, default_value=default)
 +        return _yaml.node_get(node, expected_type, member_name, default_value=default, allow_none=allow_none)
      def node_get_project_path(self, node, key, *,
                                check_is_file=False, check_is_dir=False):

tests/format/project.py

@@ -200,3 +200,10 @@ def test_element_path_project_path_contains_symlinks(cli, datafiles, tmpdir):
          f.write("kind: manual\n")
      result = cli.run(project=linked_project, args=['show', 'element.bst'])
      result.assert_success()
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR))
 +def test_empty_depends(cli, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename, "empty-depends")
 +    result = cli.run(project=project, args=['show', 'manual.bst'])
 +    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)

tests/format/project/empty-depends/manual.bst

 +kind: manual
++
 +depends:

tests/format/project/empty-depends/project.conf

+name: test

[Notes] [Git][BuildStream/buildstream][master] 4 commits: _yaml.py: Added `allow_none` parameter to _yaml.node_get()

Tristan Van Berkom pushed to branch master at BuildStream / buildstream

Commits:

5 changed files:

Changes: