From cec4676346f9f77585110bac1160130eb32ad9f9 Mon Sep 17 00:00:00 2001 From: Raoul Hidalgo Charman Date: Thu, 2 May 2019 16:01:13 +0100 Subject: source.py: ensure previous track refs are updated Updates the refs in the job process (but doesn't write), to ensure following sources can see consistency of previous sourcse has been updated. `_save_ref` is renamed `_set_ref` with writing to file now optional. This also changes the previous_source_access test to use a remote, so that it actually tests this cornercase. Fixes #1010 --- buildstream/_scheduler/queues/trackqueue.py | 5 +-- buildstream/source.py | 38 ++++++++++------------ tests/sources/previous_source_access.py | 8 +++++ .../previous_source_access/elements/target.bst | 4 +-- tests/sources/previous_source_access/project.conf | 3 ++ 5 files changed, 33 insertions(+), 25 deletions(-) diff --git a/buildstream/_scheduler/queues/trackqueue.py b/buildstream/_scheduler/queues/trackqueue.py index d7e6546f3..72a79a532 100644 --- a/buildstream/_scheduler/queues/trackqueue.py +++ b/buildstream/_scheduler/queues/trackqueue.py @@ -53,9 +53,10 @@ class TrackQueue(Queue): if status == JobStatus.FAIL: return - # Set the new refs in the main process one by one as they complete + # Set the new refs in the main process one by one as they complete, + # writing to bst files this time for unique_id, new_ref in result: source = Plugin._lookup(unique_id) - source._save_ref(new_ref) + source._set_ref(new_ref, save=True) element._tracking_done() diff --git a/buildstream/source.py b/buildstream/source.py index 2a3218cfc..ed4dd9617 100644 --- a/buildstream/source.py +++ b/buildstream/source.py @@ -701,24 +701,6 @@ class Source(Plugin): return key - # Wrapper for set_ref(), also returns whether it changed. - # - def _set_ref(self, ref, node): - current_ref = self.get_ref() - changed = False - - # This comparison should work even for tuples and lists, - # but we're mostly concerned about simple strings anyway. - if current_ref != ref: - changed = True - - # Set the ref regardless of whether it changed, the - # TrackQueue() will want to update a specific node with - # the ref, regardless of whether the original has changed. - self.set_ref(ref, node) - - return changed - # _project_refs(): # # Gets the appropriate ProjectRefs object for this source, @@ -795,7 +777,7 @@ class Source(Plugin): return redundant_ref - # _save_ref() + # _set_ref() # # Persists the ref for this source. This will decide where to save the # ref, or refuse to persist it, depending on active ref-storage project @@ -803,6 +785,7 @@ class Source(Plugin): # # Args: # new_ref (smth): The new reference to save + # save (bool): Whether to write the new reference to file or not # # Returns: # (bool): Whether the ref has changed @@ -810,7 +793,7 @@ class Source(Plugin): # Raises: # (SourceError): In the case we encounter errors saving a file to disk # - def _save_ref(self, new_ref): + def _set_ref(self, new_ref, *, save): context = self._get_context() project = self._get_project() @@ -838,7 +821,17 @@ class Source(Plugin): # # Step 2 - Set the ref in memory, and determine changed state # - if not self._set_ref(new_ref, node): + current_ref = self.get_ref() # pylint: disable=assignment-from-no-return + + # Set the ref regardless of whether it changed, the + # TrackQueue() will want to update a specific node with + # the ref, regardless of whether the original has changed. + self.set_ref(new_ref, node) + + if current_ref == new_ref or not save: + # Note: We do not look for and propagate changes at this point + # which might result in desync depending if something changes about + # tracking in the future. For now, this is quite safe. return False def do_save_refs(refs): @@ -902,6 +895,9 @@ class Source(Plugin): if current_ref != new_ref: self.info("Found new revision: {}".format(new_ref)) + # Save ref in local process for subsequent sources + self._set_ref(new_ref, save=False) + return new_ref # _requires_previous_sources() diff --git a/tests/sources/previous_source_access.py b/tests/sources/previous_source_access.py index f7045383c..a04283257 100644 --- a/tests/sources/previous_source_access.py +++ b/tests/sources/previous_source_access.py @@ -1,6 +1,7 @@ import os import pytest +from buildstream import _yaml from tests.testutils import cli DATA_DIR = os.path.join( @@ -17,6 +18,13 @@ DATA_DIR = os.path.join( def test_custom_transform_source(cli, tmpdir, datafiles): project = os.path.join(datafiles.dirname, datafiles.basename) + # Set the project_dir alias in project.conf to the path to the tested project + project_config_path = os.path.join(project, "project.conf") + project_config = _yaml.load(project_config_path) + aliases = _yaml.node_get(project_config, dict, "aliases") + aliases["project_dir"] = "file://{}".format(project) + _yaml.dump(_yaml.node_sanitize(project_config), project_config_path) + # Ensure we can track result = cli.run(project=project, args=[ 'track', 'target.bst' diff --git a/tests/sources/previous_source_access/elements/target.bst b/tests/sources/previous_source_access/elements/target.bst index c9d3b9bb6..fd54a28d0 100644 --- a/tests/sources/previous_source_access/elements/target.bst +++ b/tests/sources/previous_source_access/elements/target.bst @@ -1,6 +1,6 @@ kind: import sources: -- kind: local - path: files/file +- kind: remote + url: project_dir:/files/file - kind: foo_transform diff --git a/tests/sources/previous_source_access/project.conf b/tests/sources/previous_source_access/project.conf index 1749b3dba..5d50ec2c5 100644 --- a/tests/sources/previous_source_access/project.conf +++ b/tests/sources/previous_source_access/project.conf @@ -3,6 +3,9 @@ name: foo element-path: elements +aliases: + project_dir: file://{project_dir} + plugins: - origin: local path: plugins/sources -- cgit v1.2.1