[Notes] [Git][BuildStream/buildstream][master] 6 commits: _includes: better provenance on recursive include

[Jürg Billeter <gitlab mg gitlab com>]

Title: GitLab

Jürg Billeter pushed to branch master at BuildStream / buildstream

Commits:

• 1ed63e54
  by Angelos Evripiotis at 2019-02-11T09:24:48Z

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

• 02e48209
  by Angelos Evripiotis at 2019-02-11T09:24:48Z

  _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'.
  

• 4336e3bf
  by Angelos Evripiotis at 2019-02-11T09:24:48Z

  _includes: better error on including directory
  
  Previously, include a directory 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'.
  

• 3f6c5000
  by Angelos Evripiotis at 2019-02-11T09:24:48Z

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

• adde0c94
  by Angelos Evripiotis at 2019-02-11T09:24:48Z

  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.
  

• a66f8379
  by Jürg Billeter at 2019-02-11T13:52:54Z

  Merge branch 'aevri/include-error' into 'master'
  
  More user-friendly reporting on include errors
  
  See merge request BuildStream/buildstream!891

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
...	...	@@ -27,6 +28,46 @@ def test_include_project_file(cli, datafiles):
27	28	assert loaded['included'] == 'True'
28	29
29	30
	31	+def test_include_missing_file(cli, tmpdir):
	32	+ tmpdir.join('project.conf').write('{"name": "test"}')
	33	+ element = tmpdir.join('include_missing_file.bst')
	34	+
	35	+ # Normally we would use dicts and _yaml.dump to write such things, but here
	36	+ # we want to be sure of a stable line and column number.
	37	+ element.write(textwrap.dedent("""
	38	+ kind: manual
	39	+
	40	+ "(@)":
	41	+ - nosuch.yaml
	42	+ """).strip())
	43	+
	44	+ result = cli.run(project=str(tmpdir), args=['show', str(element.basename)])
	45	+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_FILE)
	46	+ # Make sure the root cause provenance is in the output.
	47	+ assert 'line 4 column 2' in result.stderr
	48	+
	49	+
	50	+def test_include_dir(cli, tmpdir):
	51	+ tmpdir.join('project.conf').write('{"name": "test"}')
	52	+ tmpdir.mkdir('subdir')
	53	+ element = tmpdir.join('include_dir.bst')
	54	+
	55	+ # Normally we would use dicts and _yaml.dump to write such things, but here
	56	+ # we want to be sure of a stable line and column number.
	57	+ element.write(textwrap.dedent("""
	58	+ kind: manual
	59	+
	60	+ "(@)":
	61	+ - subdir/
	62	+ """).strip())
	63	+
	64	+ result = cli.run(project=str(tmpdir), args=['show', str(element.basename)])
	65	+ result.assert_main_error(
	66	+ ErrorDomain.LOAD, LoadErrorReason.LOADING_DIRECTORY)
	67	+ # Make sure the root cause provenance is in the output.
	68	+ assert 'line 4 column 2' in result.stderr
	69	+
	70	+
30	71	@pytest.mark.datafiles(DATA_DIR)
31	72	def test_include_junction_file(cli, tmpdir, datafiles):
32	73	project = os.path.join(str(datafiles), 'junction')
...	...	@@ -47,7 +88,7 @@ def test_include_junction_file(cli, tmpdir, datafiles):
47	88
48	89
49	90	@pytest.mark.datafiles(DATA_DIR)
50		-def test_include_junction_options(cli, tmpdir, datafiles):
	91	+def test_include_junction_options(cli, datafiles):
51	92	project = os.path.join(str(datafiles), 'options')
52	93
53	94	result = cli.run(project=project, args=[
...	...	@@ -128,7 +169,7 @@ def test_junction_element_not_partial_project_file(cli, tmpdir, datafiles):
128	169
129	170
130	171	@pytest.mark.datafiles(DATA_DIR)
131		-def test_include_element_overrides(cli, tmpdir, datafiles):
	172	+def test_include_element_overrides(cli, datafiles):
132	173	project = os.path.join(str(datafiles), 'overrides')
133	174
134	175	result = cli.run(project=project, args=[
...	...	@@ -143,7 +184,7 @@ def test_include_element_overrides(cli, tmpdir, datafiles):
143	184
144	185
145	186	@pytest.mark.datafiles(DATA_DIR)
146		-def test_include_element_overrides_composition(cli, tmpdir, datafiles):
	187	+def test_include_element_overrides_composition(cli, datafiles):
147	188	project = os.path.join(str(datafiles), 'overrides')
148	189
149	190	result = cli.run(project=project, args=[
...	...	@@ -158,7 +199,7 @@ def test_include_element_overrides_composition(cli, tmpdir, datafiles):
158	199
159	200
160	201	@pytest.mark.datafiles(DATA_DIR)
161		-def test_include_element_overrides_sub_include(cli, tmpdir, datafiles):
	202	+def test_include_element_overrides_sub_include(cli, datafiles):
162	203	project = os.path.join(str(datafiles), 'sub-include')
163	204
164	205	result = cli.run(project=project, args=[
...	...	@@ -192,7 +233,7 @@ def test_junction_do_not_use_included_overrides(cli, tmpdir, datafiles):
192	233
193	234
194	235	@pytest.mark.datafiles(DATA_DIR)
195		-def test_conditional_in_fragment(cli, tmpdir, datafiles):
	236	+def test_conditional_in_fragment(cli, datafiles):
196	237	project = os.path.join(str(datafiles), 'conditional')
197	238
198	239	result = cli.run(project=project, args=[
...	...	@@ -222,7 +263,7 @@ def test_inner(cli, datafiles):
222	263
223	264
224	265	@pytest.mark.datafiles(DATA_DIR)
225		-def test_recusive_include(cli, tmpdir, datafiles):
	266	+def test_recursive_include(cli, datafiles):
226	267	project = os.path.join(str(datafiles), 'recursive')
227	268
228	269	result = cli.run(project=project, args=[
...	...	@@ -231,6 +272,7 @@ def test_recusive_include(cli, tmpdir, datafiles):
231	272	'--format', '%{vars}',
232	273	'element.bst'])
233	274	result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.RECURSIVE_INCLUDE)
	275	+ assert 'line 2 column 2' in result.stderr
234	276
235	277
236	278	@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.