diff options
-rw-r--r-- | doc/source/format_declaring.rst | 11 | ||||
-rw-r--r-- | doc/source/using_config.rst | 2 | ||||
-rw-r--r-- | src/buildstream/_loader/loadelement.pyx | 7 | ||||
-rw-r--r-- | src/buildstream/_loader/loader.py | 6 | ||||
-rw-r--r-- | src/buildstream/_loader/metaelement.py | 1 | ||||
-rw-r--r-- | src/buildstream/_loader/types.pyx | 28 | ||||
-rw-r--r-- | src/buildstream/element.py | 30 | ||||
-rw-r--r-- | tests/format/dependencies.py | 14 | ||||
-rw-r--r-- | tests/format/dependencies1/elements/invalidnonstrict.bst | 10 | ||||
-rw-r--r-- | tests/format/dependencies1/elements/invalidstrict.bst | 9 | ||||
-rw-r--r-- | tests/frontend/show.py | 54 | ||||
-rw-r--r-- | tests/frontend/strict-depends/elements/base.bst | 5 | ||||
-rw-r--r-- | tests/frontend/strict-depends/elements/non-strict-depends.bst | 4 | ||||
-rw-r--r-- | tests/frontend/strict-depends/elements/strict-depends.bst | 5 | ||||
-rw-r--r-- | tests/frontend/strict-depends/files/hello.txt | 1 | ||||
-rw-r--r-- | tests/frontend/strict-depends/project.conf | 2 |
16 files changed, 173 insertions, 16 deletions
diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst index a38973347..80365c6d5 100644 --- a/doc/source/format_declaring.rst +++ b/doc/source/format_declaring.rst @@ -370,6 +370,7 @@ Dependency dictionary: - filename: foo.bst type: build junction: baseproject.bst + strict: false Attributes: @@ -400,6 +401,16 @@ Attributes: The ``junction`` attribute is available since :ref:`format version 1 <project_format_version>` +* ``strict`` + + This attribute can be used to specify that this element should + be rebuilt when the dependency changes, even when + :ref:`strict mode <user_config_strict_mode>` has been turned off. + + This is appropriate whenever a dependency's output is consumed + verbatim in the output of the depending element, for instance + when static linking is in use. + Cross-junction dependencies ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/source/using_config.rst b/doc/source/using_config.rst index 8a6b9ac76..967dde2ed 100644 --- a/doc/source/using_config.rst +++ b/doc/source/using_config.rst @@ -201,6 +201,8 @@ configuration will be used as fallback. instance-name: main +.. _user_config_strict_mode: + Strict build plan ~~~~~~~~~~~~~~~~~ The strict build plan option decides whether you want elements diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx index 867de6f29..fb1dd1d15 100644 --- a/src/buildstream/_loader/loadelement.pyx +++ b/src/buildstream/_loader/loadelement.pyx @@ -39,18 +39,21 @@ cdef int _next_synthetic_counter(): # A link from a LoadElement to its dependencies. # # Keeps a link to one of the current Element's dependencies, together with -# its dependency type. +# its dependency attributes. # # Args: # element (LoadElement): a LoadElement on which there is a dependency # dep_type (str): the type of dependency this dependency link is +# strict (bint): whether the dependency is strict cdef class Dependency: cdef readonly LoadElement element cdef readonly str dep_type + cdef readonly bint strict - def __cinit__(self, LoadElement element, str dep_type): + def __cinit__(self, LoadElement element, str dep_type, bint strict): self.element = element self.dep_type = dep_type + self.strict = strict # LoadElement(): diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 061d28bf0..89458d40c 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -124,7 +124,7 @@ class Loader(): dummy_target = LoadElement(Node.from_dict({}), "", self) # Pylint is not very happy with Cython and can't understand 'dependencies' is a list dummy_target.dependencies.extend( # pylint: disable=no-member - Dependency(element, Symbol.RUNTIME) + Dependency(element, Symbol.RUNTIME, False) for element in target_elements ) @@ -344,7 +344,7 @@ class Loader(): # All is well, push the dependency onto the LoadElement # Pylint is not very happy with Cython and can't understand 'dependencies' is a list current_element[0].dependencies.append( # pylint: disable=no-member - Dependency(dep_element, dep.dep_type)) + Dependency(dep_element, dep.dep_type, dep.strict)) else: # We do not have any more dependencies to load for this # element on the queue, report any invalid dep names @@ -499,6 +499,8 @@ class Loader(): meta_element.build_dependencies.append(meta_dep) if dep.dep_type != 'build': meta_element.dependencies.append(meta_dep) + if dep.strict: + meta_element.strict_dependencies.append(meta_dep) element.meta_done = True diff --git a/src/buildstream/_loader/metaelement.py b/src/buildstream/_loader/metaelement.py index 67d2ec771..00d8560f8 100644 --- a/src/buildstream/_loader/metaelement.py +++ b/src/buildstream/_loader/metaelement.py @@ -56,5 +56,6 @@ class MetaElement(): self.sandbox = sandbox or Node.from_dict({}) self.build_dependencies = [] self.dependencies = [] + self.strict_dependencies = [] self.first_pass = first_pass self.is_junction = kind == "junction" diff --git a/src/buildstream/_loader/types.pyx b/src/buildstream/_loader/types.pyx index 0a223f9a9..42a7b801f 100644 --- a/src/buildstream/_loader/types.pyx +++ b/src/buildstream/_loader/types.pyx @@ -44,6 +44,7 @@ class Symbol(): DIRECTORY = "directory" JUNCTION = "junction" SANDBOX = "sandbox" + STRICT = "strict" # Dependency() @@ -63,6 +64,7 @@ cdef class Dependency: cdef public str name cdef public str dep_type cdef public str junction + cdef public bint strict def __init__(self, Node dep, @@ -75,13 +77,14 @@ cdef class Dependency: self.name = dep.as_str() self.dep_type = default_dep_type self.junction = None + self.strict = False elif type(dep) is MappingNode: if default_dep_type: - (<MappingNode> dep).validate_keys(['filename', 'junction']) + (<MappingNode> dep).validate_keys(['filename', 'junction', 'strict']) dep_type = default_dep_type else: - (<MappingNode> dep).validate_keys(['filename', 'type', 'junction']) + (<MappingNode> dep).validate_keys(['filename', 'type', 'junction', 'strict']) # Make type optional, for this we set it to None dep_type = (<MappingNode> dep).get_str(<str> Symbol.TYPE, None) @@ -95,11 +98,32 @@ cdef class Dependency: self.name = (<MappingNode> dep).get_str(<str> Symbol.FILENAME) self.dep_type = dep_type self.junction = (<MappingNode> dep).get_str(<str> Symbol.JUNCTION, None) + self.strict = (<MappingNode> dep).get_bool(<str> Symbol.STRICT, False) + + # Here we disallow explicitly setting 'strict' to False. + # + # This is in order to keep the door open to allowing the project.conf + # set the default of dependency 'strict'-ness which might be useful + # for projects which use mostly static linking and the like, in which + # case we can later interpret explicitly non-strict dependencies + # as an override of the project default. + # + if self.strict == False and Symbol.STRICT in dep: + provenance = dep.get_scalar(Symbol.STRICT).get_provenance() + raise LoadError("{}: Setting 'strict' to False is unsupported" + .format(provenance), LoadErrorReason.INVALID_DATA) else: raise LoadError("{}: Dependency is not specified as a string or a dictionary".format(self.provenance), LoadErrorReason.INVALID_DATA) + # Only build dependencies are allowed to be strict + # + if self.strict and self.dep_type == Symbol.RUNTIME: + raise LoadError("{}: Runtime dependency {} specified as `strict`.".format(self.provenance, self.name), + LoadErrorReason.INVALID_DATA, + detail="Only dependencies required at build time may be declared `strict`.") + # `:` characters are not allowed in filename if a junction was # explicitly specified if self.junction and ':' in self.name: diff --git a/src/buildstream/element.py b/src/buildstream/element.py index 10c8320fa..633e29c88 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -208,6 +208,7 @@ class Element(Plugin): self.__runtime_dependencies = [] # Direct runtime dependency Elements self.__build_dependencies = [] # Direct build dependency Elements + self.__strict_dependencies = [] # Direct build dependency subset which require strict rebuilds self.__reverse_build_deps = set() # Direct reverse build dependency Elements self.__reverse_runtime_deps = set() # Direct reverse runtime dependency Elements self.__build_deps_without_strict_cache_key = None # Number of build dependencies without a strict key @@ -989,6 +990,10 @@ class Element(Plugin): dependency = Element._new_from_meta(meta_dep, task) element.__build_dependencies.append(dependency) dependency.__reverse_build_deps.add(element) + + if meta_dep in meta.strict_dependencies: + element.__strict_dependencies.append(dependency) + no_of_build_deps = len(element.__build_dependencies) element.__build_deps_without_strict_cache_key = no_of_build_deps element.__build_deps_without_cache_key = no_of_build_deps @@ -3107,17 +3112,22 @@ class Element(Plugin): if self.__weak_cache_key is None: # Calculate weak cache key + # # Weak cache key includes names of direct build dependencies - # but does not include keys of dependencies. - if self.BST_STRICT_REBUILD: - dependencies = [ - e._get_cache_key(strength=_KeyStrength.WEAK) - for e in self.dependencies(Scope.BUILD) - ] - else: - dependencies = [ - [e.project_name, e.name] for e in self.dependencies(Scope.BUILD, recurse=False) - ] + # so as to only trigger rebuilds when the shape of the + # dependencies change. + # + # Some conditions cause dependencies to be strict, such + # that this element will be rebuilt anyway if the dependency + # changes even in non strict mode, for these cases we just + # encode the dependency's weak cache key instead of it's name. + # + dependencies = [ + e._get_cache_key(strength=_KeyStrength.WEAK) + if self.BST_STRICT_REBUILD or e in self.__strict_dependencies + else [e.project_name, e.name] + for e in self.dependencies(Scope.BUILD) + ] self.__weak_cache_key = self._calculate_cache_key(dependencies) diff --git a/tests/format/dependencies.py b/tests/format/dependencies.py index e8c50d3bc..f92b89afa 100644 --- a/tests/format/dependencies.py +++ b/tests/format/dependencies.py @@ -51,6 +51,20 @@ def test_invalid_dependency_type(cli, datafiles): @pytest.mark.datafiles(DATA_DIR) +def test_invalid_strict_dependency(cli, datafiles): + project = os.path.join(str(datafiles), 'dependencies1') + result = cli.run(project=project, args=['show', 'invalidstrict.bst']) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) + + +@pytest.mark.datafiles(DATA_DIR) +def test_invalid_non_strict_dependency(cli, datafiles): + project = os.path.join(str(datafiles), 'dependencies1') + result = cli.run(project=project, args=['show', 'invalidnonstrict.bst']) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) + + +@pytest.mark.datafiles(DATA_DIR) def test_circular_dependency(cli, datafiles): project = os.path.join(str(datafiles), 'dependencies1') result = cli.run(project=project, args=['show', 'circulartarget.bst']) diff --git a/tests/format/dependencies1/elements/invalidnonstrict.bst b/tests/format/dependencies1/elements/invalidnonstrict.bst new file mode 100644 index 000000000..3cb31a077 --- /dev/null +++ b/tests/format/dependencies1/elements/invalidnonstrict.bst @@ -0,0 +1,10 @@ +kind: manual +description: | + + This is an invalid non strict dependency because it is + currently illegal to explicitly set a dependency to be + non-strict (even though this is the default). + +depends: +- filename: firstdep.bst + strict: false diff --git a/tests/format/dependencies1/elements/invalidstrict.bst b/tests/format/dependencies1/elements/invalidstrict.bst new file mode 100644 index 000000000..8893b4aa9 --- /dev/null +++ b/tests/format/dependencies1/elements/invalidstrict.bst @@ -0,0 +1,9 @@ +kind: manual +description: | + + This is an invalid strict dependency because runtime + dependencies cannot be strict. + +runtime-depends: +- filename: firstdep.bst + strict: true diff --git a/tests/frontend/show.py b/tests/frontend/show.py index 0aa720681..0d444a925 100644 --- a/tests/frontend/show.py +++ b/tests/frontend/show.py @@ -560,3 +560,57 @@ def test_max_jobs(cli, datafiles, cli_value, config_value): else: # Check that we got the explicitly set value assert loaded_value == int(expected_value) + + +# This tests that cache keys behave as expected when +# dependencies have been specified as `strict` and +# when building in strict mode. +# +# This test will: +# +# * Build the target once (and assert that it is cached) +# * Modify some local files which are imported +# by an import element which the target depends on +# * Assert that the cached state of the target element +# is as expected +# +# We run the test twice, once with an element which strict +# depends on the changing import element, and one which +# depends on it regularly. +# +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'strict-depends')) +@pytest.mark.parametrize("target, expected_state", [ + ("non-strict-depends.bst", "cached"), + ("strict-depends.bst", "waiting"), +]) +def test_strict_dependencies(cli, datafiles, target, expected_state): + project = str(datafiles) + + # Configure non strict mode, this will have + # an effect on the build and the `bst show` + # commands run via cli.get_element_states() + cli.configure({ + 'projects': { + 'test': { + 'strict': False + } + } + }) + + result = cli.run(project=project, silent=True, args=['build', target]) + result.assert_success() + + states = cli.get_element_states(project, ['base.bst', target]) + assert states['base.bst'] == 'cached' + assert states[target] == 'cached' + + # Now modify the file, effectively causing the common base.bst + # dependency to change it's cache key + hello_path = os.path.join(project, 'files', 'hello.txt') + with open(hello_path, 'w') as f: + f.write("Goodbye") + + # Now assert that we have the states we expect as a result + states = cli.get_element_states(project, ['base.bst', target]) + assert states['base.bst'] == 'buildable' + assert states[target] == expected_state diff --git a/tests/frontend/strict-depends/elements/base.bst b/tests/frontend/strict-depends/elements/base.bst new file mode 100644 index 000000000..ed6197699 --- /dev/null +++ b/tests/frontend/strict-depends/elements/base.bst @@ -0,0 +1,5 @@ +kind: import + +sources: +- kind: local + path: files diff --git a/tests/frontend/strict-depends/elements/non-strict-depends.bst b/tests/frontend/strict-depends/elements/non-strict-depends.bst new file mode 100644 index 000000000..9ab119bde --- /dev/null +++ b/tests/frontend/strict-depends/elements/non-strict-depends.bst @@ -0,0 +1,4 @@ +kind: stack + +build-depends: +- base.bst diff --git a/tests/frontend/strict-depends/elements/strict-depends.bst b/tests/frontend/strict-depends/elements/strict-depends.bst new file mode 100644 index 000000000..1e4e29414 --- /dev/null +++ b/tests/frontend/strict-depends/elements/strict-depends.bst @@ -0,0 +1,5 @@ +kind: stack + +build-depends: +- filename: base.bst + strict: true diff --git a/tests/frontend/strict-depends/files/hello.txt b/tests/frontend/strict-depends/files/hello.txt new file mode 100644 index 000000000..f62144808 --- /dev/null +++ b/tests/frontend/strict-depends/files/hello.txt @@ -0,0 +1 @@ +pony diff --git a/tests/frontend/strict-depends/project.conf b/tests/frontend/strict-depends/project.conf new file mode 100644 index 000000000..627522526 --- /dev/null +++ b/tests/frontend/strict-depends/project.conf @@ -0,0 +1,2 @@ +name: test +element-path: elements |