Merge branch 'tristan/bst-1/strict-rebuild' into 'bst-1'
Support strict build dependencies (bst 1)
See merge request BuildStream/buildstream!1574
diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index b861e86..ec929ea 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -474,6 +474,8 @@
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)
return meta_element
diff --git a/buildstream/_loader/metaelement.py b/buildstream/_loader/metaelement.py
index c13d559..370ce55 100644
--- a/buildstream/_loader/metaelement.py
+++ b/buildstream/_loader/metaelement.py
@@ -54,4 +54,5 @@
self.sandbox = sandbox
self.build_dependencies = []
self.dependencies = []
+ self.strict_dependencies = []
self.first_pass = first_pass
diff --git a/buildstream/_loader/types.py b/buildstream/_loader/types.py
index eb6932b..193fb5d 100644
--- a/buildstream/_loader/types.py
+++ b/buildstream/_loader/types.py
@@ -46,6 +46,7 @@
DIRECTORY = "directory"
JUNCTION = "junction"
SANDBOX = "sandbox"
+ STRICT = "strict"
# Dependency()
@@ -68,13 +69,14 @@
self.name = dep
self.dep_type = default_dep_type
self.junction = None
+ self.strict = False
elif isinstance(dep, Mapping):
if default_dep_type:
- _yaml.node_validate(dep, ['filename', 'junction'])
+ _yaml.node_validate(dep, ['filename', 'junction', 'strict'])
dep_type = default_dep_type
else:
- _yaml.node_validate(dep, ['filename', 'type', 'junction'])
+ _yaml.node_validate(dep, ['filename', 'type', 'junction', 'strict'])
# Make type optional, for this we set it to None
dep_type = _yaml.node_get(dep, str, Symbol.TYPE, default_value=None)
@@ -89,11 +91,33 @@
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)
+ self.strict = _yaml.node_get(dep, bool, Symbol.STRICT, default_value=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 is False and Symbol.STRICT in dep:
+ provenance = _yaml.node_get_provenance(dep, key=Symbol.STRICT)
+ raise LoadError(LoadErrorReason.INVALID_DATA,
+ "{}: Setting 'strict' to False is unsupported"
+ .format(provenance))
else:
raise LoadError(LoadErrorReason.INVALID_DATA,
"{}: Dependency is not specified as a string or a dictionary".format(provenance))
+ # Only build dependencies are allowed to be strict
+ #
+ if self.strict and self.dep_type == Symbol.RUNTIME:
+ raise LoadError(LoadErrorReason.INVALID_DATA,
+ "{}: Runtime dependency {} specified as `strict`.".format(self.provenance, self.name),
+ 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/buildstream/element.py b/buildstream/element.py
index 5f0e300..703f062 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -201,6 +201,7 @@
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_dependencies = set() # Direct reverse dependency Elements
self.__ready_for_runtime = False # Wether the element has all its dependencies ready and has a cache key
self.__sources = [] # List of Sources
@@ -939,6 +940,9 @@
element.__build_dependencies.append(dependency)
dependency.__reverse_dependencies.add(element)
+ if meta_dep in meta.strict_dependencies:
+ element.__strict_dependencies.append(dependency)
+
return element
# _get_redundant_source_refs()
@@ -1069,16 +1073,20 @@
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.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.name
+ for e in self.dependencies(Scope.BUILD)
+ ]
self.__weak_cache_key = self.__calculate_cache_key(dependencies)
diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst
index f69d4a4..5c9d640 100644
--- a/doc/source/format_declaring.rst
+++ b/doc/source/format_declaring.rst
@@ -350,6 +350,7 @@
- filename: foo.bst
type: build
junction: baseproject.bst
+ strict: false
Attributes:
@@ -380,6 +381,16 @@
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 c707cd0..8b6cb58 100644
--- a/doc/source/using_config.rst
+++ b/doc/source/using_config.rst
@@ -65,6 +65,8 @@
If you give a list of URLs, earlier entries in the list will have higher
priority than later ones.
+.. _user_config_strict_mode:
+
Strict build plan
~~~~~~~~~~~~~~~~~
The strict build plan option decides whether you want elements
diff --git a/tests/frontend/show.py b/tests/frontend/show.py
index 8bb0bab..92fa1f5 100644
--- a/tests/frontend/show.py
+++ b/tests/frontend/show.py
@@ -11,10 +11,8 @@
from . import configure_project
# Project directory
-DATA_DIR = os.path.join(
- os.path.dirname(os.path.realpath(__file__)),
- "project",
-)
+TOP_DIR = os.path.dirname(os.path.realpath(__file__))
+DATA_DIR = os.path.join(TOP_DIR, "project")
@pytest.mark.datafiles(DATA_DIR)
@@ -371,3 +369,57 @@
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(TOP_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, 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, 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 0000000..ed61976
--- /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 0000000..9ab119b
--- /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 0000000..1e4e294
--- /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 0000000..f621448
--- /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 0000000..6275225
--- /dev/null
+++ b/tests/frontend/strict-depends/project.conf
@@ -0,0 +1,2 @@
+name: test
+element-path: elements
diff --git a/tests/loader/dependencies.py b/tests/loader/dependencies.py
index cb750fc..e4b9c73 100644
--- a/tests/loader/dependencies.py
+++ b/tests/loader/dependencies.py
@@ -124,6 +124,28 @@
@pytest.mark.datafiles(DATA_DIR)
+def test_invalid_strict_dependency(cli, datafiles):
+ basedir = os.path.join(datafiles.dirname, datafiles.basename)
+ loader = make_loader(basedir)
+
+ with pytest.raises(LoadError) as exc:
+ element = loader.load(['elements/invalidstrict.bst'])[0]
+
+ assert (exc.value.reason == LoadErrorReason.INVALID_DATA)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_invalid_non_strict_dependency(cli, datafiles):
+ basedir = os.path.join(datafiles.dirname, datafiles.basename)
+ loader = make_loader(basedir)
+
+ with pytest.raises(LoadError) as exc:
+ element = loader.load(['elements/invalidnonstrict.bst'])[0]
+
+ assert (exc.value.reason == LoadErrorReason.INVALID_DATA)
+
+
+@pytest.mark.datafiles(DATA_DIR)
def test_build_dependency(datafiles):
basedir = os.path.join(datafiles.dirname, datafiles.basename)
loader = make_loader(basedir)
diff --git a/tests/loader/dependencies/elements/invalidnonstrict.bst b/tests/loader/dependencies/elements/invalidnonstrict.bst
new file mode 100644
index 0000000..3cb31a0
--- /dev/null
+++ b/tests/loader/dependencies/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/loader/dependencies/elements/invalidstrict.bst b/tests/loader/dependencies/elements/invalidstrict.bst
new file mode 100644
index 0000000..8893b4a
--- /dev/null
+++ b/tests/loader/dependencies/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
