[Notes] [Git][BuildStream/buildstream][aevri/include-error] 5 commits: _includes: better provenance on recursive include

[Angelos Evripiotis <gitlab mg gitlab com>]

Title: GitLab

Angelos Evripiotis pushed to branch aevri/include-error at BuildStream / buildstream

Commits:

• b85f8b6c
  by Angelos Evripiotis at 2019-02-06T09:39:47Z

  _includes: better provenance on recursive include
  
  Use the provenance of the include block, instead of the whole node.
  

• 0da7736e
  by Angelos Evripiotis at 2019-02-06T10:26:22Z

  _includes: better error on missing include
  
  Previously, a missing include would result in an error like this:
  
      Could not find file at not-a-file.include
  
  Note that the file containing the include was not mentioned.
  
  Now we get an error like this instead:
  
      element.bst [line 7 column 5]: Include block references a file that
      could not be found: 'not-a-file.include'.
  

• 0f348440
  by Angelos Evripiotis at 2019-02-06T10:27:13Z

  _includes: better error on including directory
  
  Previously, a missing include would result in an error like this:
  
      mydir is a directory. bst command expects a .bst file.
  
  Note that the file containing the include was not mentioned.
  
  Now we get an error like this instead:
  
      element.bst [line 12 column 0]: Include block references a
      directory instead of a file: 'mydir'.
  

• da759ab7
  by Angelos Evripiotis at 2019-02-06T10:27:21Z

  _includes: re-use file_path variable
  
  Avoid an unnecessary call to os.path.join().
  

• a4a63c8e
  by Angelos Evripiotis at 2019-02-06T10:27:21Z

  tests/format/include: remove unused tmpdir's
  
  Don't create and remove temp dirs unnecessarily when they are not used,
  looks like these were just copy-pastes without intended side-effects.
  

2 changed files:

• buildstream/_includes.py
• tests/format/include.py

Changes:

buildstream/_includes.py

---

...	...	@@ -40,19 +40,34 @@ class Includes:
40	40	includes = [_yaml.node_get(node, str, '(@)')]
41	41	else:
42	42	includes = _yaml.node_get(node, list, '(@)', default_value=None)
	43	+
	44	+ include_provenance = None
43	45	if '(@)' in node:
	46	+ include_provenance = _yaml.node_get_provenance(node, key='(@)')
44	47	del node['(@)']
45	48
46	49	if includes:
47	50	for include in reversed(includes):
48	51	if only_local and ':' in include:
49	52	continue
50		- include_node, file_path, sub_loader = self._include_file(include,
51		- current_loader)
	53	+ try:
	54	+ include_node, file_path, sub_loader = self._include_file(include,
	55	+ current_loader)
	56	+ except LoadError as e:
	57	+ if e.reason == LoadErrorReason.MISSING_FILE:
	58	+ message = "{}: Include block references a file that could not be found: '{}'.".format(
	59	+ include_provenance, include)
	60	+ raise LoadError(LoadErrorReason.MISSING_FILE, message) from e
	61	+ elif e.reason == LoadErrorReason.LOADING_DIRECTORY:
	62	+ message = "{}: Include block references a directory instead of a file: '{}'.".format(
	63	+ include_provenance, include)
	64	+ raise LoadError(LoadErrorReason.LOADING_DIRECTORY, message) from e
	65	+ else:
	66	+ raise
	67	+
52	68	if file_path in included:
53		- provenance = _yaml.node_get_provenance(node)
54	69	raise LoadError(LoadErrorReason.RECURSIVE_INCLUDE,
55		- "{}: trying to recursively include {}". format(provenance,
	70	+ "{}: trying to recursively include {}". format(include_provenance,
56	71	file_path))
57	72	# Because the included node will be modified, we need
58	73	# to copy it so that we do not modify the toplevel
...	...	@@ -101,7 +116,7 @@ class Includes:
101	116	file_path = os.path.join(directory, include)
102	117	key = (current_loader, file_path)
103	118	if key not in self._loaded:
104		- self._loaded[key] = _yaml.load(os.path.join(directory, include),
	119	+ self._loaded[key] = _yaml.load(file_path,
105	120	shortname=shortname,
106	121	project=project,
107	122	copy_tree=self._copy_tree)

tests/format/include.py

---

1	1	import os
	2	+import textwrap
2	3	import pytest
3	4	from buildstream import _yaml
4	5	from buildstream._exceptions import ErrorDomain, LoadErrorReason
...	...	@@ -25,6 +26,46 @@ def test_include_project_file(cli, datafiles):
25	26	assert loaded['included'] == 'True'
26	27
27	28
	29	+def test_include_missing_file(cli, tmpdir):
	30	+ tmpdir.join('project.conf').write('{"name": "test"}')
	31	+ element = tmpdir.join('include_missing_file.bst')
	32	+
	33	+ # Normally we would use dicts and _yaml.dump to write such things, but here
	34	+ # we want to be sure of a stable line and column number.
	35	+ element.write(textwrap.dedent("""
	36	+ kind: manual
	37	+
	38	+ "(@)":
	39	+ - nosuch.yaml
	40	+ """).strip())
	41	+
	42	+ result = cli.run(project=str(tmpdir), args=['show', str(element.basename)])
	43	+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
	44	+ # Make sure the root cause provenance is in the output.
	45	+ assert 'line 4 column 2' in result.stderr
	46	+
	47	+
	48	+def test_include_dir(cli, tmpdir):
	49	+ tmpdir.join('project.conf').write('{"name": "test"}')
	50	+ tmpdir.mkdir('subdir')
	51	+ element = tmpdir.join('include_dir.bst')
	52	+
	53	+ # Normally we would use dicts and _yaml.dump to write such things, but here
	54	+ # we want to be sure of a stable line and column number.
	55	+ element.write(textwrap.dedent("""
	56	+ kind: manual
	57	+
	58	+ "(@)":
	59	+ - subdir/
	60	+ """).strip())
	61	+
	62	+ result = cli.run(project=str(tmpdir), args=['show', str(element.basename)])
	63	+ result.assert_main_error(
	64	+ ErrorDomain.LOAD, LoadErrorReason.LOADING_DIRECTORY)
	65	+ # Make sure the root cause provenance is in the output.
	66	+ assert 'line 4 column 2' in result.stderr
	67	+
	68	+
28	69	@pytest.mark.datafiles(DATA_DIR)
29	70	def test_include_junction_file(cli, tmpdir, datafiles):
30	71	project = os.path.join(str(datafiles), 'junction')
...	...	@@ -45,7 +86,7 @@ def test_include_junction_file(cli, tmpdir, datafiles):
45	86
46	87
47	88	@pytest.mark.datafiles(DATA_DIR)
48		-def test_include_junction_options(cli, tmpdir, datafiles):
	89	+def test_include_junction_options(cli, datafiles):
49	90	project = os.path.join(str(datafiles), 'options')
50	91
51	92	result = cli.run(project=project, args=[
...	...	@@ -126,7 +167,7 @@ def test_junction_element_not_partial_project_file(cli, tmpdir, datafiles):
126	167
127	168
128	169	@pytest.mark.datafiles(DATA_DIR)
129		-def test_include_element_overrides(cli, tmpdir, datafiles):
	170	+def test_include_element_overrides(cli, datafiles):
130	171	project = os.path.join(str(datafiles), 'overrides')
131	172
132	173	result = cli.run(project=project, args=[
...	...	@@ -141,7 +182,7 @@ def test_include_element_overrides(cli, tmpdir, datafiles):
141	182
142	183
143	184	@pytest.mark.datafiles(DATA_DIR)
144		-def test_include_element_overrides_composition(cli, tmpdir, datafiles):
	185	+def test_include_element_overrides_composition(cli, datafiles):
145	186	project = os.path.join(str(datafiles), 'overrides')
146	187
147	188	result = cli.run(project=project, args=[
...	...	@@ -156,7 +197,7 @@ def test_include_element_overrides_composition(cli, tmpdir, datafiles):
156	197
157	198
158	199	@pytest.mark.datafiles(DATA_DIR)
159		-def test_include_element_overrides_sub_include(cli, tmpdir, datafiles):
	200	+def test_include_element_overrides_sub_include(cli, datafiles):
160	201	project = os.path.join(str(datafiles), 'sub-include')
161	202
162	203	result = cli.run(project=project, args=[
...	...	@@ -190,7 +231,7 @@ def test_junction_do_not_use_included_overrides(cli, tmpdir, datafiles):
190	231
191	232
192	233	@pytest.mark.datafiles(DATA_DIR)
193		-def test_conditional_in_fragment(cli, tmpdir, datafiles):
	234	+def test_conditional_in_fragment(cli, datafiles):
194	235	project = os.path.join(str(datafiles), 'conditional')
195	236
196	237	result = cli.run(project=project, args=[
...	...	@@ -220,7 +261,7 @@ def test_inner(cli, datafiles):
220	261
221	262
222	263	@pytest.mark.datafiles(DATA_DIR)
223		-def test_recusive_include(cli, tmpdir, datafiles):
	264	+def test_recursive_include(cli, datafiles):
224	265	project = os.path.join(str(datafiles), 'recursive')
225	266
226	267	result = cli.run(project=project, args=[
...	...	@@ -229,6 +270,7 @@ def test_recusive_include(cli, tmpdir, datafiles):
229	270	'--format', '%{vars}',
230	271	'element.bst'])
231	272	result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.RECURSIVE_INCLUDE)
	273	+ assert 'line 2 column 2' in result.stderr
232	274
233	275
234	276	@pytest.mark.datafiles(DATA_DIR)

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.