Merge branch 'tristan/missing-file-errors-1.2' into 'bst-1.2'

Improve error reporting when files are not found  (1.2)

See merge request BuildStream/buildstream!1217

18 files changed, 122 insertions, 6 deletions

diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index c6bc87726..eec60b193 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -202,11 +202,12 @@ class Loader():
     #    rewritable (bool): Whether we should load in round trippable mode
     #    ticker (callable): A callback to report loaded filenames to the frontend
     #    fetch_subprojects (bool): Whether to fetch subprojects while loading
+    #    provenance (Provenance): The location from where the file was referred to, or None
     #
     # Returns:
     #    (LoadElement): A loaded LoadElement
     #
-    def _load_file(self, filename, rewritable, ticker, fetch_subprojects):
+    def _load_file(self, filename, rewritable, ticker, fetch_subprojects, provenance=None):
 
         # Silently ignore already loaded files
         if filename in self._elements:
@@ -222,21 +223,34 @@ class Loader():
             node = _yaml.load(fullpath, shortname=filename, copy_tree=rewritable, project=self.project)
         except LoadError as e:
             if e.reason == LoadErrorReason.MISSING_FILE:
+
+                if self.project.junction:
+                    message = "Could not find element '{}' in project referred to by junction element '{}'" \
+                              .format(filename, self.project.junction.name)
+                else:
+                    message = "Could not find element '{}' in elements directory '{}'".format(filename, self._basedir)
+
+                if provenance:
+                    message = "{}: {}".format(provenance, message)
+
                 # If we can't find the file, try to suggest plausible
                 # alternatives by stripping the element-path from the given
                 # filename, and verifying that it exists.
-                message = "Could not find element '{}' in elements directory '{}'".format(filename, self._basedir)
                 detail = None
                 elements_dir = os.path.relpath(self._basedir, self.project.directory)
                 element_relpath = os.path.relpath(filename, elements_dir)
                 if filename.startswith(elements_dir) and os.path.exists(os.path.join(self._basedir, element_relpath)):
                     detail = "Did you mean '{}'?".format(element_relpath)
+
                 raise LoadError(LoadErrorReason.MISSING_FILE,
                                 message, detail=detail) from e
+
             elif e.reason == LoadErrorReason.LOADING_DIRECTORY:
                 # If a <directory>.bst file exists in the element path,
                 # let's suggest this as a plausible alternative.
                 message = str(e)
+                if provenance:
+                    message = "{}: {}".format(provenance, message)
                 detail = None
                 if os.path.exists(os.path.join(self._basedir, filename + '.bst')):
                     element_name = filename + '.bst'
@@ -262,13 +276,14 @@ class Loader():
         # Load all dependency files for the new LoadElement
         for dep in element.deps:
             if dep.junction:
-                self._load_file(dep.junction, rewritable, ticker, fetch_subprojects)
+                self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, dep.provenance)
                 loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
                                           fetch_subprojects=fetch_subprojects)
             else:
                 loader = self
 
-            dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects)
+            dep_element = loader._load_file(dep.name, rewritable, ticker,
+                                            fetch_subprojects, dep.provenance)
 
             if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
                 raise LoadError(LoadErrorReason.INVALID_DATA,
diff --git a/buildstream/_yaml.py b/buildstream/_yaml.py
index 4ee12a18c..bd3ab15da 100644
--- a/buildstream/_yaml.py
+++ b/buildstream/_yaml.py
@@ -70,7 +70,11 @@ class Provenance():
 
     # Convert a Provenance to a string for error reporting
     def __str__(self):
-        return "{} [line {:d} column {:d}]".format(self.filename.shortname, self.line, self.col)
+        filename = self.filename.shortname
+        if self.filename.project and self.filename.project.junction:
+            filename = "{}:{}".format(self.filename.project.junction.name, self.filename.shortname)
+
+        return "{} [line {:d} column {:d}]".format(filename, self.line, self.col)
 
     # Abstract method
     def clone(self):
diff --git a/tests/format/project.py b/tests/format/project.py
index a27f3a5f7..abd0c4238 100644
--- a/tests/format/project.py
+++ b/tests/format/project.py
@@ -27,6 +27,26 @@ def test_missing_project_name(cli, datafiles):
 
 
 @pytest.mark.datafiles(os.path.join(DATA_DIR))
+def test_missing_element(cli, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename, "missing-element")
+    result = cli.run(project=project, args=['show', 'manual.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
+
+    # Assert that we have the expected provenance encoded into the error
+    assert "manual.bst [line 4 column 2]" in result.stderr
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR))
+def test_missing_junction(cli, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename, "missing-junction")
+    result = cli.run(project=project, args=['show', 'manual.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
+
+    # Assert that we have the expected provenance encoded into the error
+    assert "manual.bst [line 4 column 2]" in result.stderr
+
+
+@pytest.mark.datafiles(os.path.join(DATA_DIR))
 def test_empty_project_name(cli, datafiles):
     project = os.path.join(datafiles.dirname, datafiles.basename, "emptyname")
     result = cli.run(project=project, args=['workspace', 'list'])
diff --git a/tests/format/project/missing-element/manual.bst b/tests/format/project/missing-element/manual.bst
new file mode 100644
index 000000000..6dd3d7178
--- /dev/null
+++ b/tests/format/project/missing-element/manual.bst
@@ -0,0 +1,4 @@
+kind: manual
+
+depends:
+- missing.bst
diff --git a/tests/format/project/missing-element/project.conf b/tests/format/project/missing-element/project.conf
new file mode 100644
index 000000000..b32753625
--- /dev/null
+++ b/tests/format/project/missing-element/project.conf
@@ -0,0 +1 @@
+name: test
diff --git a/tests/format/project/missing-junction/manual.bst b/tests/format/project/missing-junction/manual.bst
new file mode 100644
index 000000000..0e0ae0860
--- /dev/null
+++ b/tests/format/project/missing-junction/manual.bst
@@ -0,0 +1,5 @@
+kind: manual
+
+depends:
+- filename: element.bst
+  junction: missing.bst
diff --git a/tests/format/project/missing-junction/project.conf b/tests/format/project/missing-junction/project.conf
new file mode 100644
index 000000000..b32753625
--- /dev/null
+++ b/tests/format/project/missing-junction/project.conf
@@ -0,0 +1 @@
+name: test
diff --git a/tests/loader/junctions.py b/tests/loader/junctions.py
index a02961fb5..aee164396 100644
--- a/tests/loader/junctions.py
+++ b/tests/loader/junctions.py
@@ -3,7 +3,7 @@ import pytest
 import shutil
 
 from buildstream import _yaml, ElementError
-from buildstream._exceptions import LoadError, LoadErrorReason
+from buildstream._exceptions import ErrorDomain, LoadError, LoadErrorReason
 from tests.testutils import cli, create_repo
 from tests.testutils.site import HAVE_GIT
 
@@ -48,6 +48,36 @@ def test_simple_build(cli, tmpdir, datafiles):
 
 
 @pytest.mark.datafiles(DATA_DIR)
+def test_missing_file_in_subproject(cli, datafiles):
+    project = os.path.join(str(datafiles), 'missing-element')
+    result = cli.run(project=project, args=['show', 'target.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
+
+    # Assert that we have the expected provenance encoded into the error
+    assert "target.bst [line 4 column 2]" in result.stderr
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_missing_file_in_subsubproject(cli, datafiles):
+    project = os.path.join(str(datafiles), 'missing-element')
+    result = cli.run(project=project, args=['show', 'sub-target.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
+
+    # Assert that we have the expected provenance encoded into the error
+    assert "junction-A.bst:target.bst [line 4 column 2]" in result.stderr
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_missing_junction_in_subproject(cli, datafiles):
+    project = os.path.join(str(datafiles), 'missing-element')
+    result = cli.run(project=project, args=['show', 'sub-target-bad-junction.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
+
+    # Assert that we have the expected provenance encoded into the error
+    assert "junction-A.bst:bad-junction-target.bst [line 4 column 2]" in result.stderr
+
+
+@pytest.mark.datafiles(DATA_DIR)
 def test_nested_simple(cli, tmpdir, datafiles):
     foo = os.path.join(str(datafiles), 'foo')
     copy_subprojects(foo, datafiles, ['base'])
diff --git a/tests/loader/junctions/missing-element/junction-A.bst b/tests/loader/junctions/missing-element/junction-A.bst
new file mode 100644
index 000000000..74079f990
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junction-A.bst
@@ -0,0 +1,4 @@
+kind: junction
+sources:
+- kind: local
+  path: junctionA
diff --git a/tests/loader/junctions/missing-element/junctionA/bad-junction-target.bst b/tests/loader/junctions/missing-element/junctionA/bad-junction-target.bst
new file mode 100644
index 000000000..c07c198cd
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junctionA/bad-junction-target.bst
@@ -0,0 +1,5 @@
+kind: manual
+
+depends:
+- filename: noelement.bst
+  junction: missing-junction.bst
diff --git a/tests/loader/junctions/missing-element/junctionA/junction-B.bst b/tests/loader/junctions/missing-element/junctionA/junction-B.bst
new file mode 100644
index 000000000..bc66d7851
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junctionA/junction-B.bst
@@ -0,0 +1,4 @@
+kind: junction
+sources:
+- kind: local
+  path: junctionB
diff --git a/tests/loader/junctions/missing-element/junctionA/junctionB/project.conf b/tests/loader/junctions/missing-element/junctionA/junctionB/project.conf
new file mode 100644
index 000000000..41b8d6c72
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junctionA/junctionB/project.conf
@@ -0,0 +1 @@
+name: projectB
diff --git a/tests/loader/junctions/missing-element/junctionA/project.conf b/tests/loader/junctions/missing-element/junctionA/project.conf
new file mode 100644
index 000000000..5f6ab28a2
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junctionA/project.conf
@@ -0,0 +1 @@
+name: projectA
diff --git a/tests/loader/junctions/missing-element/junctionA/target.bst b/tests/loader/junctions/missing-element/junctionA/target.bst
new file mode 100644
index 000000000..9c3d0bf0e
--- /dev/null
+++ b/tests/loader/junctions/missing-element/junctionA/target.bst
@@ -0,0 +1,5 @@
+kind: stack
+
+depends:
+- filename: missing.bst
+  junction: junction-B.bst
diff --git a/tests/loader/junctions/missing-element/project.conf b/tests/loader/junctions/missing-element/project.conf
new file mode 100644
index 000000000..b32753625
--- /dev/null
+++ b/tests/loader/junctions/missing-element/project.conf
@@ -0,0 +1 @@
+name: test
diff --git a/tests/loader/junctions/missing-element/sub-target-bad-junction.bst b/tests/loader/junctions/missing-element/sub-target-bad-junction.bst
new file mode 100644
index 000000000..f48f6cec9
--- /dev/null
+++ b/tests/loader/junctions/missing-element/sub-target-bad-junction.bst
@@ -0,0 +1,5 @@
+kind: stack
+
+depends:
+- filename: bad-junction-target.bst
+  junction: junction-A.bst
diff --git a/tests/loader/junctions/missing-element/sub-target.bst b/tests/loader/junctions/missing-element/sub-target.bst
new file mode 100644
index 000000000..79e8bc684
--- /dev/null
+++ b/tests/loader/junctions/missing-element/sub-target.bst
@@ -0,0 +1,5 @@
+kind: stack
+
+depends:
+- filename: target.bst
+  junction: junction-A.bst
diff --git a/tests/loader/junctions/missing-element/target.bst b/tests/loader/junctions/missing-element/target.bst
new file mode 100644
index 000000000..69ecef75c
--- /dev/null
+++ b/tests/loader/junctions/missing-element/target.bst
@@ -0,0 +1,5 @@
+kind: stack
+
+depends:
+- filename: missing.bst
+  junction: junction-A.bst