[Notes] [Git][BuildStream/buildstream][danielsilverstone-ct/yaml-sentine

Daniel Silverstone pushed to branch danielsilverstone-ct/yaml-sentinel-rework at BuildStream / buildstream

Commits:

80762ecb

by Benjamin Schubert at 2018-10-26T09:45:39Z

Check if command is a str and replace by list before checking existence

Currently we would make sure the sandbox had a command before
converting it to a list if it was given as a string. That meant that a string
command would never exist and the check be invalid.

This also adds the same logic in the dummy sandbox for consistency.

f131c407

by Benjamin Schubert at 2018-10-26T10:09:51Z

Merge branch 'bschubert/fix-command-sandbox' into 'master'

Check is command is a str and replace by list before checking existence

Closes #728

See merge request BuildStream/buildstream!898

54d89aa1

by Daniel Silverstone at 2018-10-26T10:20:36Z

_loader/loadelement.py: Use enumerate() in dependency extraction

In cases where elements end up with large numbers of dependencies,
the n-squared performance in `_extract_depends_from_node` can produce
significant delays during project load.  This patch corrects that.
In addition we were double-retrieving provenance which was unnecessary.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

e4c18ba4

by Phil Dawson at 2018-10-26T10:43:55Z

Merge branch 'danielsilverstone-ct/use-enumerate' into 'master'

_loader/loadelement.py: Use enumerate() in dependency extraction

See merge request BuildStream/buildstream!901

0ba52977

by Daniel Silverstone at 2018-10-26T10:45:14Z

various: Move _sentinel from utils.py to _yaml.py

The `_sentinel` in `utils.py` was used only for yaml node tracking.
As such, simplify matters by removing it from `utils.py` and move it
to `_yaml.py` which means that we no longer need to try and avoid a
circular import issue by means of runtime importing.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

Changes:

buildstream/_loader/loadelement.py

@@ -146,8 +146,8 @@ def _extract_depends_from_node(node, *, key=None):
      depends = _yaml.node_get(node, list, key, default_value=[])
      output_deps = []
 -    for dep in depends:
 -        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[depends.index(dep)])
 +    for index, dep in enumerate(depends):
 +        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
          if isinstance(dep, str):
              dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type)
@@ -177,10 +177,8 @@ def _extract_depends_from_node(node, *, key=None):
                                      provenance=dep_provenance)
          else:
 -            index = depends.index(dep)
 -            p = _yaml.node_get_provenance(node, key=key, indices=[index])
              raise LoadError(LoadErrorReason.INVALID_DATA,
 -                            "{}: Dependency is not specified as a string or a dictionary".format(p))
 +                            "{}: Dependency is not specified as a string or a dictionary".format(dep_provenance))
          output_deps.append(dependency)

buildstream/_yaml.py

@@ -335,16 +335,9 @@ def node_get_provenance(node, key=None, indices=None):
      return provenance
 -# Helper to use utils.sentinel without unconditional utils import,
 -# which causes issues for completion.
 -#
 -# Local private, but defined here because sphinx appears to break if
 -# it's not defined before any functions calling it in default kwarg
 -# values.
 -#
 -def _get_sentinel():
 -    from .utils import _sentinel
 -    return _sentinel
 +# A sentinel to be used as a default argument for functions that need
 +# to distinguish between a kwarg set to None and an unset kwarg.
 +_sentinel = object()
  # node_get()
@@ -368,10 +361,10 @@ def _get_sentinel():
  # Note:
  #    Returned strings are stripped of leading and trailing whitespace
+ #
 -def node_get(node, expected_type, key, indices=None, default_value=_get_sentinel()):
 +def node_get(node, expected_type, key, indices=None, default_value=_sentinel):
      value = node.get(key, default_value)
      provenance = node_get_provenance(node)
 -    if value is _get_sentinel():
 +    if value is _sentinel:
          raise LoadError(LoadErrorReason.INVALID_DATA,
                          "{}: Dictionary did not contain expected key '{}'".format(provenance, key))

buildstream/element.py

@@ -451,7 +451,7 @@ class Element(Plugin):
          return None
 -    def node_subst_member(self, node, member_name, default=utils._sentinel):
 +    def node_subst_member(self, node, member_name, default=_yaml._sentinel):
          """Fetch the value of a string node member, substituting any variables
          in the loaded value with the element contextual variables.

buildstream/plugin.py

@@ -321,7 +321,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=utils._sentinel):
 +    def node_get_member(self, node, expected_type, member_name, default=_yaml._sentinel):
          """Fetch the value of a node member, raising an error if the value is
          missing or incorrectly typed.

buildstream/sandbox/_sandboxbwrap.py

@@ -66,15 +66,15 @@ class SandboxBwrap(Sandbox):
          cwd = self._get_work_directory(cwd=cwd)
          env = self._get_environment(cwd=cwd, env=env)
 +        # Convert single-string argument to a list
 +        if isinstance(command, str):
 +            command = [command]
++
          if not self._has_command(command[0], env):
              raise SandboxError("Staged artifacts do not provide command "
                                 "'{}'".format(command[0]),
                                 reason='missing-command')
 -        # We want command args as a list of strings
 -        if isinstance(command, str):
 -            command = [command]
+-
          # Create the mount map, this will tell us where
          # each mount point needs to be mounted from and to
          mount_map = MountMap(self, flags & SandboxFlags.ROOT_READ_ONLY)

buildstream/sandbox/_sandboxchroot.py

@@ -57,15 +57,15 @@ class SandboxChroot(Sandbox):
          cwd = self._get_work_directory(cwd=cwd)
          env = self._get_environment(cwd=cwd, env=env)
 +        # Convert single-string argument to a list
 +        if isinstance(command, str):
 +            command = [command]
++
          if not self._has_command(command[0], env):
              raise SandboxError("Staged artifacts do not provide command "
                                 "'{}'".format(command[0]),
                                 reason='missing-command')
 -        # Command must be a list
 -        if isinstance(command, str):
 -            command = [command]
+-
          stdout, stderr = self._get_output()
          # Create the mount map, this will tell us where

buildstream/sandbox/_sandboxdummy.py

@@ -33,6 +33,10 @@ class SandboxDummy(Sandbox):
          cwd = self._get_work_directory(cwd=cwd)
          env = self._get_environment(cwd=cwd, env=env)
 +        # Convert single-string argument to a list
 +        if isinstance(command, str):
 +            command = [command]
++
          if not self._has_command(command[0], env):
              raise SandboxError("Staged artifacts do not provide command "
                                 "'{}'".format(command[0]),

buildstream/utils.py

@@ -654,10 +654,6 @@ def _pretty_size(size, dec_places=0):
      return "{size:g}{unit}".format(size=round(psize, dec_places), unit=unit)
 -# A sentinel to be used as a default argument for functions that need
 -# to distinguish between a kwarg set to None and an unset kwarg.
 -_sentinel = object()
+-
  # Main process pid
  _main_pid = os.getpid()

[Notes] [Git][BuildStream/buildstream][danielsilverstone-ct/yaml-sentinel-rework] 5 commits: Check if command is a str and replace by list before checking existence

Daniel Silverstone pushed to branch danielsilverstone-ct/yaml-sentinel-rework at BuildStream / buildstream

Commits:

8 changed files:

Changes: