Merge branch 'tristan/fix-overriding-links' into 'master'

Fix junction overrides to resolve links

Closes #1398

See merge request BuildStream/buildstream!2078

11 files changed, 262 insertions, 5 deletions

diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 080ec0093..c260eefe5 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -74,6 +74,7 @@ class Loader:
 
         self._meta_elements = {}  # Dict of resolved meta elements by name
         self._elements = {}  # Dict of elements
+        self._links = {}  # Dict of link target target paths indexed by link element paths
         self._loaders = {}  # Dict of junction loaders
         self._loader_search_provenances = {}  # Dictionary of provenance nodes of ongoing child loader searches
 
@@ -196,7 +197,7 @@ class Loader:
         loader = self
 
         circular_provenance_node = self._loader_search_provenances.get(name, None)
-        if circular_provenance_node:
+        if circular_provenance_node and load_subprojects:
 
             assert provenance_node
 
@@ -209,12 +210,14 @@ class Loader:
                 detail=detail,
             )
 
-        self._loader_search_provenances[name] = provenance_node
+        if load_subprojects:
+            self._loader_search_provenances[name] = provenance_node
 
         for junction_name in junction_path:
             loader = loader._get_loader(junction_name, provenance_node, load_subprojects=load_subprojects)
 
-        del self._loader_search_provenances[name]
+        if load_subprojects:
+            del self._loader_search_provenances[name]
 
         return loader
 
@@ -322,8 +325,73 @@ class Loader:
 
         self._elements[filename] = element
 
+        #
+        # Update link caches in the ancestry
+        #
+        if element.link_target is not None:
+            link_path = filename
+            target_path = element.link_target.as_str()  # pylint: disable=no-member
+
+            # First resolve the link in this loader's cache
+            #
+            self._resolve_link(link_path, target_path)
+
+            # Now resolve the link in parent project loaders
+            #
+            loader = self
+            while loader._parent:
+                junction = loader.project.junction
+                link_path = junction.name + ":" + link_path
+                target_path = junction.name + ":" + target_path
+
+                # Resolve the link
+                loader = loader._parent
+                loader._resolve_link(link_path, target_path)
+
         return element
 
+    # _resolve_link():
+    #
+    # Resolves a link in the loader's link cache.
+    #
+    # This will first insert the new link -> target relationship
+    # into the cache, and will also update any existing targets
+    # which might have pointed to this link, to point to the new
+    # target instead.
+    #
+    # Args:
+    #    link_path (str): The local project relative real path to a link
+    #    target_path (str): The new target for this link
+    #
+    def _resolve_link(self, link_path, target_path):
+        self._links[link_path] = target_path
+
+        for cached_link_path, cached_target_path in self._links.items():
+            if self._expand_link(cached_target_path) == link_path:
+                self._links[cached_link_path] = target_path
+
+    # _expand_link():
+    #
+    # Expands any links in the provided path and returns a real path with
+    # known link elements substituted for their targets.
+    #
+    # Args:
+    #    path (str): A project relative path
+    #
+    # Returns:
+    #    (str): The same path with any links expanded
+    #
+    def _expand_link(self, path):
+
+        # FIXME: This simply returns the first link, maybe
+        #        this needs to be more iterative, or sorted by
+        #        number of path components, or smth
+        for link, target in self._links.items():
+            if path.startswith(link):
+                return target + path[len(link) :]
+
+        return path
+
     # _load_file():
     #
     # Semi-Iteratively load bst files
@@ -469,6 +537,37 @@ class Loader:
                 check_elements.remove(this_element)
                 validated.add(this_element)
 
+    # _search_for_local_override():
+    #
+    # Search this project's active override list for an override, while
+    # considering any link elements.
+    #
+    # Args:
+    #    override_path (str): The real relative path to search for
+    #
+    # Returns:
+    #    (ScalarNode): The overridding node from this project's junction, or None
+    #
+    def _search_for_local_override(self, override_path):
+        junction = self.project.junction
+        if junction is None:
+            return None
+
+        # Try the override without any link substitutions first
+        with suppress(KeyError):
+            return junction.overrides[override_path]
+
+        #
+        # If we did not get an exact match here, we might still have
+        # an override where a link was used to specify the override.
+        #
+        for override_key, override_node in junction.overrides.items():
+            resolved_path = self._expand_link(override_key)
+            if resolved_path == override_path:
+                return override_node
+
+        return None
+
     # _search_for_override():
     #
     # Search parent projects for an overridden subproject to replace this junction.
@@ -490,7 +589,7 @@ class Loader:
         overriding_loaders = []
         while loader._parent:
             junction = loader.project.junction
-            override_node = junction.overrides.get(override_path, None)
+            override_node = loader._search_for_local_override(override_path)
             if override_node:
                 overriding_loaders.append((loader._parent, override_node))
 
@@ -681,8 +780,95 @@ class Loader:
         loader = project.loader
         self._loaders[filename] = loader
 
+        # Now we've loaded a junction and it's project, we need to try to shallow
+        # load the overrides of this project and any projects in the ancestry which
+        # have overrides referring to this freshly loaded project.
+        #
+        # This is to ensure that link elements have been resolved as much as possible
+        # before we try to look for an override.
+        #
+        iter_loader = loader
+        while iter_loader._parent:
+            iter_loader._shallow_load_overrides()
+            iter_loader = iter_loader._parent
+
         return loader
 
+    # _shallow_load_overrides():
+    #
+    # Loads any of the override elements on this loader's junction
+    #
+    def _shallow_load_overrides(self):
+        if not self.project.junction:
+            return
+
+        junction = self.project.junction
+
+        # Iterate over the keys, we want to ensure that links are resolved for
+        # override paths specified in junctions, while the targets of these paths
+        # are not consequential.
+        #
+        for override_path, override_target in junction.overrides.items():
+
+            # Ensure that we resolve indirect links, in case that shallow loading
+            # an element results in loading a link, we need to discover if it's
+            # target is also a link.
+            #
+            path = override_path
+            provenance_node = override_target
+            while path is not None:
+                path, provenance_node = self._shallow_load_path(path, provenance_node)
+
+    # _shallow_load_path()
+    #
+    # Perform a shallow load of an element by it's relative path, this is
+    # used to load elements which might be specified by their path and might
+    # not be used in the resulting load, like paths to elements overridden by
+    # junctions.
+    #
+    # It is only important to shallow load these referenced elements in case
+    # they are links which need to be known later on.
+    #
+    # Args:
+    #    path (str): The path to load
+    #    provenance_node (Node): The node to use for provenance
+    #
+    # Returns:
+    #    (str): The target of the loaded link element, if it was a link element
+    #           and it could be loaded presently, otherwise None.
+    #    (ScalarNode): The link target real node, if a link target was returned
+    #
+    def _shallow_load_path(self, path, provenance_node):
+        if ":" in path:
+            junction, element_name = path.rsplit(":", 1)
+            target_loader = self.get_loader(junction, provenance_node, load_subprojects=False)
+
+            # Subproject not loaded, discard this shallow load attempt
+            #
+            if target_loader is None:
+                return None, None
+        else:
+            junction = None
+            element_name = path
+            target_loader = self
+
+        # If the element is already loaded in the target loader, then there
+        # is no need for a shallow load.
+        if element_name in target_loader._elements:
+            element = target_loader._elements[element_name]
+        else:
+            # Shallow load the the element.
+            element = target_loader._load_file_no_deps(element_name, provenance_node)
+
+        if element.link_target:
+            link_target = element.link_target.as_str()
+            if junction:
+                return "{}:{}".format(junction, link_target), element.link_target
+
+            return link_target, element.link_target
+
+        return None, None
+
     # _parse_name():
     #
     # Get junction and base name of element along with loader for the sub-project
diff --git a/tests/format/junctions.py b/tests/format/junctions.py
index f5858f7e2..c4eea2d1c 100644
--- a/tests/format/junctions.py
+++ b/tests/format/junctions.py
@@ -400,12 +400,25 @@ def test_full_path_not_found(cli, tmpdir, datafiles, target, provenance):
     [
         # Test that we can override a subproject junction of a subproject
         ("target-overridden-subsubproject.bst", "subsubsub.txt"),
+        # Test that we can override a subproject junction of a subproject, when that junction is a link
+        ("target-overridden-subsubproject-link.bst", "subsubsub.txt"),
         # Test that we can override a subproject junction of a subproject's subproject
         ("target-overridden-subsubsubproject.bst", "surprise.txt"),
+        # Test that we can override a subproject junction of a subproject's subproject, which using links to address them
+        ("target-overridden-subsubsubproject-link.bst", "surprise.txt"),
+        # Test that we can override a subproject junction of a subproject's subproject, using various levels of links indirection
+        ("target-overridden-subsubsubproject-indirect-link.bst", "surprise.txt"),
         # Test that we can override a subproject junction with a deep subproject path
         ("target-overridden-with-deepsubproject.bst", "deepsurprise.txt"),
     ],
-    ids=["override-subproject", "override-subsubproject", "override-subproject-with-subsubproject"],
+    ids=[
+        "override-subproject",
+        "override-subproject-link",
+        "override-subsubproject",
+        "override-subsubproject-link",
+        "override-subsubproject-indirect-link",
+        "override-subproject-with-subsubproject",
+    ],
 )
 def test_overrides(cli, tmpdir, datafiles, target, expected):
     project = os.path.join(str(datafiles), "overrides")
diff --git a/tests/format/junctions/overrides/subproject-with-deep-override-indirect-link.bst b/tests/format/junctions/overrides/subproject-with-deep-override-indirect-link.bst
new file mode 100644
index 000000000..c356f3ff0
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject-with-deep-override-indirect-link.bst
@@ -0,0 +1,8 @@
+kind: junction
+sources:
+- kind: local
+  path: subproject
+
+config:
+  overrides:
+    subsubsubproject-indirect-link.bst: overridden-subsubsubproject.bst
diff --git a/tests/format/junctions/overrides/subproject-with-deep-override-link.bst b/tests/format/junctions/overrides/subproject-with-deep-override-link.bst
new file mode 100644
index 000000000..5e7f0254a
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject-with-deep-override-link.bst
@@ -0,0 +1,8 @@
+kind: junction
+sources:
+- kind: local
+  path: subproject
+
+config:
+  overrides:
+    subsubproject-link.bst:subsubsubproject.bst: overridden-subsubsubproject.bst
diff --git a/tests/format/junctions/overrides/subproject-with-overridden-link.bst b/tests/format/junctions/overrides/subproject-with-overridden-link.bst
new file mode 100644
index 000000000..7688e882a
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject-with-overridden-link.bst
@@ -0,0 +1,8 @@
+kind: junction
+sources:
+- kind: local
+  path: subproject
+
+config:
+  overrides:
+    subsubproject-link.bst: overridden-subsubproject.bst
diff --git a/tests/format/junctions/overrides/subproject/subsubproject-link.bst b/tests/format/junctions/overrides/subproject/subsubproject-link.bst
new file mode 100644
index 000000000..246a5e41c
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject/subsubproject-link.bst
@@ -0,0 +1,4 @@
+kind: link
+
+config:
+  target: subsubproject.bst
diff --git a/tests/format/junctions/overrides/subproject/subsubproject/subsubsubproject-link.bst b/tests/format/junctions/overrides/subproject/subsubproject/subsubsubproject-link.bst
new file mode 100644
index 000000000..1c7d844a3
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject/subsubproject/subsubsubproject-link.bst
@@ -0,0 +1,4 @@
+kind: link
+
+config:
+  target: subsubsubproject.bst
diff --git a/tests/format/junctions/overrides/subproject/subsubsubproject-indirect-link.bst b/tests/format/junctions/overrides/subproject/subsubsubproject-indirect-link.bst
new file mode 100644
index 000000000..f6a16edae
--- /dev/null
+++ b/tests/format/junctions/overrides/subproject/subsubsubproject-indirect-link.bst
@@ -0,0 +1,4 @@
+kind: link
+
+config:
+  target: subsubproject-link.bst:subsubsubproject-link.bst
diff --git a/tests/format/junctions/overrides/target-overridden-subsubproject-link.bst b/tests/format/junctions/overrides/target-overridden-subsubproject-link.bst
new file mode 100644
index 000000000..d81d39a2a
--- /dev/null
+++ b/tests/format/junctions/overrides/target-overridden-subsubproject-link.bst
@@ -0,0 +1,8 @@
+kind: stack
+
+# Similar test as target-overridden-subsubproject.bst, but we
+# test that this works equally if we are referring to a subsubproject
+# which is in fact a link.
+#
+depends:
+- subproject-with-overridden-link.bst:subsubproject-link.bst:target.bst
diff --git a/tests/format/junctions/overrides/target-overridden-subsubsubproject-indirect-link.bst b/tests/format/junctions/overrides/target-overridden-subsubsubproject-indirect-link.bst
new file mode 100644
index 000000000..1ce1c62cc
--- /dev/null
+++ b/tests/format/junctions/overrides/target-overridden-subsubsubproject-indirect-link.bst
@@ -0,0 +1,7 @@
+kind: stack
+
+# Similar test as target-overridden-subsubsubproject.bst, except that
+# we use links in the addressing of overrides.
+#
+depends:
+- subproject-with-deep-override-indirect-link.bst:subsubproject.bst:subsubsubproject.bst:target.bst
diff --git a/tests/format/junctions/overrides/target-overridden-subsubsubproject-link.bst b/tests/format/junctions/overrides/target-overridden-subsubsubproject-link.bst
new file mode 100644
index 000000000..580632f0c
--- /dev/null
+++ b/tests/format/junctions/overrides/target-overridden-subsubsubproject-link.bst
@@ -0,0 +1,7 @@
+kind: stack
+
+# Similar test as target-overridden-subsubsubproject.bst, except that
+# we use links in the addressing of overrides.
+#
+depends:
+- subproject-with-deep-override-link.bst:subsubproject.bst:subsubsubproject.bst:target.bst