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.
diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index 28e2f59..163e2ae 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -551,24 +551,13 @@
# (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 @@
# 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 @@
# 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 @@
# 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 @@
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 @@
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 @@
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 @@
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.
-@pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("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")
-
-
-@pytest.mark.datafiles(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()
