source.py: ensure previous track refs are updatedtristan/backport-previous-sources

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

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