[Notes] [Git][BuildStream/buildstream][abderrahim/artifact-cache-junctio

Abderrahim Kitouni pushed to branch abderrahim/artifact-cache-junction at BuildStream / buildstream

Commits:

3590ca8c

by Abderrahim Kitouni at 2019-01-28T14:59:19Z

requirements/requirements.in: require protobuf >= 3.6

This is needed since 0f2bc3754

39b952dc

by Abderrahim Kitouni at 2019-01-28T15:19:04Z

requirements/requirements.in: require Click >= 7.0

This is needed since 629a6e524, and was lost in the conversion to requirements.in

80b36d0c

by Javier Jardón at 2019-01-28T17:54:52Z

Merge branch 'abderrahim/protobuf-version' into 'master'

requirements/requirements.in: update minimum versions

Closes #884

See merge request BuildStream/buildstream!1114

a1ab48da

by Valentin David at 2019-01-28T21:30:26Z

Fix crash when spawned job completes very fast

Job can complete before we return from `Job.span()` to
`Scheduler._spawn_job()`, so that `_active_jobs` would not yet contain
the job.

This would print a stack on the console and try to run a second time
the job which can have unexpected effects.

In order to reproduce the issue, in
`buildstream/_scheduler/jobs/job.py`, in `Job.spawn`,
add a call to `time.sleep()` right before call to
`asyncio.get_child_watcher()`.

This fixes issue #857.

2fcb4491

by Jürg Billeter at 2019-01-28T22:36:22Z

Merge branch 'valentindavid/crash_in_scheduler_857' into 'master'

Fix crash when spawned job completes very fast

Closes #857

See merge request BuildStream/buildstream!1095

1c05a092

by Valentin David at 2019-01-29T05:58:17Z

Fix type of error codes in CAS server

Fixes #882.

785da59c

by Jürg Billeter at 2019-01-29T06:49:10Z

Merge branch 'valentindavid/wrong_type_in_status_code' into 'master'

Fix type of error codes in CAS server

Closes #882

See merge request BuildStream/buildstream!1099

ddef91ea

by Valentin David at 2019-01-29T07:23:35Z

Make sure testing cache directory exists

Fixes #873

6a4c8611

by Jürg Billeter at 2019-01-29T08:18:45Z

Merge branch 'valentindavid/make_cache_dir' into 'master'

Make sure testing cache directory exists

Closes #873

See merge request BuildStream/buildstream!1092

86c8e414

by Angelos Evripiotis at 2019-01-29T10:39:29Z

BREAK:remove unconditional 'are you sure?' prompts

This is a breaking change, as it affects behaviour that people might be
relying on. An entry has been added to NEWS.

As proposed on the mailing list, this change removes the unconditional
prompts on:

    o: bst workspace reset
    o: bst workspace close --remove-dir

If interactive, these commands would always interrupt you with a prompt
like this:

    This will remove all your changes, are you sure?

This seems like it may just save someone's work some time. It may also
condition folks to hit 'y' quickly without thinking.

This change also makes the non-interactive behaviour consistent with the
interactive behaviour in the default case. There is also the case of the
prompt configured by 'really-workspace-close-project-inaccessible',
which may be tackled in later work.

This change also removes the new config options to suppress those
prompts, and their associated news entry.

The relevant bit of the mailing list conversation is here:
https://mail.gnome.org/archives/buildstream-list/2018-December/msg00106.html

The issue to make interactive and non-interactive behaviour consistent
is here:
https://gitlab.com/BuildStream/buildstream/issues/744

aae35e13

by Angelos Evripiotis at 2019-01-29T12:08:07Z

Merge branch 'aevri/are_you_sure' into 'master'

BREAK:remove unconditional 'are you sure?' prompts

See merge request BuildStream/buildstream!1061

03111d39

by Dor Askayo at 2019-01-30T10:35:06Z

filter.py: don't recurse when staging dependencies

Also bump the element's version so cached artifacts would be
invalidated.

Fixes #883

7256bb0c

by James Ennis at 2019-01-30T11:34:04Z

Merge branch 'doraskayo/filter-indirect-deps-fix' into 'master'

filter.py: don't recurse when staging dependencies

Closes #883

See merge request BuildStream/buildstream!1110

36746730

by Chandan Singh at 2019-01-31T10:50:05Z

tox.ini: Specify minimum version of click-man

`click-man` versions < 0.3.0 do not properly support multiple
entrypoints. Since this was added to `tox` after `0.3.0` was released,
`tox` should never be pulling older versions. But, let's add it here for
documentation purposes.

See
https://gitlab.com/BuildStream/buildstream/merge_requests/1107#note_135187046
for some background on this.

fa4a21ce

by Chandan Singh at 2019-01-31T12:15:43Z

Merge branch 'chandan/min-version-click-man' into 'master'

tox.ini: Specify minimum version of click-man

See merge request BuildStream/buildstream!1120

dd791373

by Chandan Singh at 2019-01-31T14:32:44Z

testutils/site.py: Support parsing more exotic git versions

We use output of `git --version` to determine if we can run some tests
that rely on features from newer git versions. Usually, we expect the
output to be like:

    git version 2.17.2

On some platforms, like MacOS, there could be a suffix after the version
string, so that it looks something like:

    git version 2.17.2 (Apple Git-113)

This causes things to fail like so:

    ValueError: invalid literal for int() with base 10: '2 (Apple Git-113)\n'

Fix logic around `HAVE_OLD_GIT` such that we split the output of
`git --version` without limit on how many times we split. Previously we
used to split only twice so the suffixes like `(Apple Git-113)` are not
part of the parsed version.

96c0fbd6

by Chandan Singh at 2019-01-31T15:39:19Z

Merge branch 'chandan/fix-git-version-mac' into 'master'

testutils/site.py: Support parsing more exotic git versions

See merge request BuildStream/buildstream!1118

d25e2795

by Benjamin Schubert at 2019-01-31T17:06:23Z

Add LoadElement in dependency list for LoadElement idrectly

This removes the need for the 'Dependency' list to then be matched
with the corresponding LoadElement and will allow iterating the
graph more easily

2d0eebbf

by Benjamin Schubert at 2019-01-31T17:06:23Z

Pass element directly to collect_element

This simplifies the loading

583bd97d

by Benjamin Schubert at 2019-02-01T10:26:37Z

Merge branch 'bschubert/loader' into 'master'

Cleanup loader by linking LoadElements sooner

See merge request BuildStream/buildstream!1122

51cec3da

by Phil Dawson at 2019-02-01T14:25:44Z

tests/cachekey: Test cache keys are independent of target elements

2b38aabe

by Phil Dawson at 2019-02-01T15:33:00Z

Merge branch 'phil/cache-key-stability-test' into 'master'

tests/cachekey: Test cache keys are independent of target elements

See merge request BuildStream/buildstream!1123

6a661be6

by Abderrahim Kitouni at 2019-02-01T16:15:03Z

_artifactcache.py: don't leak the project specific remote caches

the code for initializing remotes added the project specific remote
caches to the global list instead of making a copy.

Fixes #618

1c20cab4

by Abderrahim Kitouni at 2019-02-01T16:15:11Z

_project.py: use artifact caches from the parent project for junctions

This makes a junction use the artifact cache of the parent project
before the ones defined for the junction

Fixes #401

24 changed files:

NEWS
buildstream/_artifactcache.py
buildstream/_cas/casserver.py
buildstream/_context.py
buildstream/_frontend/cli.py
buildstream/_loader/loadelement.py
buildstream/_loader/loader.py
buildstream/_project.py
buildstream/_scheduler/scheduler.py
buildstream/data/userconfig.yaml
buildstream/plugins/elements/filter.py
conftest.py
requirements/requirements.in
tests/cachekey/cachekey.py
+ tests/cachekey/project/elements/key-stability/aaa.bst
+ tests/cachekey/project/elements/key-stability/t1.bst
+ tests/cachekey/project/elements/key-stability/t2.bst
+ tests/cachekey/project/elements/key-stability/top-level.bst
+ tests/cachekey/project/elements/key-stability/zzz.bst
tests/elements/filter.py
+ tests/elements/filter/basic/elements/input-with-deps.bst
+ tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst
tests/testutils/site.py
tox.ini

Changes:

NEWS

@@ -50,6 +50,11 @@ buildstream 1.3.1
      an error message and a hint instead, to avoid bothering folks that just
      made a mistake.
 +  o BREAKING CHANGE: The unconditional 'Are you sure?' prompts have been
 +    removed. These would always ask you if you were sure when running
 +    'bst workspace close --remove-dir' or 'bst workspace reset'. They got in
 +    the way too often.
++
    o Failed builds are included in the cache as well.
      `bst checkout` will provide anything in `%{install-root}`.
      A build including cached fails will cause any dependant elements
@@ -87,12 +92,6 @@ buildstream 1.3.1
      instead of just a specially-formatted build-root with a `root` and `scratch`
      subdirectory.
 -  o The buildstream.conf file learned new
 -    'prompt.really-workspace-close-remove-dir' and
 -    'prompt.really-workspace-reset-hard' options. These allow users to suppress
 -    certain confirmation prompts, e.g. double-checking that the user meant to
 -    run the command as typed.
+-
    o Due to the element `build tree` being cached in the respective artifact their
      size in some cases has significantly increased. In *most* cases the build trees
      are not utilised when building targets, as such by default bst 'pull' & 'build'

buildstream/_artifactcache.py

@@ -467,7 +467,7 @@ class ArtifactCache():
      #     on_failure (callable): Called if we fail to contact one of the caches.
+     #
      def initialize_remotes(self, *, on_failure=None):
 -        remote_specs = self.global_remote_specs
 +        remote_specs = list(self.global_remote_specs)
          for project in self.project_remote_specs:
              remote_specs += self.project_remote_specs[project]
@@ -1046,8 +1046,5 @@ class ArtifactCache():
  #   A list of ArtifactCacheSpec instances describing the remote artifact caches.
+ #
  def _configured_remote_artifact_cache_specs(context, project):
 -    project_overrides = context.get_overrides(project.name)
 -    project_extra_specs = ArtifactCache.specs_from_config_node(project_overrides)
+-
      return list(utils._deduplicate(
 -        project_extra_specs + project.artifact_cache_specs + context.artifact_cache_specs))
 +        project.artifact_cache_specs + context.artifact_cache_specs))

buildstream/_cas/casserver.py

@@ -324,7 +324,7 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres
              blob_response.digest.size_bytes = digest.size_bytes
              if len(blob_request.data) != digest.size_bytes:
 -                blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 +                blob_response.status.code = code_pb2.FAILED_PRECONDITION
                  continue
              try:
@@ -335,10 +335,10 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres
                      out.flush()
                      server_digest = self.cas.add_object(path=out.name)
                      if server_digest.hash != digest.hash:
 -                        blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 +                        blob_response.status.code = code_pb2.FAILED_PRECONDITION
              except ArtifactTooLargeException:
 -                blob_response.status.code = grpc.StatusCode.RESOURCE_EXHAUSTED
 +                blob_response.status.code = code_pb2.RESOURCE_EXHAUSTED
          return response

buildstream/_context.py

@@ -121,18 +121,10 @@ class Context():
          # Whether or not to attempt to pull build trees globally
          self.pull_buildtrees = None
 -        # Boolean, whether we double-check with the user that they meant to
 -        # remove a workspace directory.
 -        self.prompt_workspace_close_remove_dir = None
+-
          # Boolean, whether we double-check with the user that they meant to
          # close the workspace when they're using it to access the project.
          self.prompt_workspace_close_project_inaccessible = None
 -        # Boolean, whether we double-check with the user that they meant to do
 -        # a hard reset of a workspace, potentially losing changes.
 -        self.prompt_workspace_reset_hard = None
+-
          # Whether elements must be rebuilt when their dependencies have changed
          self._strict_build_plan = None
@@ -260,16 +252,10 @@ class Context():
          prompt = _yaml.node_get(
              defaults, Mapping, 'prompt')
          _yaml.node_validate(prompt, [
 -            'really-workspace-close-remove-dir',
              'really-workspace-close-project-inaccessible',
 -            'really-workspace-reset-hard',
          ])
 -        self.prompt_workspace_close_remove_dir = _node_get_option_str(
 -            prompt, 'really-workspace-close-remove-dir', ['ask', 'yes']) == 'ask'
          self.prompt_workspace_close_project_inaccessible = _node_get_option_str(
              prompt, 'really-workspace-close-project-inaccessible', ['ask', 'yes']) == 'ask'
 -        self.prompt_workspace_reset_hard = _node_get_option_str(
 -            prompt, 'really-workspace-reset-hard', ['ask', 'yes']) == 'ask'
          # Load per-projects overrides
          self._project_overrides = _yaml.node_get(defaults, Mapping, 'projects', default_value={})

buildstream/_frontend/cli.py

@@ -841,11 +841,6 @@ def workspace_close(app, remove_dir, all_, elements):
          if nonexisting:
              raise AppError("Workspace does not exist", detail="\n".join(nonexisting))
 -        if app.interactive and remove_dir and app.context.prompt_workspace_close_remove_dir:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          for element_name in elements:
              app.stream.workspace_close(element_name, remove_dir=remove_dir)
@@ -879,11 +874,6 @@ def workspace_reset(app, soft, track_, all_, elements):
          if all_ and not app.stream.workspace_exists():
              raise AppError("No open workspaces to reset")
 -        if app.interactive and not soft and app.context.prompt_workspace_reset_hard:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          if all_:
              elements = tuple(element_name for element_name, _ in app.context.get_workspaces().list())

buildstream/_loader/loadelement.py

@@ -39,6 +39,20 @@ from .types import Symbol, Dependency
  #    loader (Loader): The Loader object for this element
+ #
  class LoadElement():
 +    # Dependency():
 +    #
 +    # A link from a LoadElement to its dependencies.
 +    #
 +    # Keeps a link to one of the current Element's dependencies, together with
 +    # its dependency type.
 +    #
 +    # Args:
 +    #    element (LoadElement): a LoadElement on which there is a dependency
 +    #    dep_type (str): the type of dependency this dependency link is
 +    class Dependency:
 +        def __init__(self, element, dep_type):
 +            self.element = element
 +            self.dep_type = dep_type
      def __init__(self, node, filename, loader):
@@ -74,8 +88,11 @@ class LoadElement():
              'build-depends', 'runtime-depends',
          ])
 -        # Extract the Dependencies
 -        self.deps = _extract_depends_from_node(self.node)
 +        self.dependencies = []
++
 +    @property
 +    def junction(self):
 +        return self._loader.project.junction
      # depends():
+     #
@@ -101,8 +118,8 @@ class LoadElement():
              return
          self._dep_cache = {}
 -        for dep in self.deps:
 -            elt = self._loader.get_element_for_dep(dep)
 +        for dep in self.dependencies:
 +            elt = dep.element
              # Ensure the cache of the element we depend on
              elt._ensure_depends_cache()

buildstream/_loader/loader.py

@@ -19,7 +19,6 @@
  import os
  from functools import cmp_to_key
 -from collections import namedtuple
  from collections.abc import Mapping
  import tempfile
  import shutil
@@ -32,8 +31,8 @@ from .._profile import Topics, profile_start, profile_end
  from .._includes import Includes
  from .._yamlcache import YamlCache
 -from .types import Symbol, Dependency
 -from .loadelement import LoadElement
 +from .types import Symbol
 +from .loadelement import LoadElement, _extract_depends_from_node
  from . import MetaElement
  from . import MetaSource
  from ..types import CoreWarnings
@@ -112,7 +111,7 @@ class Loader():
          # First pass, recursively load files and populate our table of LoadElements
+         #
 -        deps = []
 +        target_elements = []
          # XXX This will need to be changed to the context's top-level project if this method
          # is ever used for subprojects
@@ -122,10 +121,10 @@ class Loader():
          with YamlCache.open(self._context, cache_file) as yaml_cache:
              for target in targets:
                  profile_start(Topics.LOAD_PROJECT, target)
 -                junction, name, loader = self._parse_name(target, rewritable, ticker,
 -                                                          fetch_subprojects=fetch_subprojects)
 -                loader._load_file(name, rewritable, ticker, fetch_subprojects, yaml_cache)
 -                deps.append(Dependency(name, junction=junction))
 +                _junction, name, loader = self._parse_name(target, rewritable, ticker,
 +                                                           fetch_subprojects=fetch_subprojects)
 +                element = loader._load_file(name, rewritable, ticker, fetch_subprojects, yaml_cache)
 +                target_elements.append(element)
                  profile_end(Topics.LOAD_PROJECT, target)
+         #
@@ -134,29 +133,29 @@ class Loader():
          # Set up a dummy element that depends on all top-level targets
          # to resolve potential circular dependencies between them
 -        DummyTarget = namedtuple('DummyTarget', ['name', 'full_name', 'deps'])
+-
 -        dummy = DummyTarget(name='', full_name='', deps=deps)
 -        self._elements[''] = dummy
 +        dummy_target = LoadElement("", "", self)
 +        dummy_target.dependencies.extend(
 +            LoadElement.Dependency(element, Symbol.RUNTIME)
 +            for element in target_elements
 +        )
          profile_key = "_".join(t for t in targets)
          profile_start(Topics.CIRCULAR_CHECK, profile_key)
 -        self._check_circular_deps('')
 +        self._check_circular_deps(dummy_target)
          profile_end(Topics.CIRCULAR_CHECK, profile_key)
          ret = []
+         #
          # Sort direct dependencies of elements by their dependency ordering
+         #
 -        for target in targets:
 -            profile_start(Topics.SORT_DEPENDENCIES, target)
 -            junction, name, loader = self._parse_name(target, rewritable, ticker,
 -                                                      fetch_subprojects=fetch_subprojects)
 -            loader._sort_dependencies(name)
 -            profile_end(Topics.SORT_DEPENDENCIES, target)
 +        for element in target_elements:
 +            loader = element._loader
 +            profile_start(Topics.SORT_DEPENDENCIES, element.name)
 +            loader._sort_dependencies(element)
 +            profile_end(Topics.SORT_DEPENDENCIES, element.name)
              # Finally, wrap what we have into LoadElements and return the target
+             #
 -            ret.append(loader._collect_element(name))
 +            ret.append(loader._collect_element(element))
          return ret
@@ -184,22 +183,6 @@ class Loader():
              if os.path.exists(self._tempdir):
                  shutil.rmtree(self._tempdir)
 -    # get_element_for_dep():
 -    #
 -    # Gets a cached LoadElement by Dependency object
 -    #
 -    # This is used by LoadElement
 -    #
 -    # Args:
 -    #    dep (Dependency): The dependency to search for
 -    #
 -    # Returns:
 -    #    (LoadElement): The cached LoadElement
 -    #
 -    def get_element_for_dep(self, dep):
 -        loader = self._get_loader_for_dep(dep)
 -        return loader._elements[dep.name]
+-
      ###########################################
      #            Private Methods              #
      ###########################################
@@ -272,8 +255,10 @@ class Loader():
          self._elements[filename] = element
 +        dependencies = _extract_depends_from_node(node)
++
          # Load all dependency files for the new LoadElement
 -        for dep in element.deps:
 +        for dep in dependencies:
              if dep.junction:
                  self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
                  loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
@@ -288,7 +273,9 @@ class Loader():
                                  "{}: Cannot depend on junction"
                                  .format(dep.provenance))
 -        deps_names = [dep.name for dep in element.deps]
 +            element.dependencies.append(LoadElement.Dependency(dep_element, dep.dep_type))
++
 +        deps_names = [dep.name for dep in dependencies]
          self._warn_invalid_elements(deps_names)
          return element
@@ -299,12 +286,12 @@ class Loader():
      # dependencies already resolved.
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to check
 +    #    element (str): The element to check
+     #
      # Raises:
      #    (LoadError): In case there was a circular dependency error
+     #
 -    def _check_circular_deps(self, element_name, check_elements=None, validated=None, sequence=None):
 +    def _check_circular_deps(self, element, check_elements=None, validated=None, sequence=None):
          if check_elements is None:
              check_elements = {}
@@ -313,38 +300,31 @@ class Loader():
          if sequence is None:
              sequence = []
 -        element = self._elements[element_name]
+-
 -        # element name must be unique across projects
 -        # to be usable as key for the check_elements and validated dicts
 -        element_name = element.full_name
+-
          # Skip already validated branches
 -        if validated.get(element_name) is not None:
 +        if validated.get(element) is not None:
              return
 -        if check_elements.get(element_name) is not None:
 +        if check_elements.get(element) is not None:
              # Create `chain`, the loop of element dependencies from this
              # element back to itself, by trimming everything before this
              # element from the sequence under consideration.
 -            chain = sequence[sequence.index(element_name):]
 -            chain.append(element_name)
 +            chain = sequence[sequence.index(element.full_name):]
 +            chain.append(element.full_name)
              raise LoadError(LoadErrorReason.CIRCULAR_DEPENDENCY,
                              ("Circular dependency detected at element: {}\n" +
                               "Dependency chain: {}")
 -                            .format(element.name, " -> ".join(chain)))
 +                            .format(element.full_name, " -> ".join(chain)))
          # Push / Check each dependency / Pop
 -        check_elements[element_name] = True
 -        sequence.append(element_name)
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            loader._check_circular_deps(dep.name, check_elements, validated, sequence)
 -        del check_elements[element_name]
 +        check_elements[element] = True
 +        sequence.append(element.full_name)
 +        for dep in element.dependencies:
 +            dep.element._loader._check_circular_deps(dep.element, check_elements, validated, sequence)
 +        del check_elements[element]
          sequence.pop()
          # Eliminate duplicate paths
 -        validated[element_name] = True
 +        validated[element] = True
      # _sort_dependencies():
+     #
@@ -357,28 +337,21 @@ class Loader():
      # sorts throughout the build process.
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to sort
 +    #    element (LoadElement): The element to sort
+     #
 -    def _sort_dependencies(self, element_name, visited=None):
 +    def _sort_dependencies(self, element, visited=None):
          if visited is None:
 -            visited = {}
 +            visited = set()
 -        element = self._elements[element_name]
+-
 -        # element name must be unique across projects
 -        # to be usable as key for the visited dict
 -        element_name = element.full_name
+-
 -        if visited.get(element_name) is not None:
 +        if element in visited:
              return
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            loader._sort_dependencies(dep.name, visited=visited)
 +        for dep in element.dependencies:
 +            dep.element._loader._sort_dependencies(dep.element, visited=visited)
          def dependency_cmp(dep_a, dep_b):
 -            element_a = self.get_element_for_dep(dep_a)
 -            element_b = self.get_element_for_dep(dep_b)
 +            element_a = dep_a.element
 +            element_b = dep_b.element
              # Sort on inter element dependency first
              if element_a.depends(element_b):
@@ -395,21 +368,21 @@ class Loader():
                      return -1
              # All things being equal, string comparison.
 -            if dep_a.name > dep_b.name:
 +            if element_a.name > element_b.name:
                  return 1
 -            elif dep_a.name < dep_b.name:
 +            elif element_a.name < element_b.name:
                  return -1
              # Sort local elements before junction elements
              # and use string comparison between junction elements
 -            if dep_a.junction and dep_b.junction:
 -                if dep_a.junction > dep_b.junction:
 +            if element_a.junction and element_b.junction:
 +                if element_a.junction > element_b.junction:
                      return 1
 -                elif dep_a.junction < dep_b.junction:
 +                elif element_a.junction < element_b.junction:
                      return -1
 -            elif dep_a.junction:
 +            elif element_a.junction:
                  return -1
 -            elif dep_b.junction:
 +            elif element_b.junction:
                  return 1
              # This wont ever happen
@@ -418,26 +391,23 @@ class Loader():
          # Now dependency sort, we ensure that if any direct dependency
          # directly or indirectly depends on another direct dependency,
          # it is found later in the list.
 -        element.deps.sort(key=cmp_to_key(dependency_cmp))
 +        element.dependencies.sort(key=cmp_to_key(dependency_cmp))
 -        visited[element_name] = True
 +        visited.add(element)
      # _collect_element()
+     #
      # Collect the toplevel elements we have
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to sort
 +    #    element (LoadElement): The element for which to load a MetaElement
+     #
      # Returns:
      #    (MetaElement): A recursively loaded MetaElement
+     #
 -    def _collect_element(self, element_name):
+-
 -        element = self._elements[element_name]
+-
 +    def _collect_element(self, element):
          # Return the already built one, if we already built it
 -        meta_element = self._meta_elements.get(element_name)
 +        meta_element = self._meta_elements.get(element.name)
          if meta_element:
              return meta_element
@@ -461,10 +431,10 @@ class Loader():
                  del source[Symbol.DIRECTORY]
              index = sources.index(source)
 -            meta_source = MetaSource(element_name, index, element_kind, kind, source, directory)
 +            meta_source = MetaSource(element.name, index, element_kind, kind, source, directory)
              meta_sources.append(meta_source)
 -        meta_element = MetaElement(self.project, element_name, element_kind,
 +        meta_element = MetaElement(self.project, element.name, element_kind,
                                     elt_provenance, meta_sources,
                                     _yaml.node_get(node, Mapping, Symbol.CONFIG, default_value={}),
                                     _yaml.node_get(node, Mapping, Symbol.VARIABLES, default_value={}),
@@ -475,12 +445,12 @@ class Loader():
                                     element_kind == 'junction')
          # Cache it now, make sure it's already there before recursing
 -        self._meta_elements[element_name] = meta_element
 +        self._meta_elements[element.name] = meta_element
          # Descend
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            meta_dep = loader._collect_element(dep.name)
 +        for dep in element.dependencies:
 +            loader = dep.element._loader
 +            meta_dep = loader._collect_element(dep.element)
              if dep.dep_type != 'runtime':
                  meta_element.build_dependencies.append(meta_dep)
              if dep.dep_type != 'build':
@@ -539,7 +509,7 @@ class Loader():
                  return None
          # meta junction element
 -        meta_element = self._collect_element(filename)
 +        meta_element = self._collect_element(self._elements[filename])
          if meta_element.kind != 'junction':
              raise LoadError(LoadErrorReason.INVALID_DATA,
                              "{}: Expected junction but element kind is {}".format(filename, meta_element.kind))
@@ -601,23 +571,6 @@ class Loader():
          return loader
 -    # _get_loader_for_dep():
 -    #
 -    # Gets the appropriate Loader for a Dependency object
 -    #
 -    # Args:
 -    #    dep (Dependency): A Dependency object
 -    #
 -    # Returns:
 -    #    (Loader): The Loader object to use for this Dependency
 -    #
 -    def _get_loader_for_dep(self, dep):
 -        if dep.junction:
 -            # junction dependency, delegate to appropriate loader
 -            return self._loaders[dep.junction]
 -        else:
 -            return self
+-
      # _parse_name():
+     #
      # Get junction and base name of element along with loader for the sub-project

buildstream/_project.py

@@ -549,7 +549,15 @@ class Project():
+         #
          # Load artifacts pull/push configuration for this project
 -        self.artifact_cache_specs = ArtifactCache.specs_from_config_node(config, self.directory)
 +        project_specs = ArtifactCache.specs_from_config_node(config, self.directory)
 +        override_specs = ArtifactCache.specs_from_config_node(
 +            self._context.get_overrides(self.name), self.directory)
++
 +        self.artifact_cache_specs = override_specs + project_specs
++
 +        if self.junction:
 +            parent = self.junction._get_project()
 +            self.artifact_cache_specs = parent.artifact_cache_specs + self.artifact_cache_specs
          # Load remote-execution configuration for this project
          project_specs = SandboxRemote.specs_from_config_node(config, self.directory)

buildstream/_scheduler/scheduler.py

@@ -314,10 +314,10 @@ class Scheduler():
      #    job (Job): The job to spawn
+     #
      def _spawn_job(self, job):
 -        job.spawn()
          self._active_jobs.append(job)
          if self._job_start_callback:
              self._job_start_callback(job)
 +        job.spawn()
      # Callback for the cache size job
      def _cache_size_job_complete(self, status, cache_size):

buildstream/data/userconfig.yaml

@@ -112,14 +112,6 @@ logging:
+ #
  prompt:
 -  # Whether to really proceed with 'bst workspace close --remove-dir' removing
 -  # a workspace directory, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always remove, without asking.
 -  #
 -  really-workspace-close-remove-dir: ask
+-
    # Whether to really proceed with 'bst workspace close' when doing so would
    # stop them from running bst commands in this workspace.
+   #
@@ -127,11 +119,3 @@ prompt:
    #  yes - Always close, without asking.
+   #
    really-workspace-close-project-inaccessible: ask
+-
 -  # Whether to really proceed with 'bst workspace reset' doing a hard reset of
 -  # a workspace, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always hard reset, without asking.
 -  #
 -  really-workspace-reset-hard: ask

buildstream/plugins/elements/filter.py

@@ -47,6 +47,8 @@ from buildstream import Element, ElementError, Scope
  class FilterElement(Element):
      # pylint: disable=attribute-defined-outside-init
 +    BST_ARTIFACT_VERSION = 1
++
      # The filter element's output is its dependencies, so
      # we must rebuild if the dependencies change even when
      # not in strict build plans.
@@ -102,7 +104,7 @@ class FilterElement(Element):
      def assemble(self, sandbox):
          with self.timed_activity("Staging artifact", silent_nested=True):
 -            for dep in self.dependencies(Scope.BUILD):
 +            for dep in self.dependencies(Scope.BUILD, recurse=False):
                  dep.stage_artifact(sandbox, include=self.include,
                                     exclude=self.exclude, orphans=self.include_orphans)
          return ""

conftest.py

@@ -54,6 +54,7 @@ class IntegrationCache():
      def __init__(self, cache):
          cache = os.path.abspath(cache)
 +        os.makedirs(cache, exist_ok=True)
          # Use the same sources every time
          self.sources = os.path.join(cache, 'sources')

requirements/requirements.in

 -Click
 +Click >= 7.0
  grpcio >= 1.10
  Jinja2 >= 2.10
  pluginbase
 -protobuf >= 3.5
 +protobuf >= 3.6
  psutil
  # According to ruamel.yaml's PyPI page, we are suppose to use
  # "<=0.15" in production until 0.15 becomes API stable.

tests/cachekey/cachekey.py

@@ -214,3 +214,41 @@ def test_cache_key_fatal_warnings(cli, tmpdir, first_warnings, second_warnings,
      second_keys = run_get_cache_key("second", second_warnings)
      assert compare_cache_keys(first_keys, second_keys) == identical_keys
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_keys_stable_over_targets(cli, datafiles):
 +    root_element = 'elements/key-stability/top-level.bst'
 +    target1 = 'elements/key-stability/t1.bst'
 +    target2 = 'elements/key-stability/t2.bst'
++
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    full_graph_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        root_element
 +    ])
 +    full_graph_result.assert_success()
 +    all_cache_keys = parse_output_keys(full_graph_result.output)
++
 +    ordering1_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        target1,
 +        target2
 +    ])
 +    ordering1_result.assert_success()
 +    ordering1_cache_keys = parse_output_keys(ordering1_result.output)
++
 +    ordering2_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        target2,
 +        target1
 +    ])
 +    ordering2_result.assert_success()
 +    ordering2_cache_keys = parse_output_keys(ordering2_result.output)
++
 +    for element in ordering1_cache_keys:
 +        assert ordering1_cache_keys[element] == ordering2_cache_keys[element]
 +        assert ordering1_cache_keys[element] == all_cache_keys[element]

tests/cachekey/project/elements/key-stability/aaa.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/aaa.bst

tests/cachekey/project/elements/key-stability/t1.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/t1.bst
 +depends:
 +- elements/key-stability/zzz.bst

tests/cachekey/project/elements/key-stability/t2.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/t2.bst
 +depends:
 +- elements/key-stability/aaa.bst
 +- elements/key-stability/zzz.bst

tests/cachekey/project/elements/key-stability/top-level.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/top-level.bst
 +depends:
 +- elements/key-stability/t1.bst
 +- elements/key-stability/t2.bst

tests/cachekey/project/elements/key-stability/zzz.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/zzz.bst

tests/elements/filter.py

@@ -464,3 +464,23 @@ def test_filter_track_multi_exclude(datafiles, cli, tmpdir):
      assert "ref" not in new_input["sources"][0]
      new_input2 = _yaml.load(input2_file)
      assert new_input2["sources"][0]["ref"] == ref
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'basic'))
 +def test_filter_include_with_indirect_deps(datafiles, cli, tmpdir):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    result = cli.run(project=project, args=[
 +        'build', 'output-include-with-indirect-deps.bst'])
 +    result.assert_success()
++
 +    checkout = os.path.join(tmpdir.dirname, tmpdir.basename, 'checkout')
 +    result = cli.run(project=project, args=[
 +        'artifact', 'checkout', 'output-include-with-indirect-deps.bst', '--directory', checkout])
 +    result.assert_success()
++
 +    # direct dependencies should be staged and filtered
 +    assert os.path.exists(os.path.join(checkout, "baz"))
++
 +    # indirect dependencies shouldn't be staged and filtered
 +    assert not os.path.exists(os.path.join(checkout, "foo"))
 +    assert not os.path.exists(os.path.join(checkout, "bar"))

tests/elements/filter/basic/elements/input-with-deps.bst

 +kind: import
++
 +depends:
 +- filename: input.bst
++
 +sources:
 +- kind: local
 +  path: files
++
 +public:
 +  bst:
 +    split-rules:
 +      baz:
 +      - /baz

tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst

 +kind: filter
++
 +depends:
 +- filename: input-with-deps.bst
 +  type: build

tests/testutils/site.py

@@ -18,7 +18,7 @@ try:
      utils.get_host_tool('git')
      HAVE_GIT = True
      out = str(subprocess.check_output(['git', '--version']), "utf-8")
 -    version = tuple(int(x) for x in out.split(' ', 2)[2].split('.'))
 +    version = tuple(int(x) for x in out.split(' ')[2].split('.'))
      HAVE_OLD_GIT = version < (1, 8, 5)
  except ProgramNotFoundError:
      HAVE_GIT = False

tox.ini

@@ -88,5 +88,5 @@ whitelist_externals =
  commands =
      python3 setup.py --command-packages=click_man.commands man_pages
  deps =
 -    click-man
 +    click-man >= 0.3.0
      -rrequirements/requirements.txt

[Notes] [Git][BuildStream/buildstream][abderrahim/artifact-cache-junction] 24 commits: requirements/requirements.in: require protobuf >= 3.6

Abderrahim Kitouni pushed to branch abderrahim/artifact-cache-junction at BuildStream / buildstream

Commits:

24 changed files:

Changes: