diff options
author | Jürg Billeter <j@bitron.ch> | 2019-03-14 10:40:56 +0000 |
---|---|---|
committer | Jürg Billeter <j@bitron.ch> | 2019-03-16 07:24:58 +0100 |
commit | 142ef9cdc6ed3c5b0e91ca450dfcd608822e0a60 (patch) | |
tree | cf803c726ddd833f52fc333d17d340e7c82fb991 | |
parent | 3dc95d4ba2bf6c236bc69f35c098f563ab0f65aa (diff) | |
download | buildstream-142ef9cdc6ed3c5b0e91ca450dfcd608822e0a60.tar.gz |
_artifact.py: Do not create empty buildtree directories
Creating an empty buildtree directory when buildtree caching is disabled
means cached_buildtree() can return True even if no buildtree is
available, which breaks logic in the frontend and in Stream.
This adds the buildtree_exists() method to indicate whether an artifact
was created with a buildtree, independent of whether the buildtree is
available in the local cache.
-rw-r--r-- | buildstream/_artifact.py | 18 | ||||
-rw-r--r-- | buildstream/_frontend/cli.py | 5 | ||||
-rw-r--r-- | buildstream/_stream.py | 13 | ||||
-rw-r--r-- | buildstream/element.py | 13 | ||||
-rw-r--r-- | tests/integration/artifact.py | 14 | ||||
-rw-r--r-- | tests/integration/shellbuildtrees.py | 8 |
6 files changed, 50 insertions, 21 deletions
diff --git a/buildstream/_artifact.py b/buildstream/_artifact.py index 0c1e71194..a2600b995 100644 --- a/buildstream/_artifact.py +++ b/buildstream/_artifact.py @@ -128,7 +128,6 @@ class Artifact(): assemblevdir = CasBasedDirectory(cas_cache=self._artifacts.cas) logsvdir = assemblevdir.descend("logs", create=True) metavdir = assemblevdir.descend("meta", create=True) - buildtreevdir = assemblevdir.descend("buildtree", create=True) # Create artifact directory structure assembledir = os.path.join(rootdir, 'artifact') @@ -150,6 +149,7 @@ class Artifact(): # there will not be an applicable sandbox_build_dir. if sandbox_build_dir: + buildtreevdir = assemblevdir.descend("buildtree", create=True) buildtreevdir.import_files(sandbox_build_dir) # Write some logs out to normal directories: logsdir and metadir @@ -227,6 +227,22 @@ class Artifact(): return True + # buildtree_exists() + # + # Check if artifact was created with a buildtree. This does not check + # whether the buildtree is present in the local cache. + # + # Returns: + # (bool): True if artifact was created with buildtree + # + def buildtree_exists(self): + + if not self._element._cached(): + return False + + artifact_vdir, _ = self._get_directory() + return artifact_vdir._exists('buildtree') + # load_public_data(): # # Loads the public data from the cached artifact diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py index 74a529934..4974e2bfc 100644 --- a/buildstream/_frontend/cli.py +++ b/buildstream/_frontend/cli.py @@ -555,9 +555,10 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) ] cached = element._cached_buildtree() + buildtree_exists = element._buildtree_exists() if cli_buildtree in ("always", "try"): use_buildtree = cli_buildtree - if not cached and use_buildtree == "always": + if not cached and buildtree_exists and use_buildtree == "always": click.echo("WARNING: buildtree is not cached locally, will attempt to pull from available remotes", err=True) else: @@ -566,7 +567,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) if app.interactive and cli_buildtree == "ask": if cached and bool(click.confirm('Do you want to use the cached buildtree?')): use_buildtree = "always" - elif not cached: + elif buildtree_exists: try: choice = click.prompt("Do you want to pull & use a cached buildtree?", type=click.Choice(['try', 'always', 'never']), diff --git a/buildstream/_stream.py b/buildstream/_stream.py index 4a475d83f..4a21f5002 100644 --- a/buildstream/_stream.py +++ b/buildstream/_stream.py @@ -180,12 +180,17 @@ class Stream(): # Now check if the buildtree was successfully fetched if element._cached_buildtree(): buildtree = True + if not buildtree: + if element._buildtree_exists(): + message = "Buildtree is not cached locally or in available remotes" + else: + message = "Artifact was created without buildtree" + if usebuildtree == "always": - raise StreamError("Buildtree is not cached locally or in available remotes") + raise StreamError(message) else: - self._message(MessageType.INFO, "Buildtree is not cached locally or in available remotes, " + - "shell will be loaded without it") + self._message(MessageType.INFO, message + ", shell will be loaded without it") else: buildtree = True @@ -1420,7 +1425,7 @@ class Stream(): for element in elements: # Check if element is partially cached without its buildtree, as the element # artifact may not be cached at all - if element._cached() and not element._cached_buildtree(): + if element._cached() and not element._cached_buildtree() and element._buildtree_exists(): required_list.append(element) return required_list diff --git a/buildstream/element.py b/buildstream/element.py index 9ec65edaa..43bbd6939 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -1845,7 +1845,7 @@ class Element(Plugin): # Do not push elements that aren't cached, or that are cached with a dangling buildtree # ref unless element type is expected to have an an empty buildtree directory - if not self._cached_buildtree(): + if not self._cached_buildtree() and self._buildtree_exists(): return True # Do not push tainted artifact @@ -2056,6 +2056,17 @@ class Element(Plugin): def _cached_buildtree(self): return self.__artifact.cached_buildtree() + # _buildtree_exists() + # + # Check if artifact was created with a buildtree. This does not check + # whether the buildtree is present in the local cache. + # + # Returns: + # (bool): True if artifact was created with buildtree + # + def _buildtree_exists(self): + return self.__artifact.buildtree_exists() + # _fetch() # # Fetch the element's sources. diff --git a/tests/integration/artifact.py b/tests/integration/artifact.py index 35e7f56ae..98c6ce13f 100644 --- a/tests/integration/artifact.py +++ b/tests/integration/artifact.py @@ -77,23 +77,20 @@ def test_cache_buildtrees(cli, tmpdir, datafiles): assert cli.get_element_state(project, element_name) == 'cached' assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name)) - # The extracted buildtree dir should be empty, as we set the config - # to not cache buildtrees + # The buildtree dir should not exist, as we set the config to not cache buildtrees. cache_key = cli.get_element_key(project, element_name) elementdigest = share1.has_artifact('test', element_name, cache_key) with cas_extract_buildtree(elementdigest) as buildtreedir: - assert os.path.isdir(buildtreedir) - assert not os.listdir(buildtreedir) + assert not os.path.isdir(buildtreedir) # Delete the local cached artifacts, and assert the when pulled with --pull-buildtrees - # that is was cached in share1 as expected with an empty buildtree dir + # that is was cached in share1 as expected without a buildtree dir shutil.rmtree(os.path.join(str(tmpdir), 'cas')) assert cli.get_element_state(project, element_name) != 'cached' result = cli.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name]) assert element_name in result.get_pulled_elements() with cas_extract_buildtree(elementdigest) as buildtreedir: - assert os.path.isdir(buildtreedir) - assert not os.listdir(buildtreedir) + assert not os.path.isdir(buildtreedir) shutil.rmtree(os.path.join(str(tmpdir), 'cas')) # Assert that the default behaviour of pull to not include buildtrees on the artifact @@ -148,5 +145,4 @@ def test_cache_buildtrees(cli, tmpdir, datafiles): cache_key = cli.get_element_key(project, element_name) elementdigest = share3.has_artifact('test', element_name, cache_key) with cas_extract_buildtree(elementdigest) as buildtreedir: - assert os.path.isdir(buildtreedir) - assert not os.listdir(buildtreedir) + assert not os.path.isdir(buildtreedir) diff --git a/tests/integration/shellbuildtrees.py b/tests/integration/shellbuildtrees.py index 13d21548e..31af8a0e4 100644 --- a/tests/integration/shellbuildtrees.py +++ b/tests/integration/shellbuildtrees.py @@ -71,8 +71,8 @@ def test_buildtree_staged_warn_empty_cached(cli_integration, tmpdir, datafiles): res = cli_integration.run(project=project, args=[ 'shell', '--build', '--use-buildtree', 'always', element_name, '--', 'cat', 'test' ]) - res.assert_shell_error() - assert "Artifact contains an empty buildtree" in res.stderr + res.assert_main_error(ErrorDomain.APP, None) + assert "Artifact was created without buildtree" in res.stderr @pytest.mark.datafiles(DATA_DIR) @@ -148,8 +148,8 @@ def test_buildtree_from_failure_option_never(cli_integration, tmpdir, datafiles) res = cli_integration.run(project=project, args=[ 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test' ]) - res.assert_shell_error() - assert "Artifact contains an empty buildtree" in res.stderr + res.assert_main_error(ErrorDomain.APP, None) + assert "Artifact was created without buildtree" in res.stderr @pytest.mark.datafiles(DATA_DIR) |