[Notes] [Git][BuildStream/buildstream][tpollard/591] 7 commits: Report p

Tiago Gomes pushed to branch tpollard/591 at BuildStream / buildstream

Commits:

a38d9163

by Valentin David at 2018-08-30T10:26:57Z

Report processing errors from tracking

Failures to write files when tracking were not reported.

Fixes #533.

02138181

by Tristan Van Berkom at 2018-08-30T11:36:00Z

Merge branch 'valentindavid/post_tracking_errors' into 'master'

Report processing errors from tracking

Closes #533

See merge request BuildStream/buildstream!747

d23d4f3e

by Tristan Van Berkom at 2018-08-30T12:02:27Z

_artifactcache/artifactcache.py: Write the cache_size file atomically

This is causing issues while the size file is being read and written
simultaneously.

The proper fix will be to read/add/save the file atomically and that
will require locking, but this fix is a good stop gap to existing crashes.

bb6aa5a5

by Tristan Van Berkom at 2018-08-30T12:27:55Z

Merge branch 'tristan/atomic-cache-size-file' into 'master'

_artifactcache/artifactcache.py: Write the cache_size file atomically

See merge request BuildStream/buildstream!762

680e4fe1

by knownexus at 2018-08-30T13:37:55Z

Replacing string 'bzr' with value from host tools

8cd719eb

by Phillip Smyth at 2018-08-30T15:47:06Z

Merge branch 'bzr_fix' into 'master'

Replacing string 'bzr' with value from host tools

See merge request BuildStream/buildstream!764

18d0bfb4

by Tom Pollard at 2018-08-30T19:14:33Z

buildstream/_project.py: Report if project.conf is missing name

Explicitly check that project.conf contains a name. This resolves
the issue of the provenance check from _yaml.py incorrectly reporting
the offending file as the default_config_node projectconfig.yaml
when no name key exists in the pre_config_node dict.

5 changed files:

buildstream/_artifactcache/artifactcache.py
buildstream/_project.py
buildstream/_scheduler/queues/trackqueue.py
tests/frontend/track.py
tests/testutils/repo/bzr.py

Changes:

buildstream/_artifactcache/artifactcache.py

@@ -571,7 +571,7 @@ class ArtifactCache():
      def _write_cache_size(self, size):
          assert isinstance(size, int)
          size_file_path = os.path.join(self.context.artifactdir, CACHE_SIZE_FILE)
 -        with open(size_file_path, "w") as f:
 +        with utils.save_file_atomic(size_file_path, "w") as f:
              f.write(str(size))
      # _read_cache_size()

buildstream/_project.py

@@ -398,6 +398,11 @@ class Project():
                  "Project requested format version {}, but BuildStream {}.{} only supports up until format version {}"
                  .format(format_version, major, minor, BST_FORMAT_VERSION))
 +        # Check if project.conf contains name key field
 +        if self._project_conf.get('name') is None:
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: project.conf does not contain expected key '{}'".format(projectfile, 'name'))
++
          # The project name, element path and option declarations
          # are constant and cannot be overridden by option conditional statements
          self.name = _yaml.node_get(pre_config_node, str, 'name')

buildstream/_scheduler/queues/trackqueue.py

@@ -58,18 +58,9 @@ class TrackQueue(Queue):
          # Set the new refs in the main process one by one as they complete
          for unique_id, new_ref in result:
              source = _plugin_lookup(unique_id)
 -            try:
 -                # We appear processed if at least one source has changed
 -                if source._save_ref(new_ref):
 -                    changed = True
 -            except SourceError as e:
 -                # FIXME: We currently dont have a clear path to
 -                #        fail the scheduler from the main process, so
 -                #        this will just warn and BuildStream will exit
 -                #        with a success code.
 -                #
 -                source.warn("Failed to update project file",
 -                            detail="{}".format(e))
 +            # We appear processed if at least one source has changed
 +            if source._save_ref(new_ref):
 +                changed = True
          element._tracking_done()

tests/frontend/track.py

 +import stat
  import os
  import pytest
  from tests.testutils import cli, create_repo, ALL_REPO_KINDS, generate_junction
@@ -634,3 +635,36 @@ def test_track_junction_included(cli, tmpdir, datafiles, ref_storage, kind):
      result = cli.run(project=project, args=['track', 'junction.bst'])
      result.assert_success()
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.parametrize("kind", [(kind) for kind in ALL_REPO_KINDS])
 +def test_track_error_cannot_write_file(cli, tmpdir, datafiles, kind):
 +    if os.geteuid() == 0:
 +        pytest.skip("This is not testable with root permissions")
++
 +    project = str(datafiles)
 +    dev_files_path = os.path.join(project, 'files', 'dev-files')
 +    element_path = os.path.join(project, 'elements')
 +    element_name = 'track-test-{}.bst'.format(kind)
++
 +    configure_project(project, {
 +        'ref-storage': 'inline'
 +    })
++
 +    repo = create_repo(kind, str(tmpdir))
 +    ref = repo.create(dev_files_path)
++
 +    element_full_path = os.path.join(element_path, element_name)
 +    generate_element(repo, element_full_path)
++
 +    st = os.stat(element_path)
 +    try:
 +        read_mask = stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH
 +        os.chmod(element_path, stat.S_IMODE(st.st_mode) & ~read_mask)
++
 +        result = cli.run(project=project, args=['track', element_name])
 +        result.assert_main_error(ErrorDomain.STREAM, None)
 +        result.assert_task_error(ErrorDomain.SOURCE, 'save-ref-error')
 +    finally:
 +        os.chmod(element_path, stat.S_IMODE(st.st_mode))

tests/testutils/repo/bzr.py

@@ -2,6 +2,7 @@ import os
  import subprocess
  import pytest
 +from buildstream import utils
  from .repo import Repo
  from ..site import HAVE_BZR
@@ -16,15 +17,16 @@ class Bzr(Repo):
          if not HAVE_BZR:
              pytest.skip("bzr is not available")
          super(Bzr, self).__init__(directory, subdir)
 +        self.bzr = utils.get_host_tool('bzr')
      def create(self, directory):
          branch_dir = os.path.join(self.repo, 'trunk')
 -        subprocess.call(['bzr', 'init-repo', self.repo], env=BZR_ENV)
 -        subprocess.call(['bzr', 'init', branch_dir], env=BZR_ENV)
 +        subprocess.call([self.bzr, 'init-repo', self.repo], env=BZR_ENV)
 +        subprocess.call([self.bzr, 'init', branch_dir], env=BZR_ENV)
          self.copy_directory(directory, branch_dir)
 -        subprocess.call(['bzr', 'add', '.'], env=BZR_ENV, cwd=branch_dir)
 -        subprocess.call(['bzr', 'commit', '--message="Initial commit"'],
 +        subprocess.call([self.bzr, 'add', '.'], env=BZR_ENV, cwd=branch_dir)
 +        subprocess.call([self.bzr, 'commit', '--message="Initial commit"'],
                          env=BZR_ENV, cwd=branch_dir)
          return self.latest_commit()
@@ -42,7 +44,7 @@ class Bzr(Repo):
      def latest_commit(self):
          output = subprocess.check_output([
 -            'bzr', 'version-info',
 +            self.bzr, 'version-info',
              '--custom', '--template={revno}',
              os.path.join(self.repo, 'trunk')
          ], env=BZR_ENV)

[Notes] [Git][BuildStream/buildstream][tpollard/591] 7 commits: Report processing errors from tracking

Tiago Gomes pushed to branch tpollard/591 at BuildStream / buildstream

Commits:

5 changed files:

Changes: