From b68b6d14564584df0da60f41483021568bd91b91 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Mon, 10 Jun 2019 14:20:23 +0100 Subject: _yaml: Add 'as_bool()' and 'is_none()' to ScalarNode - 'as_bool()' casts a ScalarNode into a boolean, understanding both 'True' and 'False' as truthy-falsy values, as per node_get(type=bool) behavior - 'is_none()' allwos checking whether the scalar node contains a 'None' value. Since 'None' cannot be used when working with booleans, we need to have a way of checking for 'None' when we actually need the information of whether the value is unset. - Adapt all call places to use the new API --- src/buildstream/_cas/casremote.py | 2 +- src/buildstream/_context.py | 8 ++++---- src/buildstream/_gitsourcebase.py | 8 ++++---- src/buildstream/_options/optionbool.py | 4 ++-- src/buildstream/_project.py | 10 +++++----- src/buildstream/_variables.pyx | 2 +- src/buildstream/_workspaces.py | 2 +- src/buildstream/_yaml.pxd | 3 +++ src/buildstream/_yaml.pyx | 28 +++++++++++++++++++++++++--- src/buildstream/plugin.py | 4 ++-- src/buildstream/plugins/elements/compose.py | 4 ++-- src/buildstream/plugins/elements/filter.py | 2 +- src/buildstream/plugins/elements/script.py | 3 +-- src/buildstream/plugins/sources/remote.py | 2 +- tests/format/include.py | 6 +++--- tests/sources/git.py | 4 ++-- 16 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/buildstream/_cas/casremote.py b/src/buildstream/_cas/casremote.py index 0195e217a..56cfea20f 100644 --- a/src/buildstream/_cas/casremote.py +++ b/src/buildstream/_cas/casremote.py @@ -33,7 +33,7 @@ class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key def _new_from_config_node(spec_node, basedir=None): _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert', 'instance-name']) url = spec_node.get_str('url') - push = _yaml.node_get(spec_node, bool, 'push', default_value=False) + push = spec_node.get_bool('push', default=False) if not url: provenance = _yaml.node_get_provenance(spec_node, 'url') raise LoadError(LoadErrorReason.INVALID_DATA, diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py index e285f6b6a..f32f5aa91 100644 --- a/src/buildstream/_context.py +++ b/src/buildstream/_context.py @@ -273,7 +273,7 @@ class Context(): self.remote_execution_specs = SandboxRemote.specs_from_config_node(defaults) # Load pull build trees configuration - self.pull_buildtrees = _yaml.node_get(cache, bool, 'pull-buildtrees') + self.pull_buildtrees = cache.get_bool('pull-buildtrees') # Load cache build trees configuration self.cache_buildtrees = _node_get_option_str( @@ -287,8 +287,8 @@ class Context(): 'debug', 'element-format', 'message-format' ]) self.log_key_length = _yaml.node_get(logging, int, 'key-length') - self.log_debug = _yaml.node_get(logging, bool, 'debug') - self.log_verbose = _yaml.node_get(logging, bool, 'verbose') + self.log_debug = logging.get_bool('debug') + self.log_verbose = logging.get_bool('verbose') self.log_error_lines = _yaml.node_get(logging, int, 'error-lines') self.log_message_lines = _yaml.node_get(logging, int, 'message-lines') self.log_element_format = logging.get_str('element-format') @@ -423,7 +423,7 @@ class Context(): # so work out if we should be strict, and then cache the result toplevel = self.get_toplevel_project() overrides = self.get_overrides(toplevel.name) - self._strict_build_plan = _yaml.node_get(overrides, bool, 'strict', default_value=True) + self._strict_build_plan = overrides.get_bool('strict', default=True) # If it was set by the CLI, it overrides any config # Ditto if we've already computed this, then we return the computed diff --git a/src/buildstream/_gitsourcebase.py b/src/buildstream/_gitsourcebase.py index eefa36b1d..61ecd36eb 100644 --- a/src/buildstream/_gitsourcebase.py +++ b/src/buildstream/_gitsourcebase.py @@ -388,7 +388,7 @@ class _GitSourceBase(Source): self.node_validate(tag_node, ['tag', 'commit', 'annotated']) tags = self._load_tags(node) - self.track_tags = self.node_get_member(node, bool, 'track-tags', False) + self.track_tags = node.get_bool('track-tags', default=False) self.original_url = node.get_str('url') self.mirror = self.BST_MIRROR_CLASS(self, '', self.original_url, ref, tags=tags, primary=True) @@ -405,7 +405,7 @@ class _GitSourceBase(Source): raise SourceError("{}: Git sources require a ref and/or track".format(self), reason="missing-track-and-ref") - self.checkout_submodules = self.node_get_member(node, bool, 'checkout-submodules', True) + self.checkout_submodules = node.get_bool('checkout-submodules', default=True) self.submodules = [] # Parse a dict of submodule overrides, stored in the submodule_overrides @@ -423,7 +423,7 @@ class _GitSourceBase(Source): self.submodule_overrides[path] = url if 'checkout' in submodule: - checkout = self.node_get_member(submodule, bool, 'checkout') + checkout = submodule.get_bool('checkout') self.submodule_checkout_overrides[path] = checkout self.mark_download_url(self.original_url) @@ -667,7 +667,7 @@ class _GitSourceBase(Source): for tag_node in tags_node: tag = tag_node.get_str('tag') commit_ref = tag_node.get_str('commit') - annotated = self.node_get_member(tag_node, bool, 'annotated') + annotated = tag_node.get_bool('annotated') tags.append((tag, commit_ref, annotated)) return tags diff --git a/src/buildstream/_options/optionbool.py b/src/buildstream/_options/optionbool.py index caeff9711..1e5935e48 100644 --- a/src/buildstream/_options/optionbool.py +++ b/src/buildstream/_options/optionbool.py @@ -34,13 +34,13 @@ class OptionBool(Option): super().load(node) _yaml.node_validate(node, OPTION_SYMBOLS + ['default']) - self.value = _yaml.node_get(node, bool, 'default') + self.value = node.get_bool('default') def load_value(self, node, *, transform=None): if transform: self.set_value(transform(node.get_str(self.name))) else: - self.value = _yaml.node_get(node, bool, self.name) + self.value = node.get_bool(self.name) def set_value(self, value): if value in ('True', 'true'): diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index bb1b06b43..6157206ff 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -687,13 +687,10 @@ class Project(): self._splits = config.get_mapping('split-rules') # Support backwards compatibility for fail-on-overlap - fail_on_overlap = _yaml.node_get(config, bool, 'fail-on-overlap', default_value=None) - - if (CoreWarnings.OVERLAPS not in self._fatal_warnings) and fail_on_overlap: - self._fatal_warnings.append(CoreWarnings.OVERLAPS) + fail_on_overlap = config.get_scalar('fail-on-overlap', None) # Deprecation check - if fail_on_overlap is not None: + if not fail_on_overlap.is_none(): self._context.message( Message( None, @@ -703,6 +700,9 @@ class Project(): ) ) + if (CoreWarnings.OVERLAPS not in self._fatal_warnings) and fail_on_overlap.as_bool(): + self._fatal_warnings.append(CoreWarnings.OVERLAPS) + # Load project.refs if it exists, this may be ignored. if self.ref_storage == ProjectRefStorage.PROJECT_REFS: self.refs.load(self.options) diff --git a/src/buildstream/_variables.pyx b/src/buildstream/_variables.pyx index c41ff5c3f..e8f083912 100644 --- a/src/buildstream/_variables.pyx +++ b/src/buildstream/_variables.pyx @@ -120,7 +120,7 @@ cdef class Variables: # element, then override max-jobs to be 1. # Initialize it as a string as all variables are processed as strings. # - if _yaml.node_get(node, bool, 'notparallel', None, False): + if node.get_bool('notparallel', False): _yaml.node_set(node, 'max-jobs', str(1)) cdef dict ret = {} diff --git a/src/buildstream/_workspaces.py b/src/buildstream/_workspaces.py index a4f7abc76..bcf0ac92c 100644 --- a/src/buildstream/_workspaces.py +++ b/src/buildstream/_workspaces.py @@ -630,7 +630,7 @@ class Workspaces(): # def _load_workspace(self, node): dictionary = { - 'prepared': _yaml.node_get(node, bool, 'prepared', default_value=False), + '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( diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd index b2237f075..d308a23a8 100644 --- a/src/buildstream/_yaml.pxd +++ b/src/buildstream/_yaml.pxd @@ -32,11 +32,14 @@ cdef class MappingNode(Node): cdef Node get(self, str key, default, default_constructor) cpdef MappingNode get_mapping(self, str key, default=*) cpdef ScalarNode get_scalar(self, str key, default=*) + cpdef bint get_bool(self, str key, default=*) except * cpdef str get_str(self, str key, object default=*) cdef class ScalarNode(Node): + cpdef bint as_bool(self) except * cpdef str as_str(self) + cpdef bint is_none(self) cdef class ProvenanceInformation: diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx index a7779c41e..eb73ef4d6 100644 --- a/src/buildstream/_yaml.pyx +++ b/src/buildstream/_yaml.pyx @@ -85,6 +85,25 @@ cdef class ScalarNode(Node): self.line = line self.column = column + cpdef bint is_none(self): + return self.value is None + + cpdef bint as_bool(self) except *: + if type(self.value) is bool: + return self.value + + # Don't coerce booleans to string, this makes "False" strings evaluate to True + if self.value in ('True', 'true'): + return True + elif self.value in ('False', 'false'): + return False + else: + provenance = node_get_provenance(self) + path = node_find_target(provenance.toplevel, self)[-1] + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Value of '{}' is not of the expected type '{}'" + .format(provenance, path, bool.__name__, self.value)) + cpdef str as_str(self): # We keep 'None' as 'None' to simplify the API's usage and allow chaining for users if self.value is None: @@ -132,7 +151,7 @@ cdef class MappingNode(Node): if type(value) is not ScalarNode: if value is None: - value = ScalarNode(None, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter()) + value = ScalarNode(None, self.file_index, 0, next_synthetic_counter()) else: provenance = node_get_provenance(value) raise LoadError(LoadErrorReason.INVALID_DATA, @@ -141,6 +160,11 @@ cdef class MappingNode(Node): return value + cpdef bint get_bool(self, str key, object default=_sentinel) except *: + # TODO: don't go through 'get_scalar', we can directly get everything and optimize + cdef ScalarNode scalar = self.get_scalar(key, default) + return scalar.as_bool() + cpdef str get_str(self, str key, object default=_sentinel): # TODO: don't go through 'get_scalar', we can directly get everything and optimize cdef ScalarNode scalar = self.get_scalar(key, default) @@ -574,8 +598,6 @@ def dump(object contents, str filename=None): # Returns: The Provenance of the dict, member or list element # cpdef ProvenanceInformation node_get_provenance(Node node, str key=None, list indices=None): - assert type(node.value) is dict - if key is None: # Retrieving the provenance for this node directly return ProvenanceInformation(node) diff --git a/src/buildstream/plugin.py b/src/buildstream/plugin.py index a12ff61ec..e209e5f4e 100644 --- a/src/buildstream/plugin.py +++ b/src/buildstream/plugin.py @@ -894,10 +894,10 @@ class Plugin(): project = self.__project for key, value in self.node_items(project.element_overrides): - if _yaml.node_get(value, bool, 'suppress-deprecation-warnings', default_value=False): + if value.get_bool('suppress-deprecation-warnings', default=False): silenced_warnings.add(key) for key, value in self.node_items(project.source_overrides): - if _yaml.node_get(value, bool, 'suppress-deprecation-warnings', default_value=False): + if value.get_bool('suppress-deprecation-warnings', default=False): silenced_warnings.add(key) return self.get_kind() in silenced_warnings diff --git a/src/buildstream/plugins/elements/compose.py b/src/buildstream/plugins/elements/compose.py index b672cde0c..b2e52170e 100644 --- a/src/buildstream/plugins/elements/compose.py +++ b/src/buildstream/plugins/elements/compose.py @@ -65,10 +65,10 @@ class ComposeElement(Element): # We name this variable 'integration' only to avoid # collision with the Element.integrate() method. - self.integration = self.node_get_member(node, bool, 'integrate') + self.integration = node.get_bool('integrate') self.include = self.node_get_member(node, list, 'include') self.exclude = self.node_get_member(node, list, 'exclude') - self.include_orphans = self.node_get_member(node, bool, 'include-orphans') + self.include_orphans = node.get_bool('include-orphans') def preflight(self): pass diff --git a/src/buildstream/plugins/elements/filter.py b/src/buildstream/plugins/elements/filter.py index 7fdb96d7a..e62a9d49b 100644 --- a/src/buildstream/plugins/elements/filter.py +++ b/src/buildstream/plugins/elements/filter.py @@ -173,7 +173,7 @@ class FilterElement(Element): self.include = self.node_get_member(node, list, 'include') self.exclude = self.node_get_member(node, list, 'exclude') - self.include_orphans = self.node_get_member(node, bool, 'include-orphans') + self.include_orphans = node.get_bool('include-orphans') self.include_provenance = self.node_provenance(node, member_name='include') self.exclude_provenance = self.node_provenance(node, member_name='exclude') diff --git a/src/buildstream/plugins/elements/script.py b/src/buildstream/plugins/elements/script.py index 0d194dcc1..df03bad17 100644 --- a/src/buildstream/plugins/elements/script.py +++ b/src/buildstream/plugins/elements/script.py @@ -60,8 +60,7 @@ class ScriptElement(buildstream.ScriptElement): self.set_work_dir() self.set_install_root() - self.set_root_read_only(self.node_get_member(node, bool, - 'root-read-only', False)) + self.set_root_read_only(node.get_bool('root-read-only', default=False)) # Plugin entry point diff --git a/src/buildstream/plugins/sources/remote.py b/src/buildstream/plugins/sources/remote.py index 4e9fe4f7d..bb4c11325 100644 --- a/src/buildstream/plugins/sources/remote.py +++ b/src/buildstream/plugins/sources/remote.py @@ -63,7 +63,7 @@ class RemoteSource(DownloadableFileSource): super().configure(node) self.filename = node.get_str('filename', os.path.basename(self.url)) - self.executable = self.node_get_member(node, bool, 'executable', False) + self.executable = node.get_bool('executable', default=False) if os.sep in self.filename: raise SourceError('{}: filename parameter cannot contain directories'.format(self), diff --git a/tests/format/include.py b/tests/format/include.py index 82667f19f..7c5a35a65 100644 --- a/tests/format/include.py +++ b/tests/format/include.py @@ -28,7 +28,7 @@ def test_include_project_file(cli, datafiles): 'element.bst']) result.assert_success() loaded = _yaml.load_data(result.output) - assert _yaml.node_get(loaded, bool, 'included') + assert loaded.get_bool('included') def test_include_missing_file(cli, tmpdir): @@ -87,7 +87,7 @@ def test_include_junction_file(cli, tmpdir, datafiles): 'element.bst']) result.assert_success() loaded = _yaml.load_data(result.output) - assert _yaml.node_get(loaded, bool, 'included') + assert loaded.get_bool('included') @pytest.mark.datafiles(DATA_DIR) @@ -310,4 +310,4 @@ def test_local_to_junction(cli, tmpdir, datafiles): 'element.bst']) result.assert_success() loaded = _yaml.load_data(result.output) - assert _yaml.node_get(loaded, bool, 'included') + assert loaded.get_bool('included') diff --git a/tests/sources/git.py b/tests/sources/git.py index 2b8a12997..e3f803469 100644 --- a/tests/sources/git.py +++ b/tests/sources/git.py @@ -855,7 +855,7 @@ def test_git_describe(cli, tmpdir, datafiles, ref_storage, tag_type): assert 'tag' in tag assert 'commit' in tag assert 'annotated' in tag - assert _yaml.node_get(tag, bool, 'annotated') == (tag_type == 'annotated') + assert tag.get_bool('annotated') == (tag_type == 'annotated') assert {(tag.get_str('tag'), tag.get_str('commit')) @@ -970,7 +970,7 @@ def test_git_describe_head_is_tagged(cli, tmpdir, datafiles, ref_storage, tag_ty assert 'tag' in tag assert 'commit' in tag assert 'annotated' in tag - assert _yaml.node_get(tag, bool, 'annotated') == (tag_type == 'annotated') + assert tag.get_bool('annotated') == (tag_type == 'annotated') tag_name = tag.get_str('tag') commit = tag.get_str('commit') -- cgit v1.2.1