diff options
author | Chandan Singh <csingh43@bloomberg.net> | 2018-12-06 16:18:04 +0000 |
---|---|---|
committer | Chandan Singh <csingh43@bloomberg.net> | 2019-02-26 03:05:03 +0530 |
commit | 88ace418b0e9d1608b58c78e206d203af8fc6eeb (patch) | |
tree | 325fb511046353e29b63d4e151ab304823da5f7f | |
parent | a3f68c3d7e0796f1a5c9a5615bf902c805847cd5 (diff) | |
download | buildstream-88ace418b0e9d1608b58c78e206d203af8fc6eeb.tar.gz |
loader: Allow dependencies to use ":" to refer to junctioned elements
This will allow cross-junction dependencies to be listed as strings on a
single line.
As part of this, some logic around initializing `Dependency()` objects
have been moved out of `_extract_depends_from_node()` method into the
constructor of `Dependency()` class, to keep all related code in one
place.
* _loader/types.py: While initializing `Dependency` objects, attempt to
split filenames, only if no `junction` was specified
explicitly. If a `junction` was specified, then
filenames with `:` in their names will result in an
error.
* _loader/loadelement.py: Refactor logic to initialize `Dependency()`
objects to move it to the `Dependency()`
constructor.
* tests/frontend/buildcheckout.py: Add tests to ensure the above.
* _versions.py: Bump BST_FORMAT_VERSION.
Fixes #809.
-rw-r--r-- | buildstream/_loader/loadelement.py | 35 | ||||
-rw-r--r-- | buildstream/_loader/types.py | 60 | ||||
-rw-r--r-- | buildstream/_versions.py | 2 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 136 |
4 files changed, 193 insertions, 40 deletions
diff --git a/buildstream/_loader/loadelement.py b/buildstream/_loader/loadelement.py index 17154ee64..677335404 100644 --- a/buildstream/_loader/loadelement.py +++ b/buildstream/_loader/loadelement.py @@ -18,13 +18,11 @@ # Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> # System imports -from collections.abc import Mapping from itertools import count from pyroaring import BitMap, FrozenBitMap # pylint: disable=no-name-in-module # BuildStream toplevel imports -from .._exceptions import LoadError, LoadErrorReason from .. import _yaml # Local package imports @@ -174,38 +172,7 @@ def _extract_depends_from_node(node, *, key=None): for index, dep in enumerate(depends): dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index]) - - if isinstance(dep, str): - dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type) - - elif isinstance(dep, Mapping): - if default_dep_type: - _yaml.node_validate(dep, ['filename', 'junction']) - dep_type = default_dep_type - else: - _yaml.node_validate(dep, ['filename', 'type', 'junction']) - - # Make type optional, for this we set it to None - dep_type = _yaml.node_get(dep, str, Symbol.TYPE, default_value=None) - if dep_type is None or dep_type == Symbol.ALL: - dep_type = None - elif dep_type not in [Symbol.BUILD, Symbol.RUNTIME]: - provenance = _yaml.node_get_provenance(dep, key=Symbol.TYPE) - raise LoadError(LoadErrorReason.INVALID_DATA, - "{}: Dependency type '{}' is not 'build', 'runtime' or 'all'" - .format(provenance, dep_type)) - - filename = _yaml.node_get(dep, str, Symbol.FILENAME) - junction = _yaml.node_get(dep, str, Symbol.JUNCTION, default_value=None) - dependency = Dependency(filename, - dep_type=dep_type, - junction=junction, - provenance=dep_provenance) - - else: - raise LoadError(LoadErrorReason.INVALID_DATA, - "{}: Dependency is not specified as a string or a dictionary".format(dep_provenance)) - + dependency = Dependency(dep, dep_provenance, default_dep_type=default_dep_type) output_deps.append(dependency) # Now delete the field, we dont want it anymore diff --git a/buildstream/_loader/types.py b/buildstream/_loader/types.py index 25b785532..eb6932b0b 100644 --- a/buildstream/_loader/types.py +++ b/buildstream/_loader/types.py @@ -17,6 +17,11 @@ # Authors: # Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> +from collections.abc import Mapping + +from .._exceptions import LoadError, LoadErrorReason +from .. import _yaml + # Symbol(): # @@ -56,9 +61,54 @@ class Symbol(): # dependency was declared # class Dependency(): - def __init__(self, name, - dep_type=None, junction=None, provenance=None): - self.name = name - self.dep_type = dep_type - self.junction = junction + def __init__(self, dep, provenance, default_dep_type=None): self.provenance = provenance + + if isinstance(dep, str): + self.name = dep + self.dep_type = default_dep_type + self.junction = None + + elif isinstance(dep, Mapping): + if default_dep_type: + _yaml.node_validate(dep, ['filename', 'junction']) + dep_type = default_dep_type + else: + _yaml.node_validate(dep, ['filename', 'type', 'junction']) + + # Make type optional, for this we set it to None + dep_type = _yaml.node_get(dep, str, Symbol.TYPE, default_value=None) + if dep_type is None or dep_type == Symbol.ALL: + dep_type = None + elif dep_type not in [Symbol.BUILD, Symbol.RUNTIME]: + provenance = _yaml.node_get_provenance(dep, key=Symbol.TYPE) + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Dependency type '{}' is not 'build', 'runtime' or 'all'" + .format(provenance, dep_type)) + + self.name = _yaml.node_get(dep, str, Symbol.FILENAME) + self.dep_type = dep_type + self.junction = _yaml.node_get(dep, str, Symbol.JUNCTION, default_value=None) + + else: + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Dependency is not specified as a string or a dictionary".format(provenance)) + + # `:` characters are not allowed in filename if a junction was + # explicitly specified + if self.junction and ':' in self.name: + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Dependency {} contains `:` in its name. " + "`:` characters are not allowed in filename when " + "junction attribute is specified.".format(self.provenance, self.name)) + + # Name of the element should never contain more than one `:` characters + if self.name.count(':') > 1: + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Dependency {} contains multiple `:` in its name. " + "Recursive lookups for cross-junction elements is not " + "allowed.".format(self.provenance, self.name)) + + # Attempt to split name if no junction was specified explicitly + if not self.junction and self.name.count(':') == 1: + self.junction, self.name = self.name.split(':') diff --git a/buildstream/_versions.py b/buildstream/_versions.py index 5affc7870..56fd95223 100644 --- a/buildstream/_versions.py +++ b/buildstream/_versions.py @@ -23,7 +23,7 @@ # This version is bumped whenever enhancements are made # to the `project.conf` format or the core element format. # -BST_FORMAT_VERSION = 22 +BST_FORMAT_VERSION = 23 # The base BuildStream artifact version diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index 598edbbbc..e7f67fa91 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -729,3 +729,139 @@ def test_build_checkout_cross_junction(datafiles, cli, tmpdir): filename = os.path.join(checkout, 'etc', 'animal.conf') assert os.path.exists(filename) + + +@pytest.mark.datafiles(DATA_DIR) +def test_build_junction_short_notation(cli, tmpdir, datafiles): + project = os.path.join(datafiles.dirname, datafiles.basename) + subproject_path = os.path.join(project, 'files', 'sub-project') + junction_path = os.path.join(project, 'elements', 'junction.bst') + element_path = os.path.join(project, 'elements', 'junction-dep.bst') + workspace = os.path.join(cli.directory, 'workspace') + checkout = os.path.join(cli.directory, 'checkout') + + # Create a repo to hold the subproject and generate a junction element for it + ref = generate_junction(tmpdir, subproject_path, junction_path) + + # Create a stack element to depend on a cross junction element, using + # colon (:) as the separator + element = { + 'kind': 'stack', + 'depends': ['junction.bst:import-etc.bst'] + } + _yaml.dump(element, element_path) + + # Now try to build it, this should automatically result in fetching + # the junction itself at load time. + result = cli.run(project=project, args=['build', 'junction-dep.bst']) + result.assert_success() + + # Assert that it's cached now + assert cli.get_element_state(project, 'junction-dep.bst') == 'cached' + + # Now check it out + result = cli.run(project=project, args=[ + 'artifact', 'checkout', 'junction-dep.bst', '--directory', checkout + ]) + result.assert_success() + + # Assert the content of /etc/animal.conf + filename = os.path.join(checkout, 'etc', 'animal.conf') + assert os.path.exists(filename) + with open(filename, 'r') as f: + contents = f.read() + assert contents == 'animal=Pony\n' + + +@pytest.mark.datafiles(DATA_DIR) +def test_build_junction_short_notation_filename(cli, tmpdir, datafiles): + project = os.path.join(datafiles.dirname, datafiles.basename) + subproject_path = os.path.join(project, 'files', 'sub-project') + junction_path = os.path.join(project, 'elements', 'junction.bst') + element_path = os.path.join(project, 'elements', 'junction-dep.bst') + checkout = os.path.join(cli.directory, 'checkout') + + # Create a repo to hold the subproject and generate a junction element for it + ref = generate_junction(tmpdir, subproject_path, junction_path) + + # Create a stack element to depend on a cross junction element, using + # colon (:) as the separator + element = { + 'kind': 'stack', + 'depends': [{'filename': 'junction.bst:import-etc.bst'}] + } + _yaml.dump(element, element_path) + + # Now try to build it, this should automatically result in fetching + # the junction itself at load time. + result = cli.run(project=project, args=['build', 'junction-dep.bst']) + result.assert_success() + + # Assert that it's cached now + assert cli.get_element_state(project, 'junction-dep.bst') == 'cached' + + # Now check it out + result = cli.run(project=project, args=[ + 'artifact', 'checkout', 'junction-dep.bst', '--directory', checkout + ]) + result.assert_success() + + # Assert the content of /etc/animal.conf + filename = os.path.join(checkout, 'etc', 'animal.conf') + assert os.path.exists(filename) + with open(filename, 'r') as f: + contents = f.read() + assert contents == 'animal=Pony\n' + + +@pytest.mark.datafiles(DATA_DIR) +def test_build_junction_short_notation_with_junction(cli, tmpdir, datafiles): + project = os.path.join(datafiles.dirname, datafiles.basename) + subproject_path = os.path.join(project, 'files', 'sub-project') + junction_path = os.path.join(project, 'elements', 'junction.bst') + element_path = os.path.join(project, 'elements', 'junction-dep.bst') + checkout = os.path.join(cli.directory, 'checkout') + + # Create a repo to hold the subproject and generate a junction element for it + ref = generate_junction(tmpdir, subproject_path, junction_path) + + # Create a stack element to depend on a cross junction element, using + # colon (:) as the separator + element = { + 'kind': 'stack', + 'depends': [{ + 'filename': 'junction.bst:import-etc.bst', + 'junction': 'junction.bst', + }] + } + _yaml.dump(element, element_path) + + # Now try to build it, this should fail as filenames should not contain + # `:` when junction is explicity specified + result = cli.run(project=project, args=['build', 'junction-dep.bst']) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) + + +@pytest.mark.datafiles(DATA_DIR) +def test_build_junction_short_notation_with_junction(cli, tmpdir, datafiles): + project = os.path.join(datafiles.dirname, datafiles.basename) + subproject_path = os.path.join(project, 'files', 'sub-project') + junction_path = os.path.join(project, 'elements', 'junction.bst') + element_path = os.path.join(project, 'elements', 'junction-dep.bst') + checkout = os.path.join(cli.directory, 'checkout') + + # Create a repo to hold the subproject and generate a junction element for it + ref = generate_junction(tmpdir, subproject_path, junction_path) + + # Create a stack element to depend on a cross junction element, using + # colon (:) as the separator + element = { + 'kind': 'stack', + 'depends': ['junction.bst:import-etc.bst:foo.bst'] + } + _yaml.dump(element, element_path) + + # Now try to build it, this should fail as recursive lookups for + # cross-junction elements is not allowed. + result = cli.run(project=project, args=['build', 'junction-dep.bst']) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) |