[Notes] [Git][BuildStream/buildstream][bschubert/fix-binary-output-captu

Benjamin Schubert pushed to branch bschubert/fix-binary-output-capture at BuildStream / buildstream

Commits:

f069d82f

by Daniel Silverstone at 2018-10-26T12:19:58Z

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>

283887a5

by James Ennis at 2018-10-26T13:12:12Z

Merge branch 'danielsilverstone-ct/yaml-sentinel-rework' into 'master'

Move _sentinel from utils.py to _yaml.py

See merge request BuildStream/buildstream!903

2c6cb230
by Valentin David at 2018-10-26T14:21:18Z
```
Fix some ruamel warnings during tests
```

a8250ca4

by Valentin David at 2018-10-26T15:04:25Z

Merge branch 'valentindavid/ruamel_warnings' into 'master'

Fix some ruamel warnings during tests

See merge request BuildStream/buildstream!889

1b308fe3

by Daniel Silverstone at 2018-10-26T15:50:42Z

_stream.py: Preserve stdout FD across checkout-to-stdout

In order to support things which might need to use the stdout FD
after checkout has completed writing a tarball to it, preserve it
by means of `os.dup()` and `os.dup2()`

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

65d90c31

by Daniel Silverstone at 2018-10-26T15:50:54Z

tests/testutils/runcli.py: Support binary-mode capture of stdout

In order to test things which write tarballs to stdout correctly,
we need to capture the binary output cleanly.  This ensures we're not
potentially mismatching encodings in and out.

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

ed733f3e

by Daniel Silverstone at 2018-10-26T15:51:40Z

tests/frontend/buildcheckout.py: Enable test_build_checkout_tarball_stdout

With the new support for saving the stdout FDs cleanly, and binary mode
capture in runcli, enable the tarball-to-stdout test.

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

Changes:

buildstream/_stream.py

@@ -423,9 +423,16 @@ class Stream():
                  else:
                      if location == '-':
                          with target.timed_activity("Creating tarball"):
 -                            with os.fdopen(sys.stdout.fileno(), 'wb') as fo:
 -                                with tarfile.open(fileobj=fo, mode="w|") as tf:
 -                                    sandbox_vroot.export_to_tar(tf, '.')
 +                            # Save the stdout FD to restore later
 +                            saved_fd = os.dup(sys.stdout.fileno())
 +                            try:
 +                                with os.fdopen(sys.stdout.fileno(), 'wb') as fo:
 +                                    with tarfile.open(fileobj=fo, mode="w|") as tf:
 +                                        sandbox_vroot.export_to_tar(tf, '.')
 +                            finally:
 +                                # No matter what, restore stdout for further use
 +                                os.dup2(saved_fd, sys.stdout.fileno())
 +                                os.close(saved_fd)
                      else:
                          with target.timed_activity("Creating tarball '{}'"
                                                     .format(location)):

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/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()

tests/frontend/buildcheckout.py

@@ -128,7 +128,6 @@ def test_build_checkout_tarball(datafiles, cli):
      assert os.path.join('.', 'usr', 'include', 'pony.h') in tar.getnames()
 -@pytest.mark.skip(reason="Capturing the binary output is causing a stacktrace")
  @pytest.mark.datafiles(DATA_DIR)
  def test_build_checkout_tarball_stdout(datafiles, cli):
      project = os.path.join(datafiles.dirname, datafiles.basename)
@@ -143,7 +142,7 @@ def test_build_checkout_tarball_stdout(datafiles, cli):
      checkout_args = ['checkout', '--tar', 'target.bst', '-']
 -    result = cli.run(project=project, args=checkout_args)
 +    result = cli.run(project=project, args=checkout_args, binary_capture=True)
      result.assert_success()
      with open(tarball, 'wb') as f:

tests/frontend/yamlcache.py

@@ -103,7 +103,7 @@ def test_yamlcache_used(cli, tmpdir, ref_storage, with_junction, move_project):
          yc.put_from_key(prj, element_path, key, contents)
      # Show that a variable has been added
 -    result = cli.run(project=project, args=['show', '--format', '%{vars}', 'test.bst'])
 +    result = cli.run(project=project, args=['show', '--deps', 'none', '--format', '%{vars}', 'test.bst'])
      result.assert_success()
      data = yaml.safe_load(result.output)
      assert 'modified' in data
@@ -135,7 +135,7 @@ def test_yamlcache_changed_file(cli, tmpdir, ref_storage, with_junction):
          _yaml.load(element_path, copy_tree=False, project=prj, yaml_cache=yc)
      # Show that a variable has been added
 -    result = cli.run(project=project, args=['show', '--format', '%{vars}', 'test.bst'])
 +    result = cli.run(project=project, args=['show', '--deps', 'none', '--format', '%{vars}', 'test.bst'])
      result.assert_success()
      data = yaml.safe_load(result.output)
      assert 'modified' in data

tests/testutils/runcli.py

@@ -17,7 +17,7 @@ import pytest
  # CliRunner convenience API (click.testing module) does not support
  # separation of stdout/stderr.
+ #
 -from _pytest.capture import MultiCapture, FDCapture
 +from _pytest.capture import MultiCapture, FDCapture, FDCaptureBinary
  # Import the main cli entrypoint
  from buildstream._frontend import cli as bst_cli
@@ -234,9 +234,10 @@ class Cli():
      #    silent (bool): Whether to pass --no-verbose
      #    env (dict): Environment variables to temporarily set during the test
      #    args (list): A list of arguments to pass buildstream
 +    #    binary_capture (bool): Whether to capture the stdout/stderr as binary
+     #
      def run(self, configure=True, project=None, silent=False, env=None,
 -            cwd=None, options=None, args=None):
 +            cwd=None, options=None, args=None, binary_capture=False):
          if args is None:
              args = []
          if options is None:
@@ -278,7 +279,7 @@ class Cli():
              except ValueError:
                  sys.__stdout__ = open('/dev/stdout', 'w')
 -            result = self.invoke(bst_cli, bst_args)
 +            result = self.invoke(bst_cli, bst_args, binary_capture=binary_capture)
          # Some informative stdout we can observe when anything fails
          if self.verbose:
@@ -295,7 +296,7 @@ class Cli():
          return result
 -    def invoke(self, cli, args=None, color=False, **extra):
 +    def invoke(self, cli, args=None, color=False, binary_capture=False, **extra):
          exc_info = None
          exception = None
          exit_code = 0
@@ -305,8 +306,8 @@ class Cli():
          old_stdin = sys.stdin
          with open(os.devnull) as devnull:
              sys.stdin = devnull
+-
 -            capture = MultiCapture(out=True, err=True, in_=False, Capture=FDCapture)
 +            capture_kind = FDCaptureBinary if binary_capture else FDCapture
 +            capture = MultiCapture(out=True, err=True, in_=False, Capture=capture_kind)
              capture.start_capturing()
              try:

[Notes] [Git][BuildStream/buildstream][bschubert/fix-binary-output-capture] 7 commits: various: Move _sentinel from utils.py to _yaml.py

Benjamin Schubert pushed to branch bschubert/fix-binary-output-capture at BuildStream / buildstream

Commits:

8 changed files:

Changes: