[Notes] [Git][BuildStream/buildstream][jennis/correct

James Ennis pushed to branch jennis/correct_HACKING at BuildStream / buildstream

Commits:

91d62d2f

by Chandan Singh at 2018-08-22T18:38:01Z

MANIFEST.in: Fix include for dev-requirements.txt

Currently, running any setuptools commands using `setup.py` generates a
warning like so:
        warning: manifest_maker: MANIFEST.in, line 21: unknown action 'dev-requirements.txt'

The syntax is invalid and got accidentally introduced in !637 but it's a
shame that this is not a warning and no obvious way to make it an error
if there are invalid things in MANIFEST.in file.

cd0775eb

by Chandan Singh at 2018-08-22T23:44:36Z

Merge branch 'chandan/fix-mainfest-dev-requirements' into 'master'

MANIFEST.in: Fix include for dev-requirements.txt

See merge request BuildStream/buildstream!704

b0d1aa83

by Tristan Van Berkom at 2018-08-23T05:54:05Z

_frontend/linuxapp.py: Fixing fallout from !693

When fixing terminal notifications, I had introduced a bug
with accesses to `os.environ` which triggered KeyError, this
patch fixes it.

2215859c

by Valentin David at 2018-08-23T06:40:19Z

Improve error message for deleted open workspaces

Fixes #576.

e37ac3bc

by Tristan Van Berkom at 2018-08-23T07:45:49Z

Merge branch 'valentindavid/inconsistant-workspace' into 'master'

Improve error message for deleted open workspaces

Closes #576

See merge request BuildStream/buildstream!703

2ac654a8

by Valentin David at 2018-08-23T08:02:50Z

Remove artifact extracts when artifact expires in cache

Fixes #561

176aa09f

by Tristan Van Berkom at 2018-08-23T09:10:51Z

Merge branch 'valentindavid/extract-expiry' into 'master'

Remove artifact extracts when artifact expires in cache

Closes #561

See merge request BuildStream/buildstream!685

f1e3f10f

by Mathieu Bridon at 2018-08-23T09:11:47Z

deps: Specify the minimum version required for blessings

Buildstream uses the Terminal.does_styling attribute, which was only
added in blessings 1.6.

Blessings helpfully returns an empty string when calling an nonexistent
attribute on the Terminal object, instead of failing.

Because Fedora has blessings 1.5, Buildstream thought that my terminal
couldn't handle styling, and just didn't print the status bar, silently.

Specifying the minimum version avoids this silent failure.

bea01531
by Tristan Van Berkom at 2018-08-23T09:14:40Z
```
.gitlab-ci.yml: Avoid running tests in post-merge
```

da19bcf1

by Tristan Van Berkom at 2018-08-23T09:14:40Z

README.rst: Removing the badges

Now that we only run pre-merge CI, it makes no sense to show a badge for
the coverage and pipeline status of the last run of master.

It would make sense to show a badge for the last pipeline which
was run for the last merge request that was merged to master, but
I don't see any feature to get that information.

8216ff8d

by Tristan Van Berkom at 2018-08-23T10:13:23Z

Merge branch 'tristan/reduce-gitlab-ci' into 'master'

.gitlab-ci.yml: Avoid running tests in post-merge

See merge request BuildStream/buildstream!709

ef1da7ba

by James Ennis at 2018-08-23T14:26:43Z

HACKING.rst: Remove -k flag for individual tests as this is not required

Changes:

.gitlab-ci.yml

@@ -10,6 +10,16 @@ stages:
    - test
    - post
 +# Avoid running all the tests post merge on
 +# master or on any release branch.
 +#
 +.tests-condition-template: &tests-condition
 +  only:
 +  - branches
 +  except:
 +  - master
 +  - /bst-1\..*/
++
  #####################################################
  #                  Prepare stage                    #
  #####################################################
@@ -81,20 +91,26 @@ source_dist:
  tests-debian-9:
    image: buildstream/testsuite-debian:9-master-114-4cab18e3
    <<: *linux-tests
 +  <<: *tests-condition
  tests-fedora-27:
    image: buildstream/testsuite-fedora:27-master-114-4cab18e3
    <<: *linux-tests
 +  <<: *tests-condition
  tests-fedora-28:
    image: buildstream/testsuite-fedora:28-master-114-4cab18e3
    <<: *linux-tests
 +  <<: *tests-condition
  tests-ubuntu-18.04:
    image: buildstream/testsuite-ubuntu:18.04-master-114-4cab18e3
    <<: *linux-tests
 +  <<: *tests-condition
  tests-unix:
 +  <<: *tests-condition
++
    # Use fedora here, to a) run a test on fedora and b) ensure that we
    # can get rid of ostree - this is not possible with debian-8
    image: buildstream/testsuite-fedora:27-master-114-4cab18e3
@@ -133,6 +149,15 @@ tests-unix:
  # Note: We still do not enforce a consistent installation of python3-sphinx,
  #       as it will significantly grow the backing image.
  docs:
++
 +  # Here we build the docs for every pre-merge CI, but avoid
 +  # the job on post-merge to stable branches, because we only
 +  # ever publish them from master
 +  only:
 +  - branches
 +  except:
 +  - /bst-1\..*/
++
    stage: test
    script:
    - export BST_SOURCE_CACHE="$(pwd)/cache/integration-cache/sources"
@@ -157,6 +182,8 @@ docs:
  # as an output of radon, with some conversion
+ #
  codequality:
 +  <<: *tests-condition
++
    image: docker:stable
    stage: post
    variables:
@@ -175,6 +202,8 @@ codequality:
      paths: [codeclimate.json]
  analysis:
 +  <<: *tests-condition
++
    stage: post
    script:
    - |
@@ -203,6 +232,8 @@ analysis:
  # Collate coverage reports
+ #
  coverage:
 +  <<: *tests-condition
++
    stage: post
    coverage: '/TOTAL +\d+ +\d+ +(\d+\.\d+)%/'
    script:

HACKING.rst

@@ -465,12 +465,12 @@ If you want to run a specific test or a group of tests, you
  can specify a prefix to match. E.g. if you want to run all of
  the frontend tests you can do::
 -  ./setup.py test --addopts '-k tests/frontend/'
 +  ./setup.py test --addopts 'tests/frontend/'
  Specific tests can be chosen by using the :: delimeter after the test module.
  If you wanted to run the test_build_track test within frontend/buildtrack.py you could do::
 -  ./setup.py test --adopts '-k tests/frontend/buildtrack.py::test_build_track'
 +  ./setup.py test --addopts 'tests/frontend/buildtrack.py::test_build_track'
  We also have a set of slow integration tests that are disabled by
  default - you will notice most of them marked with SKIP in the pytest

MANIFEST.in

@@ -23,4 +23,4 @@ recursive-include tests *.expected
  recursive-include buildstream/_protos *.proto
  # Requirements files
 -dev-requirements.txt
 +include dev-requirements.txt

README.rst

  About
  -----
 -.. image:: https://gitlab.com/BuildStream/buildstream/badges/master/pipeline.svg
 -   :target: https://gitlab.com/BuildStream/buildstream/commits/master
+-
 -.. image:: https://gitlab.com/BuildStream/buildstream/badges/master/coverage.svg?job=coverage
 -   :target: https://gitlab.com/BuildStream/buildstream/commits/master
  What is BuildStream?

buildstream/_artifactcache/cascache.py

@@ -30,6 +30,8 @@ from urllib.parse import urlparse
  import grpc
 +from .. import _yaml
++
  from .._protos.google.bytestream import bytestream_pb2, bytestream_pb2_grpc
  from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remote_execution_pb2_grpc
  from .._protos.buildstream.v2 import buildstream_pb2, buildstream_pb2_grpc
@@ -526,6 +528,25 @@ class CASCache(ArtifactCache):
+     #
      def remove(self, ref, *, defer_prune=False):
 +        # Remove extract if not used by other ref
 +        tree = self.resolve_ref(ref)
 +        ref_name, ref_hash = os.path.split(ref)
 +        extract = os.path.join(self.extractdir, ref_name, tree.hash)
 +        keys_file = os.path.join(extract, 'meta', 'keys.yaml')
 +        if os.path.exists(keys_file):
 +            keys_meta = _yaml.load(keys_file)
 +            keys = [keys_meta['strong'], keys_meta['weak']]
 +            remove_extract = True
 +            for other_hash in keys:
 +                if other_hash == ref_hash:
 +                    continue
 +                remove_extract = False
 +                break
++
 +            if remove_extract:
 +                utils._force_rmtree(extract)
++
 +        # Remove cache ref
          refpath = self._refpath(ref)
          if not os.path.exists(refpath):
              raise ArtifactError("Could not find artifact for ref '{}'".format(ref))

buildstream/_frontend/linuxapp.py

@@ -28,9 +28,9 @@ from .app import App
+ #
  def _osc_777_supported():
 -    term = os.environ['TERM']
 +    term = os.environ.get('TERM')
 -    if term.startswith('xterm') or term.startswith('vte'):
 +    if term and (term.startswith('xterm') or term.startswith('vte')):
          # Since vte version 4600, upstream silently ignores
          # the OSC 777 without printing garbage to the terminal.
@@ -39,10 +39,10 @@ def _osc_777_supported():
          # will trigger a desktop notification and bring attention
          # to the terminal.
+         #
 -        vte_version = os.environ['VTE_VERSION']
 +        vte_version = os.environ.get('VTE_VERSION')
          try:
              vte_version_int = int(vte_version)
 -        except ValueError:
 +        except (ValueError, TypeError):
              return False
          if vte_version_int >= 4600:

buildstream/_pipeline.py

@@ -355,10 +355,14 @@ class Pipeline():
+     #
      def assert_consistent(self, elements):
          inconsistent = []
 +        inconsistent_workspaced = []
          with self._context.timed_activity("Checking sources"):
              for element in elements:
                  if element._get_consistency() == Consistency.INCONSISTENT:
 -                    inconsistent.append(element)
 +                    if element._get_workspace():
 +                        inconsistent_workspaced.append(element)
 +                    else:
 +                        inconsistent.append(element)
          if inconsistent:
              detail = "Exact versions are missing for the following elements:\n\n"
@@ -372,6 +376,13 @@ class Pipeline():
              raise PipelineError("Inconsistent pipeline", detail=detail, reason="inconsistent-pipeline")
 +        if inconsistent_workspaced:
 +            detail = "Some workspaces do not exist but are not closed\n" + \
 +                     "Try closing them with `bst workspace close`\n\n"
 +            for element in inconsistent_workspaced:
 +                detail += "  " + element._get_full_name() + "\n"
 +            raise PipelineError("Inconsistent pipeline", detail=detail, reason="inconsistent-pipeline-workspaced")
++
      #############################################################
      #                     Private Methods                       #
      #############################################################

setup.py

@@ -273,7 +273,7 @@ setup(name='BuildStream',
            'ruamel.yaml < 0.15.52',
            'pluginbase',
            'Click',
 -          'blessings',
 +          'blessings >= 1.6',
            'jinja2 >= 2.10',
            'protobuf >= 3.5',
            'grpcio >= 1.10',

tests/artifactcache/expiry.py

@@ -268,3 +268,38 @@ def test_invalid_cache_quota(cli, datafiles, tmpdir, quota, success):
          res.assert_success()
      else:
          res.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_extract_expiry(cli, datafiles, tmpdir):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_path = 'elements'
++
 +    cli.configure({
 +        'cache': {
 +            'quota': 10000000,
 +        }
 +    })
++
 +    create_element_size('target.bst', project, element_path, [], 6000000)
 +    res = cli.run(project=project, args=['build', 'target.bst'])
 +    res.assert_success()
 +    assert cli.get_element_state(project, 'target.bst') == 'cached'
++
 +    # Force creating extract
 +    res = cli.run(project=project, args=['checkout', 'target.bst', os.path.join(str(tmpdir), 'checkout')])
 +    res.assert_success()
++
 +    extractdir = os.path.join(project, 'cache', 'artifacts', 'extract', 'test', 'target')
 +    extracts = os.listdir(extractdir)
 +    assert(len(extracts) == 1)
 +    extract = os.path.join(extractdir, extracts[0])
++
 +    # Remove target.bst from artifact cache
 +    create_element_size('target2.bst', project, element_path, [], 6000000)
 +    res = cli.run(project=project, args=['build', 'target2.bst'])
 +    res.assert_success()
 +    assert cli.get_element_state(project, 'target.bst') != 'cached'
++
 +    # Now the extract should be removed.
 +    assert not os.path.exists(extract)

tests/frontend/workspace.py

@@ -767,3 +767,16 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
      # Check that workspace config is converted correctly if necessary
      loaded_config = _yaml.node_sanitize(_yaml.load(workspace_config_path))
      assert loaded_config == parse_dict_as_yaml(expected)
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.parametrize("kind", repo_kinds)
 +def test_inconsitent_pipeline_message(cli, tmpdir, datafiles, kind):
 +    element_name, project, workspace = open_workspace(cli, tmpdir, datafiles, kind, False)
++
 +    shutil.rmtree(workspace)
++
 +    result = cli.run(project=project, args=[
 +        'build', element_name
 +    ])
 +    result.assert_main_error(ErrorDomain.PIPELINE, "inconsistent-pipeline-workspaced")

[Notes] [Git][BuildStream/buildstream][jennis/correct_HACKING] 12 commits: MANIFEST.in: Fix include for dev-requirements.txt

James Ennis pushed to branch jennis/correct_HACKING at BuildStream / buildstream

Commits:

10 changed files:

Changes: