From e317bd1d5db00ce71186048302c15aee58875d95 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Sat, 9 Mar 2019 19:55:59 +0900 Subject: _yaml.py: Report more accurate provenance information When printing the provenance, show the fully qualified element name including the junction prefix in the case that the provenance comes from a subproject. This makes much more sensible error reporting when reporting errors which originate from a subproject. --- buildstream/_yaml.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/buildstream/_yaml.py b/buildstream/_yaml.py index fc24a223b..5dde9237e 100644 --- a/buildstream/_yaml.py +++ b/buildstream/_yaml.py @@ -69,7 +69,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): -- cgit v1.2.1 From e202f90366a8bb7fc10afc1c3428fdc91463bdfb Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Sat, 9 Mar 2019 20:12:33 +0900 Subject: _loader/loader.py: Include provenance in missing file errors This fixes issue #947 --- buildstream/_loader/loader.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py index 7ccb9a37c..84b0a17d8 100644 --- a/buildstream/_loader/loader.py +++ b/buildstream/_loader/loader.py @@ -187,11 +187,12 @@ class Loader(): # ticker (callable): A callback to report loaded filenames to the frontend # fetch_subprojects (bool): Whether to fetch subprojects while loading # yaml_cache (YamlCache): A yaml cache + # 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, yaml_cache=None): + def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None, provenance=None): # Silently ignore already loaded files if filename in self._elements: @@ -212,6 +213,8 @@ class Loader(): # 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) + if provenance: + message = "{}: {}".format(provenance, message) detail = None elements_dir = os.path.relpath(self._basedir, self.project.directory) element_relpath = os.path.relpath(filename, elements_dir) @@ -223,6 +226,8 @@ class Loader(): # If a .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' @@ -250,13 +255,14 @@ class Loader(): # Load all dependency files for the new LoadElement for dep in dependencies: if dep.junction: - self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache) + self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache, 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, yaml_cache) + dep_element = loader._load_file(dep.name, rewritable, ticker, + fetch_subprojects, yaml_cache, dep.provenance) if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction': raise LoadError(LoadErrorReason.INVALID_DATA, -- cgit v1.2.1 From bb44f093f6169aee5db41a7e4523affff12ed412 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Sat, 9 Mar 2019 20:38:33 +0900 Subject: _loader/loader.py: Specify junction name in missing file errors where appropriate When a file is missing in a subproject, it is not particularly meaningful to specify the filesystem path to the elements directory of the subproject, as this temporary staging directory belongs to BuildStream and not the user. Instead, when a file is missing in a subproject, specifying the junction name is more useful. This fixes an aspect of #947 --- buildstream/_loader/loader.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py index 84b0a17d8..1607c5b5e 100644 --- a/buildstream/_loader/loader.py +++ b/buildstream/_loader/loader.py @@ -209,19 +209,28 @@ class Loader(): project=self.project, yaml_cache=yaml_cache) 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) - if provenance: - message = "{}: {}".format(provenance, message) 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 .bst file exists in the element path, # let's suggest this as a plausible alternative. -- cgit v1.2.1 From f4a92e16ad2913de845ab5de866bbb0b963f349a Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Sat, 9 Mar 2019 18:51:49 +0900 Subject: tests/format/project.py: Added tests for missing files and missing junctions These also assert that the provenance of the references to missing files are reported, guarding for regressions of issue #947 --- tests/format/project.py | 20 ++++++++++++++++++++ tests/format/project/missing-element/manual.bst | 4 ++++ tests/format/project/missing-element/project.conf | 1 + tests/format/project/missing-junction/manual.bst | 5 +++++ tests/format/project/missing-junction/project.conf | 1 + 5 files changed, 31 insertions(+) create mode 100644 tests/format/project/missing-element/manual.bst create mode 100644 tests/format/project/missing-element/project.conf create mode 100644 tests/format/project/missing-junction/manual.bst create mode 100644 tests/format/project/missing-junction/project.conf diff --git a/tests/format/project.py b/tests/format/project.py index c746409bb..0f29cac1e 100644 --- a/tests/format/project.py +++ b/tests/format/project.py @@ -28,6 +28,26 @@ def test_missing_project_name(cli, datafiles): result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) +@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") 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 -- cgit v1.2.1 From 8376ec6596b26151e8c81f94bca6fd71e7140ab5 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Sat, 9 Mar 2019 20:10:18 +0900 Subject: tests/format/junctions.py: Added tests for missing files across junction boundaries These include assertions for the expected provenance in the errors, protecting against regressions of #947 --- tests/format/junctions.py | 30 ++++++++++++++++++++++ .../junctions/missing-element/junction-A.bst | 4 +++ .../junctionA/bad-junction-target.bst | 5 ++++ .../missing-element/junctionA/junction-B.bst | 4 +++ .../junctionA/junctionB/project.conf | 1 + .../missing-element/junctionA/project.conf | 1 + .../junctions/missing-element/junctionA/target.bst | 5 ++++ .../format/junctions/missing-element/project.conf | 1 + .../missing-element/sub-target-bad-junction.bst | 5 ++++ .../junctions/missing-element/sub-target.bst | 5 ++++ tests/format/junctions/missing-element/target.bst | 5 ++++ 11 files changed, 66 insertions(+) create mode 100644 tests/format/junctions/missing-element/junction-A.bst create mode 100644 tests/format/junctions/missing-element/junctionA/bad-junction-target.bst create mode 100644 tests/format/junctions/missing-element/junctionA/junction-B.bst create mode 100644 tests/format/junctions/missing-element/junctionA/junctionB/project.conf create mode 100644 tests/format/junctions/missing-element/junctionA/project.conf create mode 100644 tests/format/junctions/missing-element/junctionA/target.bst create mode 100644 tests/format/junctions/missing-element/project.conf create mode 100644 tests/format/junctions/missing-element/sub-target-bad-junction.bst create mode 100644 tests/format/junctions/missing-element/sub-target.bst create mode 100644 tests/format/junctions/missing-element/target.bst diff --git a/tests/format/junctions.py b/tests/format/junctions.py index 0c94bb51a..5c6ebd0bd 100644 --- a/tests/format/junctions.py +++ b/tests/format/junctions.py @@ -58,6 +58,36 @@ def test_build_of_same_junction_used_twice(cli, datafiles): result.assert_success() +@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') diff --git a/tests/format/junctions/missing-element/junction-A.bst b/tests/format/junctions/missing-element/junction-A.bst new file mode 100644 index 000000000..74079f990 --- /dev/null +++ b/tests/format/junctions/missing-element/junction-A.bst @@ -0,0 +1,4 @@ +kind: junction +sources: +- kind: local + path: junctionA diff --git a/tests/format/junctions/missing-element/junctionA/bad-junction-target.bst b/tests/format/junctions/missing-element/junctionA/bad-junction-target.bst new file mode 100644 index 000000000..c07c198cd --- /dev/null +++ b/tests/format/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/format/junctions/missing-element/junctionA/junction-B.bst b/tests/format/junctions/missing-element/junctionA/junction-B.bst new file mode 100644 index 000000000..bc66d7851 --- /dev/null +++ b/tests/format/junctions/missing-element/junctionA/junction-B.bst @@ -0,0 +1,4 @@ +kind: junction +sources: +- kind: local + path: junctionB diff --git a/tests/format/junctions/missing-element/junctionA/junctionB/project.conf b/tests/format/junctions/missing-element/junctionA/junctionB/project.conf new file mode 100644 index 000000000..41b8d6c72 --- /dev/null +++ b/tests/format/junctions/missing-element/junctionA/junctionB/project.conf @@ -0,0 +1 @@ +name: projectB diff --git a/tests/format/junctions/missing-element/junctionA/project.conf b/tests/format/junctions/missing-element/junctionA/project.conf new file mode 100644 index 000000000..5f6ab28a2 --- /dev/null +++ b/tests/format/junctions/missing-element/junctionA/project.conf @@ -0,0 +1 @@ +name: projectA diff --git a/tests/format/junctions/missing-element/junctionA/target.bst b/tests/format/junctions/missing-element/junctionA/target.bst new file mode 100644 index 000000000..9c3d0bf0e --- /dev/null +++ b/tests/format/junctions/missing-element/junctionA/target.bst @@ -0,0 +1,5 @@ +kind: stack + +depends: +- filename: missing.bst + junction: junction-B.bst diff --git a/tests/format/junctions/missing-element/project.conf b/tests/format/junctions/missing-element/project.conf new file mode 100644 index 000000000..b32753625 --- /dev/null +++ b/tests/format/junctions/missing-element/project.conf @@ -0,0 +1 @@ +name: test diff --git a/tests/format/junctions/missing-element/sub-target-bad-junction.bst b/tests/format/junctions/missing-element/sub-target-bad-junction.bst new file mode 100644 index 000000000..f48f6cec9 --- /dev/null +++ b/tests/format/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/format/junctions/missing-element/sub-target.bst b/tests/format/junctions/missing-element/sub-target.bst new file mode 100644 index 000000000..79e8bc684 --- /dev/null +++ b/tests/format/junctions/missing-element/sub-target.bst @@ -0,0 +1,5 @@ +kind: stack + +depends: +- filename: target.bst + junction: junction-A.bst diff --git a/tests/format/junctions/missing-element/target.bst b/tests/format/junctions/missing-element/target.bst new file mode 100644 index 000000000..69ecef75c --- /dev/null +++ b/tests/format/junctions/missing-element/target.bst @@ -0,0 +1,5 @@ +kind: stack + +depends: +- filename: missing.bst + junction: junction-A.bst -- cgit v1.2.1