This is an automated email from the ASF dual-hosted git repository. juergbi pushed a commit to branch juerg/remote-cache-ci in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit d20ed3d87862c6ad8d1568131a85f563d3c778fd Author: Jürg Billeter <[email protected]> AuthorDate: Mon Oct 26 18:26:17 2020 +0100 Always require artifact files In line with the previous commit, also require artifact files for build dependencies with remote execution. Local artifact files will become optional with the upcoming remote cache support. --- src/buildstream/_artifact.py | 13 +------ src/buildstream/_context.py | 15 --------- src/buildstream/_stream.py | 9 ----- src/buildstream/element.py | 29 ---------------- tests/remoteexecution/partial.py | 73 ---------------------------------------- 5 files changed, 1 insertion(+), 138 deletions(-) diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py index 51e1f57..98f8250 100644 --- a/src/buildstream/_artifact.py +++ b/src/buildstream/_artifact.py @@ -551,24 +551,13 @@ class Artifact: # (bool): Whether artifact is in local cache # def query_cache(self): - context = self._context - artifact = self._load_proto() if not artifact: self._cached = False return False - # Determine whether directories are required - require_directories = context.require_artifact_directories - # Determine whether file contents are required as well - require_files = context.require_artifact_files or self._element._artifact_files_required() - # Check whether 'files' subdirectory is available, with or without file contents - if ( - require_directories - and str(artifact.files) - and not self._cas.contains_directory(artifact.files, with_files=require_files) - ): + if str(artifact.files) and not self._cas.contains_directory(artifact.files, with_files=True): self._cached = False return False diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py index 2c1f25a..d5d0669 100644 --- a/src/buildstream/_context.py +++ b/src/buildstream/_context.py @@ -181,12 +181,6 @@ class Context: # Whether or not to cache build trees on artifact creation self.cache_buildtrees: Optional[str] = None - # Whether directory trees are required for all artifacts in the local cache - self.require_artifact_directories: bool = True - - # Whether file contents are required for all artifacts in the local cache - self.require_artifact_files: bool = True - # Don't shoot the messenger self.messenger: Messenger = Messenger() @@ -654,15 +648,6 @@ class Context: # value which we cache here too. return self._strict_build_plan - # set_artifact_files_optional() - # - # This indicates that the current context (command or configuration) - # does not require file contents of all artifacts to be available in the - # local cache. - # - def set_artifact_files_optional(self) -> None: - self.require_artifact_files = False - def get_cascache(self) -> CASCache: if self._cascache is None: if self.log_debug: diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index 0559d8d..e22b93c 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -378,15 +378,6 @@ class Stream: # Assert that the elements are consistent _pipeline.assert_consistent(self._context, elements) - if self._context.remote_execution_specs: - # Remote execution is configured. - # Require artifact files only for target elements and their runtime dependencies. - self._context.set_artifact_files_optional() - - scope = _Scope.ALL if selection == _PipelineSelection.ALL else _Scope.RUN - for element in self.targets: - element._set_artifact_files_required(scope=scope) - source_push_enabled = self._sourcecache.has_push_remotes() # If source push is enabled, the source cache status of all elements diff --git a/src/buildstream/element.py b/src/buildstream/element.py index bba3506..38240bb 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -296,7 +296,6 @@ class Element(Plugin): self.__whitelist_regex = None # Resolved regex object to check if file is allowed to overlap self.__tainted = None # Whether the artifact is tainted and should not be shared self.__required = False # Whether the artifact is required in the current session - self.__artifact_files_required = False # Whether artifact files are required in the local cache self.__build_result = None # The result of assembling this Element (success, description, detail) # Artifact class for direct artifact composite interaction self.__artifact = None # type: Optional[Artifact] @@ -1555,31 +1554,6 @@ class Element(Plugin): def _is_required(self): return self.__required - # _set_artifact_files_required(): - # - # Mark artifact files for this element and its runtime dependencies as - # required in the local cache. - # - def _set_artifact_files_required(self, scope=_Scope.RUN): - assert utils._is_in_main_thread(), "This has an impact on all elements and must be run in the main thread" - - if self.__artifact_files_required: - # Already done - return - - self.__artifact_files_required = True - - # Request artifact files of runtime dependencies - for dep in self._dependencies(scope, recurse=False): - dep._set_artifact_files_required(scope=scope) - - # _artifact_files_required(): - # - # Returns whether artifact files for this element have been marked as required. - # - def _artifact_files_required(self): - return self.__artifact_files_required - # __should_schedule() # # Returns: @@ -2797,8 +2771,6 @@ class Element(Plugin): self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory)) - output_files_required = context.require_artifact_files or self._artifact_files_required() - sandbox = SandboxRemote( context, project, @@ -2807,7 +2779,6 @@ class Element(Plugin): stdout=stdout, stderr=stderr, config=config, - output_files_required=output_files_required, output_node_properties=output_node_properties, ) yield sandbox diff --git a/tests/remoteexecution/partial.py b/tests/remoteexecution/partial.py deleted file mode 100644 index cd87293..0000000 --- a/tests/remoteexecution/partial.py +++ /dev/null @@ -1,73 +0,0 @@ -# Pylint doesn't play well with fixtures and dependency injection from pytest -# pylint: disable=redefined-outer-name - -import os -import pytest - -from buildstream.exceptions import ErrorDomain -from buildstream.testing import cli_remote_execution as cli # pylint: disable=unused-import -from buildstream.testing.integration import assert_contains - -from tests.testutils.artifactshare import create_artifact_share - - -pytestmark = pytest.mark.remoteexecution - - -DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project") - - -# Test that `bst build` does not download file blobs of a build-only dependency -# to the local cache. [email protected](DATA_DIR) [email protected]("build_all", [True, False]) -def test_build_dependency_partial_local_cas(cli, datafiles, build_all): - project = str(datafiles) - element_name = "no-runtime-deps.bst" - builddep_element_name = "autotools/amhello.bst" - checkout = os.path.join(cli.directory, "checkout") - builddep_checkout = os.path.join(cli.directory, "builddep-checkout") - - services = cli.ensure_services() - assert set(services) == set(["action-cache", "execution", "storage"]) - - # configure pull blobs - if build_all: - cli.configure({"build": {"dependencies": "all"}}) - - result = cli.run(project=project, args=["build", element_name]) - result.assert_success() - - # Verify artifact is pulled bar files when ensure artifact files is set - result = cli.run(project=project, args=["artifact", "checkout", element_name, "--directory", checkout]) - result.assert_success() - assert_contains(checkout, ["/test"]) - - # Verify build dependencies is pulled for ALL and BUILD - result = cli.run( - project=project, args=["artifact", "checkout", builddep_element_name, "--directory", builddep_checkout] - ) - if build_all: - result.assert_success() - else: - result.assert_main_error(ErrorDomain.STREAM, "uncached-checkout-attempt") - - [email protected](DATA_DIR) -def test_build_partial_push(cli, tmpdir, datafiles): - project = str(datafiles) - share_dir = os.path.join(str(tmpdir), "artifactshare") - element_name = "no-runtime-deps.bst" - builddep_element_name = "autotools/amhello.bst" - - with create_artifact_share(share_dir) as share: - - services = cli.ensure_services() - assert set(services) == set(["action-cache", "execution", "storage"]) - - cli.config["artifacts"] = {"servers": [{"url": share.repo, "push": True,}]} - - res = cli.run(project=project, args=["build", element_name]) - res.assert_success() - - assert builddep_element_name in res.get_pushed_elements()
