diff options
-rw-r--r-- | src/buildstream/_assetcache.py | 9 | ||||
-rw-r--r-- | src/buildstream/_stream.py | 172 | ||||
-rw-r--r-- | tests/frontend/artifact_delete.py | 4 | ||||
-rw-r--r-- | tests/frontend/artifact_list_contents.py | 2 | ||||
-rw-r--r-- | tests/frontend/artifact_show.py | 67 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 2 | ||||
-rw-r--r-- | tests/frontend/push.py | 4 | ||||
-rw-r--r-- | tests/frontend/show.py | 46 | ||||
-rw-r--r-- | tests/frontend/simple/elements/compose-all.bst | 12 | ||||
-rw-r--r-- | tests/frontend/simple/elements/import-bin.bst | 4 | ||||
-rw-r--r-- | tests/frontend/simple/elements/import-dev.bst | 4 | ||||
-rw-r--r-- | tests/frontend/simple/elements/subdir/target.bst | 7 | ||||
-rw-r--r-- | tests/frontend/simple/elements/target.bst | 8 | ||||
-rwxr-xr-x | tests/frontend/simple/files/bin-files/usr/bin/hello | 3 | ||||
-rw-r--r-- | tests/frontend/simple/files/dev-files/usr/include/pony.h | 12 | ||||
-rw-r--r-- | tests/frontend/simple/project.conf | 4 |
16 files changed, 291 insertions, 69 deletions
diff --git a/src/buildstream/_assetcache.py b/src/buildstream/_assetcache.py index 68f7fd732..a0b502f2b 100644 --- a/src/buildstream/_assetcache.py +++ b/src/buildstream/_assetcache.py @@ -17,7 +17,7 @@ # Raoul Hidalgo Charman <raoul.hidalgocharman@codethink.co.uk> # import os -from fnmatch import fnmatch +import re from itertools import chain from typing import TYPE_CHECKING import grpc @@ -630,11 +630,16 @@ class AssetCache: # append the glob to optimise the os.walk() path = os.path.join(base_path, globdir) + regexer = None + if glob_expr: + expression = utils._glob2re(glob_expr) + regexer = re.compile(expression) + for root, _, files in os.walk(path): for filename in files: ref_path = os.path.join(root, filename) relative_path = os.path.relpath(ref_path, base_path) # Relative to refs head - if not glob_expr or fnmatch(relative_path, glob_expr): + if regexer is None or regexer.match(relative_path): # Obtain the mtime (the time a file was last modified) yield (os.path.getmtime(ref_path), relative_path) diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index 0a055a76d..91d7cb122 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -27,7 +27,6 @@ import shutil import tarfile import tempfile from contextlib import contextmanager, suppress -from fnmatch import fnmatch from collections import deque from typing import List, Tuple @@ -139,6 +138,7 @@ class Stream: # selection (_PipelineSelection): The selection mode for the specified targets # except_targets (list of str): Specified targets to except from fetching # use_artifact_config (bool): If artifact remote configs should be loaded + # load_artifacts (bool): Whether to load artifacts with artifact names # # Returns: # (list of Element): The selected elements @@ -149,7 +149,7 @@ class Stream: selection=_PipelineSelection.NONE, except_targets=(), use_artifact_config=False, - load_refs=False + load_artifacts=False ): with PROFILER.profile(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, "-") for t in targets)): target_objects = self._load( @@ -157,7 +157,7 @@ class Stream: selection=selection, except_targets=except_targets, use_artifact_config=use_artifact_config, - load_refs=load_refs, + load_artifacts=load_artifacts, ) return target_objects @@ -396,7 +396,7 @@ class Stream: selection=selection, use_source_config=use_source_config, source_remote_url=remote, - load_refs=True, + load_artifacts=True, ) if not self._sourcecache.has_push_remotes(): @@ -436,7 +436,7 @@ class Stream: ignore_junction_targets=ignore_junction_targets, use_artifact_config=use_config, artifact_remote_url=remote, - load_refs=True, + load_artifacts=True, ) if not self._artifacts.has_fetch_remotes(): @@ -477,7 +477,7 @@ class Stream: ignore_junction_targets=ignore_junction_targets, use_artifact_config=use_config, artifact_remote_url=remote, - load_refs=True, + load_artifacts=True, ) if not self._artifacts.has_push_remotes(): @@ -525,7 +525,7 @@ class Stream: tar=False ): - elements = self._load((target,), selection=selection, use_artifact_config=True, load_refs=True) + elements = self._load((target,), selection=selection, use_artifact_config=True, load_artifacts=True) # self.targets contains a list of the loaded target objects # if we specify --deps build, Stream._load() will return a list @@ -609,7 +609,9 @@ class Stream: # def artifact_show(self, targets, *, selection=_PipelineSelection.NONE): # Obtain list of Element and/or ArtifactElement objects - target_objects = self.load_selection(targets, selection=selection, use_artifact_config=True, load_refs=True) + target_objects = self.load_selection( + targets, selection=selection, use_artifact_config=True, load_artifacts=True + ) if self._artifacts.has_fetch_remotes(): self._pipeline.check_remotes(target_objects) @@ -634,7 +636,7 @@ class Stream: # def artifact_log(self, targets): # Return list of Element and/or ArtifactElement objects - target_objects = self.load_selection(targets, selection=_PipelineSelection.NONE, load_refs=True) + target_objects = self.load_selection(targets, selection=_PipelineSelection.NONE, load_artifacts=True) artifact_logs = {} for obj in target_objects: @@ -662,7 +664,7 @@ class Stream: # def artifact_list_contents(self, targets): # Return list of Element and/or ArtifactElement objects - target_objects = self.load_selection(targets, selection=_PipelineSelection.NONE, load_refs=True) + target_objects = self.load_selection(targets, selection=_PipelineSelection.NONE, load_artifacts=True) elements_to_files = {} for obj in target_objects: @@ -686,7 +688,7 @@ class Stream: # def artifact_delete(self, targets, *, selection=_PipelineSelection.NONE): # Return list of Element and/or ArtifactElement objects - target_objects = self.load_selection(targets, selection=selection, load_refs=True) + target_objects = self.load_selection(targets, selection=selection, load_artifacts=True) # Some of the targets may refer to the same key, so first obtain a # set of the refs to be removed. @@ -1094,17 +1096,23 @@ class Stream: # the loaded artifact elements. # # Args: - # targets - The target element names/artifact refs - # except_targets - The names of elements to except - # rewritable - Whether to load the elements in re-writable mode + # targets - The target element names/artifact refs + # except_targets - The names of elements to except + # rewritable - Whether to load the elements in re-writable mode + # valid_artifact_names: Whether artifact names are valid # # Returns: - # ([elements], [except_elements], [artifact_elements]) + # ([elements], [except_elements], [artifact_elements]) # def _load_elements_from_targets( - self, targets: List[str], except_targets: List[str], *, rewritable: bool = False + self, + targets: List[str], + except_targets: List[str], + *, + rewritable: bool = False, + valid_artifact_names: bool = False ) -> Tuple[List[Element], List[Element], List[Element]]: - names, refs = self._classify_artifacts(targets) + names, refs = self._expand_and_classify_targets(targets, valid_artifact_names=valid_artifact_names) loadable = [names, except_targets] self._project.load_context.set_rewritable(rewritable) @@ -1211,6 +1219,8 @@ class Stream: # use_source_config (bool): Whether to initialize remote source caches with the config # artifact_remote_url (str): A remote url for initializing the artifacts # source_remote_url (str): A remote url for initializing source caches + # dynamic_plan (bool): Require artifacts as needed during the build + # load_artifacts (bool): Whether to load artifacts with artifact names # # Returns: # (list of Element): The primary element selection @@ -1227,18 +1237,18 @@ class Stream: artifact_remote_url=None, source_remote_url=None, dynamic_plan=False, - load_refs=False + load_artifacts=False ): elements, except_elements, artifacts = self._load_elements_from_targets( - targets, except_targets, rewritable=False + targets, except_targets, rewritable=False, valid_artifact_names=load_artifacts ) if artifacts: - if not load_refs: - detail = "\n".join(artifact.get_artifact_name() for artifact in artifacts) - raise ArtifactElementError("Cannot perform this operation with artifact refs:", detail=detail) if selection in (_PipelineSelection.ALL, _PipelineSelection.RUN): - raise StreamError("Error: '--deps {}' is not supported for artifact refs".format(selection.value)) + raise StreamError( + "Error: '--deps {}' is not supported for artifact names".format(selection.value), + reason="deps-not-supported", + ) if ignore_junction_targets: elements = [e for e in elements if e.get_kind() != "junction"] @@ -1555,57 +1565,101 @@ class Stream: return required_list - # _classify_artifacts() + # _expand_and_classify_targets() # - # Split up a list of targets into element names and artifact refs + # Takes the user provided targets, expand any glob patterns, and + # return a new list of targets. + # + # If valid_artifact_names is specified, then glob patterns will + # also be checked for locally existing artifact names, and the + # targets will be classified into separate lists, any targets + # which are found to be an artifact name will be returned in + # the list of artifact names. # # Args: - # targets (list): A list of targets + # targets: A list of targets + # valid_artifact_names: Whether artifact names are valid # # Returns: # (list): element names present in the targets - # (list): artifact refs present in the targets + # (list): artifact names present in the targets # - def _classify_artifacts(self, targets): + def _expand_and_classify_targets( + self, targets: List[str], valid_artifact_names: bool = False + ) -> Tuple[List[str], List[str]]: + initial_targets = [] element_targets = [] - artifact_refs = [] - element_globs = [] - artifact_globs = [] + artifact_names = [] + globs = {} # Count whether a glob matched elements and artifacts + # First extract the globs for target in targets: - if target.endswith(".bst"): - if any(c in "*?[" for c in target): - element_globs.append(target) - else: - element_targets.append(target) + if any(c in "*?[" for c in target): + globs[target] = 0 else: - if any(c in "*?[" for c in target): - artifact_globs.append(target) + initial_targets.append(target) + + # Filter out any targets which are found to be artifact names + if valid_artifact_names: + for target in initial_targets: + try: + verify_artifact_ref(target) + except ArtifactElementError: + element_targets.append(target) else: - try: - verify_artifact_ref(target) - except ArtifactElementError: - element_targets.append(target) - continue - artifact_refs.append(target) - - if element_globs: - for dirpath, _, filenames in os.walk(self._project.element_path): - for filename in filenames: + artifact_names.append(target) + else: + element_targets = initial_targets + + # Expand globs for elements + all_elements = [] + element_path_length = len(self._project.element_path) + 1 + for dirpath, _, filenames in os.walk(self._project.element_path): + for filename in filenames: + if filename.endswith(".bst"): element_path = os.path.join(dirpath, filename) - length = len(self._project.element_path) + 1 - element_path = element_path[length:] # Strip out the element_path - - if any(fnmatch(element_path, glob) for glob in element_globs): - element_targets.append(element_path) + element_path = element_path[element_path_length:] # Strip out the element_path + all_elements.append(element_path) + + for glob in globs: + matched = False + for element_path in utils.glob(all_elements, glob): + element_targets.append(element_path) + matched = True + if matched: + globs[glob] = globs[glob] + 1 + + # Expand globs for artifact names + if valid_artifact_names: + for glob in globs: + matches = self._artifacts.list_artifacts(glob=glob) + if matches: + artifact_names.extend(matches) + globs[glob] = globs[glob] + 1 + + # Issue warnings and errors + unmatched = [glob for glob in globs if globs[glob] == 0] + doubly_matched = [glob for glob in globs if globs[glob] > 1] + + # Warn the user if any of the provided globs did not match anything + if unmatched: + if valid_artifact_names: + message = "No elements or artifacts matched the following glob expression(s): {}".format( + ", ".join(unmatched) + ) + else: + message = "No elements matched the following glob expression(s): {}".format(", ".join(unmatched)) + self._message(MessageType.WARN, message) - if artifact_globs: - for glob in artifact_globs: - artifact_refs.extend(self._artifacts.list_artifacts(glob=glob)) - if not artifact_refs: - self._message(MessageType.WARN, "No artifacts found for globs: {}".format(", ".join(artifact_globs))) + if doubly_matched: + raise StreamError( + "The provided glob expression(s) matched both element names and artifact names: {}".format( + ", ".join(doubly_matched) + ), + reason="glob-elements-and-artifacts", + ) - return element_targets, artifact_refs + return element_targets, artifact_names # _handle_compression() diff --git a/tests/frontend/artifact_delete.py b/tests/frontend/artifact_delete.py index 2651f567e..7b26a7644 100644 --- a/tests/frontend/artifact_delete.py +++ b/tests/frontend/artifact_delete.py @@ -256,6 +256,4 @@ def test_artifact_delete_artifact_with_deps_all_fails(cli, tmpdir, datafiles): # Try to delete the artifact with all of its dependencies result = cli.run(project=project, args=["artifact", "delete", "--deps", "all", artifact]) - result.assert_main_error(ErrorDomain.STREAM, None) - - assert "Error: '--deps all' is not supported for artifact refs" in result.stderr + result.assert_main_error(ErrorDomain.STREAM, "deps-not-supported") diff --git a/tests/frontend/artifact_list_contents.py b/tests/frontend/artifact_list_contents.py index 8bd7bdeff..ee129cc9f 100644 --- a/tests/frontend/artifact_list_contents.py +++ b/tests/frontend/artifact_list_contents.py @@ -71,7 +71,7 @@ def test_artifact_list_exact_contents_glob(cli, datafiles): assert result.exit_code == 0 # List the contents via glob - result = cli.run(project=project, args=["artifact", "list-contents", "test/*"]) + result = cli.run(project=project, args=["artifact", "list-contents", "test/**"]) assert result.exit_code == 0 # get the cahe keys for each element in the glob diff --git a/tests/frontend/artifact_show.py b/tests/frontend/artifact_show.py index de9b78c45..ebea7cf33 100644 --- a/tests/frontend/artifact_show.py +++ b/tests/frontend/artifact_show.py @@ -28,6 +28,7 @@ from tests.testutils import create_artifact_share # Project directory DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project",) +SIMPLE_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "simple",) # Test artifact show @@ -102,6 +103,72 @@ def test_artifact_show_artifact_ref(cli, tmpdir, datafiles): assert "cached {}".format(artifact_ref) in result.output +# Test artifact show glob behaviors +@pytest.mark.datafiles(SIMPLE_DIR) +@pytest.mark.parametrize( + "pattern,expected_prefixes", + [ + # List only artifact results in the test/project + # + ("test/**", ["test/target/", "test/target/", "test/compose-all/", "test/import-bin", "test/import-dev"]), + # List only artifact results by their .bst element names + # + ("**.bst", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst", "subdir/target.bst"]), + # List only the import artifact results + # + ("import*.bst", ["import-bin.bst", "import-dev.bst"]), + ], + ids=["test/**", "**.bst", "import*.bst"], +) +def test_artifact_show_glob(cli, tmpdir, datafiles, pattern, expected_prefixes): + project = str(datafiles) + + result = cli.run(project=project, args=["build", "target.bst"]) + result.assert_success() + + result = cli.run(project=project, args=["artifact", "show", pattern]) + result.assert_success() + + output = result.output.strip().splitlines() + + # Assert that the number of results match the number of expected results + assert len(output) == len(expected_prefixes) + + # Assert that each expected result was found. + for expected_prefix in expected_prefixes: + found = False + for result_line in output: + result_split = result_line.split() + if result_split[-1].startswith(expected_prefix): + found = True + break + assert found, "Expected result {} not found".format(expected_prefix) + + +# Test artifact show glob behaviors +@pytest.mark.datafiles(SIMPLE_DIR) +@pytest.mark.parametrize( + "pattern", + [ + # Catch all glob will match everything, that is an error since the glob matches + # both elements and artifacts + # + "**", + # This glob is more selective but will also match both artifacts and elements + # + "**import-bin**", + ], +) +def test_artifact_show_doubly_matched_glob_error(cli, tmpdir, datafiles, pattern): + project = str(datafiles) + + result = cli.run(project=project, args=["build", "target.bst"]) + result.assert_success() + + result = cli.run(project=project, args=["artifact", "show", pattern]) + result.assert_main_error(ErrorDomain.STREAM, "glob-elements-and-artifacts") + + # Test artifact show artifact in remote @pytest.mark.datafiles(DATA_DIR) def test_artifact_show_element_available_remotely(cli, tmpdir, datafiles): diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index 5afa5216d..709259397 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -451,7 +451,7 @@ def test_build_checkout_runtime_deps_using_ref_fails(datafiles, cli): checkout_args = ["artifact", "checkout", "--directory", checkout, "--deps", "run", "test/checkout-deps/" + key] result = cli.run(project=project, args=checkout_args) - result.assert_main_error(ErrorDomain.STREAM, None) + result.assert_main_error(ErrorDomain.STREAM, "deps-not-supported") @pytest.mark.datafiles(DATA_DIR) diff --git a/tests/frontend/push.py b/tests/frontend/push.py index 4b10b5bcd..31b0b7ec3 100644 --- a/tests/frontend/push.py +++ b/tests/frontend/push.py @@ -335,9 +335,7 @@ def test_push_artifacts_all_deps_fails(cli, tmpdir, datafiles): # Now try bst artifact push all the deps result = cli.run(project=project, args=["artifact", "push", "--deps", "all", artifact_ref]) - result.assert_main_error(ErrorDomain.STREAM, None) - - assert "Error: '--deps all' is not supported for artifact refs" in result.stderr + result.assert_main_error(ErrorDomain.STREAM, "deps-not-supported") # Tests that `bst build` won't push artifacts to the cache it just pulled from. diff --git a/tests/frontend/show.py b/tests/frontend/show.py index 4be4b72e9..81e1e629c 100644 --- a/tests/frontend/show.py +++ b/tests/frontend/show.py @@ -50,6 +50,52 @@ def test_show_fail(cli, datafiles): result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) +# Test behaviors of user supplied glob patterns +@pytest.mark.datafiles(os.path.join(DATA_DIR, "simple")) +@pytest.mark.parametrize( + "pattern,expected_elements", + [ + # Use catch all glob. This should report all elements. + # + ("**", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst", "subdir/target.bst"]), + # Only bst files, same as "**" for `bst show` + # + ("**.bst", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst", "subdir/target.bst"]), + # Use regular globbing without matching path separators, this should exclude + # the target in the subdirectory. + # + ("*.bst", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst"]), + # Report only targets in the subdirectory + # + ("subdir/*", ["subdir/target.bst"]), + # Report both targets which end in "target.bst" + # + ("**target.bst", ["target.bst", "subdir/target.bst"]), + # All elements starting with the prefix "import" + # + ("import*", ["import-bin.bst", "import-dev.bst"]), + # Glob would match artifact refs, but `bst show` does not accept these as input. + # + ("test/**", []), + ], + ids=["**", "**.bst", "*.bst", "subdir/*", "**target.bst", "import*", "test/**"], +) +def test_show_glob(cli, tmpdir, datafiles, pattern, expected_elements): + project = str(datafiles) + + result = cli.run(project=project, args=["show", "--deps", "none", "--format", "%{name}", pattern]) + result.assert_success() + + output = result.output.strip().splitlines() + + # Assert that the number of results match the number of expected results + assert len(output) == len(expected_elements) + + # Assert that each expected result was found. + for expected in expected_elements: + assert expected in output, "Expected result {} not found".format(expected) + + @pytest.mark.datafiles(os.path.join(DATA_DIR, "project")) @pytest.mark.parametrize( "target,except_,expected", diff --git a/tests/frontend/simple/elements/compose-all.bst b/tests/frontend/simple/elements/compose-all.bst new file mode 100644 index 000000000..ba47081b3 --- /dev/null +++ b/tests/frontend/simple/elements/compose-all.bst @@ -0,0 +1,12 @@ +kind: compose + +depends: +- filename: import-bin.bst + type: build +- filename: import-dev.bst + type: build + +config: + # Dont try running the sandbox, we dont have a + # runtime to run anything in this context. + integrate: False diff --git a/tests/frontend/simple/elements/import-bin.bst b/tests/frontend/simple/elements/import-bin.bst new file mode 100644 index 000000000..a847c0c23 --- /dev/null +++ b/tests/frontend/simple/elements/import-bin.bst @@ -0,0 +1,4 @@ +kind: import +sources: +- kind: local + path: files/bin-files diff --git a/tests/frontend/simple/elements/import-dev.bst b/tests/frontend/simple/elements/import-dev.bst new file mode 100644 index 000000000..152a54667 --- /dev/null +++ b/tests/frontend/simple/elements/import-dev.bst @@ -0,0 +1,4 @@ +kind: import +sources: +- kind: local + path: files/dev-files diff --git a/tests/frontend/simple/elements/subdir/target.bst b/tests/frontend/simple/elements/subdir/target.bst new file mode 100644 index 000000000..411206787 --- /dev/null +++ b/tests/frontend/simple/elements/subdir/target.bst @@ -0,0 +1,7 @@ +kind: stack +description: | + + Another target in a subdirectory + +depends: +- import-dev.bst diff --git a/tests/frontend/simple/elements/target.bst b/tests/frontend/simple/elements/target.bst new file mode 100644 index 000000000..b9432fafa --- /dev/null +++ b/tests/frontend/simple/elements/target.bst @@ -0,0 +1,8 @@ +kind: stack +description: | + + Main stack target for the bst build test + +depends: +- import-bin.bst +- compose-all.bst diff --git a/tests/frontend/simple/files/bin-files/usr/bin/hello b/tests/frontend/simple/files/bin-files/usr/bin/hello new file mode 100755 index 000000000..f534a4083 --- /dev/null +++ b/tests/frontend/simple/files/bin-files/usr/bin/hello @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "Hello !" diff --git a/tests/frontend/simple/files/dev-files/usr/include/pony.h b/tests/frontend/simple/files/dev-files/usr/include/pony.h new file mode 100644 index 000000000..40bd0c2e7 --- /dev/null +++ b/tests/frontend/simple/files/dev-files/usr/include/pony.h @@ -0,0 +1,12 @@ +#ifndef __PONY_H__ +#define __PONY_H__ + +#define PONY_BEGIN "Once upon a time, there was a pony." +#define PONY_END "And they lived happily ever after, the end." + +#define MAKE_PONY(story) \ + PONY_BEGIN \ + story \ + PONY_END + +#endif /* __PONY_H__ */ diff --git a/tests/frontend/simple/project.conf b/tests/frontend/simple/project.conf new file mode 100644 index 000000000..5ba316874 --- /dev/null +++ b/tests/frontend/simple/project.conf @@ -0,0 +1,4 @@ +# Project config for frontend build test +name: test +min-version: 2.0 +element-path: elements |