[Notes] [Git][BuildStream/buildstream][Qinusty/600-recursive-variables]

Qinusty pushed to branch Qinusty/600-recursive-variables at BuildStream / buildstream

Commits:

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

0de9c747

by Josh Smith at 2018-08-23T11:17:00Z

Add cyclic check within variable resolution

This aims to address #600, this will raise an exception when a resolved
variable contains a reference to the variable.

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:

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/_exceptions.py

@@ -217,6 +217,9 @@ class LoadErrorReason(Enum):
      # A recursive include has been encountered.
      RECURSIVE_INCLUDE = 21
 +    # A recursive variable has been encountered
 +    RECURSIVE_VARIABLE = 22
++
  # LoadError
+ #

buildstream/_variables.py

@@ -132,6 +132,12 @@ class Variables():
                  value = _yaml.node_get(variables, str, key)
                  resolved_var, item_unmatched = self._subst(value, variables)
 +                if _wrap_variable(key) in resolved_var:
 +                    raise LoadError(LoadErrorReason.RECURSIVE_VARIABLE,
 +                                    "{}: ".format(_yaml.node_get_provenance(variables, key)) +
 +                                    ("Variable '{}' references itself recursively through" +
 +                                     " another variable declariation.").format(key))
++
                  resolved[key] = resolved_var
                  unmatched += item_unmatched
@@ -162,14 +168,17 @@ class Variables():
                                  "Failed to resolve one or more variable:\n{}".format(summary))
              last_unmatched = unmatched
+-
          return resolved
      # Helper function to fetch information about the node referring to a variable
+     #
      def _find_references(self, varname):
 -        fullname = '%{' + varname + '}'
 +        fullname = _wrap_variable(varname)
          for key, value in _yaml.node_items(self.original):
              if fullname in value:
                  provenance = _yaml.node_get_provenance(self.original, key)
                  yield (key, provenance)
++
++
 +def _wrap_variable(var):
 +    return "%{" + var + "}"

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)

[Notes] [Git][BuildStream/buildstream][Qinusty/600-recursive-variables] 7 commits: Remove artifact extracts when artifact expires in cache

Qinusty pushed to branch Qinusty/600-recursive-variables at BuildStream / buildstream

Commits:

7 changed files:

Changes: