From ba6c96e863ee7460f598d51869522a43be7d94c3 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Wed, 14 Nov 2018 16:56:24 +0000 Subject: CasBasedDirectory: Change constructor to take a CASCache instead of a Context The Context was only used to obtain a reference to the CASCache and set the unused cas_directory field. --- buildstream/sandbox/_sandboxremote.py | 4 ++-- buildstream/sandbox/sandbox.py | 2 +- buildstream/storage/_casbaseddirectory.py | 10 ++++------ tests/artifactcache/push.py | 2 +- tests/sandboxes/storage-tests.py | 7 +++---- tests/storage/virtual_directory_import.py | 30 ++++++++---------------------- 6 files changed, 19 insertions(+), 36 deletions(-) diff --git a/buildstream/sandbox/_sandboxremote.py b/buildstream/sandbox/_sandboxremote.py index e27661233..389a8fec0 100644 --- a/buildstream/sandbox/_sandboxremote.py +++ b/buildstream/sandbox/_sandboxremote.py @@ -182,7 +182,7 @@ class SandboxRemote(Sandbox): # to replace the sandbox's virtual directory with that. Creating a new virtual directory object # from another hash will be interesting, though... - new_dir = CasBasedDirectory(self._get_context(), ref=dir_digest) + new_dir = CasBasedDirectory(self._get_context().artifactcache.cas, ref=dir_digest) self._set_virtual_directory(new_dir) def run(self, command, flags, *, cwd=None, env=None): @@ -191,7 +191,7 @@ class SandboxRemote(Sandbox): if isinstance(upload_vdir, FileBasedDirectory): # Make a new temporary directory to put source in - upload_vdir = CasBasedDirectory(self._get_context(), ref=None) + upload_vdir = CasBasedDirectory(self._get_context().artifactcache.cas, ref=None) upload_vdir.import_files(self.get_virtual_directory()._get_underlying_directory()) upload_vdir.recalculate_hash() diff --git a/buildstream/sandbox/sandbox.py b/buildstream/sandbox/sandbox.py index eabe57d75..a76b2d9d2 100644 --- a/buildstream/sandbox/sandbox.py +++ b/buildstream/sandbox/sandbox.py @@ -156,7 +156,7 @@ class Sandbox(): """ if self._vdir is None or self._never_cache_vdirs: if 'BST_CAS_DIRECTORIES' in os.environ: - self._vdir = CasBasedDirectory(self.__context, ref=None) + self._vdir = CasBasedDirectory(self.__context.artifactcache.cas, ref=None) else: self._vdir = FileBasedDirectory(self._root) return self._vdir diff --git a/buildstream/storage/_casbaseddirectory.py b/buildstream/storage/_casbaseddirectory.py index 700257139..c8a068d21 100644 --- a/buildstream/storage/_casbaseddirectory.py +++ b/buildstream/storage/_casbaseddirectory.py @@ -249,13 +249,11 @@ class CasBasedDirectory(Directory): _pb2_path_sep = "/" _pb2_absolute_path_prefix = "/" - def __init__(self, context, ref=None, parent=None, common_name="untitled", filename=None): - self.context = context - self.cas_directory = os.path.join(context.artifactdir, 'cas') + def __init__(self, cas_cache, ref=None, parent=None, common_name="untitled", filename=None): self.filename = filename self.common_name = common_name self.pb2_directory = remote_execution_pb2.Directory() - self.cas_cache = context.artifactcache.cas + self.cas_cache = cas_cache if ref: with open(self.cas_cache.objpath(ref), 'rb') as f: self.pb2_directory.ParseFromString(f.read()) @@ -270,7 +268,7 @@ class CasBasedDirectory(Directory): if self._directory_read: return for entry in self.pb2_directory.directories: - buildStreamDirectory = CasBasedDirectory(self.context, ref=entry.digest, + buildStreamDirectory = CasBasedDirectory(self.cas_cache, ref=entry.digest, parent=self, filename=entry.name) self.index[entry.name] = IndexEntry(entry, buildstream_object=buildStreamDirectory) for entry in self.pb2_directory.files: @@ -333,7 +331,7 @@ class CasBasedDirectory(Directory): .format(name, str(self), type(newdir))) dirnode = self._find_pb2_entry(name) else: - newdir = CasBasedDirectory(self.context, parent=self, filename=name) + newdir = CasBasedDirectory(self.cas_cache, parent=self, filename=name) dirnode = self.pb2_directory.directories.add() dirnode.name = name diff --git a/tests/artifactcache/push.py b/tests/artifactcache/push.py index be6293cf4..746884e29 100644 --- a/tests/artifactcache/push.py +++ b/tests/artifactcache/push.py @@ -225,7 +225,7 @@ def _test_push_directory(user_config_file, project_dir, artifact_dir, artifact_d if cas.has_push_remotes(): # Create a CasBasedDirectory from local CAS cache content - directory = CasBasedDirectory(context, ref=artifact_digest) + directory = CasBasedDirectory(context.artifactcache.cas, ref=artifact_digest) # Push the CasBasedDirectory object cas.push_directory(project, directory) diff --git a/tests/sandboxes/storage-tests.py b/tests/sandboxes/storage-tests.py index 553da2ba7..e646a620a 100644 --- a/tests/sandboxes/storage-tests.py +++ b/tests/sandboxes/storage-tests.py @@ -3,7 +3,7 @@ import pytest from buildstream._exceptions import ErrorDomain -from buildstream._context import Context +from buildstream._artifactcache.cascache import CASCache from buildstream.storage._casbaseddirectory import CasBasedDirectory from buildstream.storage._filebaseddirectory import FileBasedDirectory @@ -17,9 +17,8 @@ def setup_backend(backend_class, tmpdir): if backend_class == FileBasedDirectory: return backend_class(os.path.join(tmpdir, "vdir")) else: - context = Context() - context.artifactdir = os.path.join(tmpdir, "cas") - return backend_class(context) + cas_cache = CASCache(tmpdir) + return backend_class(cas_cache) @pytest.mark.parametrize("backend", [ diff --git a/tests/storage/virtual_directory_import.py b/tests/storage/virtual_directory_import.py index 3732f92d9..fa40b1723 100644 --- a/tests/storage/virtual_directory_import.py +++ b/tests/storage/virtual_directory_import.py @@ -15,18 +15,6 @@ from buildstream import utils # These are comparitive tests that check that FileBasedDirectory and # CasBasedDirectory act identically. - -class FakeArtifactCache(): - def __init__(self): - self.cas = None - - -class FakeContext(): - def __init__(self): - self.artifactdir = '' - self.artifactcache = FakeArtifactCache() - - # This is a set of example file system contents. It's a set of trees # which are either expected to be problematic or were found to be # problematic during random testing. @@ -120,8 +108,8 @@ def file_contents_are(path, contents): return file_contents(path) == contents -def create_new_casdir(root_number, fake_context, tmpdir): - d = CasBasedDirectory(fake_context) +def create_new_casdir(root_number, cas_cache, tmpdir): + d = CasBasedDirectory(cas_cache) d.import_files(os.path.join(tmpdir, "content", "root{}".format(root_number))) assert d.ref.hash != empty_hash_ref return d @@ -175,20 +163,19 @@ def directory_not_empty(path): def _import_test(tmpdir, original, overlay, generator_function, verify_contents=False): - fake_context = FakeContext() - fake_context.artifactcache.cas = CASCache(tmpdir) + cas_cache = CASCache(tmpdir) # Create some fake content generator_function(original, tmpdir) if original != overlay: generator_function(overlay, tmpdir) - d = create_new_casdir(original, fake_context, tmpdir) + d = create_new_casdir(original, cas_cache, tmpdir) - duplicate_cas = create_new_casdir(original, fake_context, tmpdir) + duplicate_cas = create_new_casdir(original, cas_cache, tmpdir) assert duplicate_cas.ref.hash == d.ref.hash - d2 = create_new_casdir(overlay, fake_context, tmpdir) + d2 = create_new_casdir(overlay, cas_cache, tmpdir) d.import_files(d2) export_dir = os.path.join(tmpdir, "output-{}-{}".format(original, overlay)) roundtrip_dir = os.path.join(tmpdir, "roundtrip-{}-{}".format(original, overlay)) @@ -247,15 +234,14 @@ def test_random_cas_import(cli, tmpdir, original): def _listing_test(tmpdir, root, generator_function): - fake_context = FakeContext() - fake_context.artifactcache.cas = CASCache(tmpdir) + cas_cache = CASCache(tmpdir) # Create some fake content generator_function(root, tmpdir) d = create_new_filedir(root, tmpdir) filelist = list(d.list_relative_paths()) - d2 = create_new_casdir(root, fake_context, tmpdir) + d2 = create_new_casdir(root, cas_cache, tmpdir) filelist2 = list(d2.list_relative_paths()) assert filelist == filelist2 -- cgit v1.2.1 From 8a0dc3a3256207e308bc9406a59ba076208869ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Thu, 15 Nov 2018 15:25:44 +0100 Subject: tests/testutils/artifactshare.py: Do not create a fake context The fake context did not set the cache quota, triggering an error. With CASCache now separate from ArtifactCache, we can instantiate a CASCache without context. --- tests/testutils/artifactshare.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py index 91dcb9bdb..5194e83cf 100644 --- a/tests/testutils/artifactshare.py +++ b/tests/testutils/artifactshare.py @@ -11,8 +11,8 @@ from multiprocessing import Process, Queue import pytest_cov from buildstream import _yaml +from buildstream._artifactcache.cascache import CASCache from buildstream._artifactcache.casserver import create_server -from buildstream._context import Context from buildstream._exceptions import CASError from buildstream._protos.build.bazel.remote.execution.v2 import remote_execution_pb2 @@ -45,10 +45,7 @@ class ArtifactShare(): os.makedirs(self.repodir) - context = Context() - context.artifactdir = self.repodir - - self.cas = context.artifactcache.cas + self.cas = CASCache(self.repodir) self.total_space = total_space self.free_space = free_space -- cgit v1.2.1 From 8722aced843a704d713fc6d0e35ce8be52735354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Thu, 15 Nov 2018 15:43:17 +0100 Subject: tests/plugins/pipeline.py: Load context default values --- tests/plugins/pipeline.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/plugins/pipeline.py b/tests/plugins/pipeline.py index 65929cf50..6cba7a125 100644 --- a/tests/plugins/pipeline.py +++ b/tests/plugins/pipeline.py @@ -14,9 +14,10 @@ DATA_DIR = os.path.join( def create_pipeline(tmpdir, basedir, target): context = Context() - project = Project(basedir, context) + context.load() context.deploydir = os.path.join(str(tmpdir), 'deploy') context.artifactdir = os.path.join(str(tmpdir), 'artifact') + project = Project(basedir, context) def dummy_handler(message, context): pass -- cgit v1.2.1 From fc56ffa4bbb0d406f8b103f7dfeaaf54fd903b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Thu, 15 Nov 2018 14:18:54 +0100 Subject: _context.py: Drop duplicated default values for user configuration The default values are in userconfig.yaml, together with the documentation. The default values should not be duplicated in _context.py. --- buildstream/_context.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/buildstream/_context.py b/buildstream/_context.py index 960f37160..b39c82b1d 100644 --- a/buildstream/_context.py +++ b/buildstream/_context.py @@ -63,25 +63,25 @@ class Context(): self.artifactdir = None # The locations from which to push and pull prebuilt artifacts - self.artifact_cache_specs = [] + self.artifact_cache_specs = None # The directory to store build logs self.logdir = None # The abbreviated cache key length to display in the UI - self.log_key_length = 0 + self.log_key_length = None # Whether debug mode is enabled - self.log_debug = False + self.log_debug = None # Whether verbose mode is enabled - self.log_verbose = False + self.log_verbose = None # Maximum number of lines to print from build logs - self.log_error_lines = 0 + self.log_error_lines = None # Maximum number of lines to print in the master log for a detailed message - self.log_message_lines = 0 + self.log_message_lines = None # Format string for printing the pipeline at startup time self.log_element_format = None @@ -90,19 +90,22 @@ class Context(): self.log_message_format = None # Maximum number of fetch or refresh tasks - self.sched_fetchers = 4 + self.sched_fetchers = None # Maximum number of build tasks - self.sched_builders = 4 + self.sched_builders = None # Maximum number of push tasks - self.sched_pushers = 4 + self.sched_pushers = None # Maximum number of retries for network tasks - self.sched_network_retries = 2 + self.sched_network_retries = None # What to do when a build fails in non interactive mode - self.sched_error_action = 'continue' + self.sched_error_action = None + + # Size of the artifact cache in bytes + self.config_cache_quota = None # Whether or not to attempt to pull build trees globally self.pull_buildtrees = None @@ -123,7 +126,6 @@ class Context(): self._workspaces = None self._log_handle = None self._log_filename = None - self.config_cache_quota = 'infinity' # load() # @@ -183,7 +185,7 @@ class Context(): cache = _yaml.node_get(defaults, Mapping, 'cache') _yaml.node_validate(cache, ['quota', 'pull-buildtrees']) - self.config_cache_quota = _yaml.node_get(cache, str, 'quota', default_value='infinity') + self.config_cache_quota = _yaml.node_get(cache, str, 'quota') # Load artifact share configuration self.artifact_cache_specs = ArtifactCache.specs_from_config_node(defaults) -- cgit v1.2.1