source.py: ensure previous track refs are updatedraoul/1010-multiple-track-fix

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, 31 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 0c07dd41e..b1346eb1b 100644
--- a/buildstream/source.py
+++ b/buildstream/source.py
@@ -742,24 +742,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()  # pylint: disable=assignment-from-no-return
-        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,
@@ -836,7 +818,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
@@ -844,6 +826,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
@@ -851,7 +834,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()
@@ -891,7 +874,15 @@ class Source(Plugin):
         #
         clean = _yaml.node_sanitize(node, dict_type=dict)
         to_modify = _yaml.node_sanitize(node, dict_type=dict)
-        if not self._set_ref(new_ref, to_modify):
+
+        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, to_modify)
+
+        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.
@@ -1008,6 +999,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 a0661ca98..800e0ced5 100644
--- a/tests/sources/previous_source_access.py
+++ b/tests/sources/previous_source_access.py
@@ -4,6 +4,7 @@
 import os
 import pytest
 
+from buildstream import _yaml
 from buildstream.testing import cli  # pylint: disable=unused-import
 
 DATA_DIR = os.path.join(
@@ -20,6 +21,13 @@ DATA_DIR = os.path.join(
 def test_custom_transform_source(cli, datafiles):
     project = str(datafiles)
 
+    # 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")
+    _yaml.node_set(aliases, "project_dir", "file://{}".format(project))
+    _yaml.dump(project_config, project_config_path)
+
     # Ensure we can track
     result = cli.run(project=project, args=[
         'source', '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