[Notes] [Git][BuildStream/buildstream][mandatory_suffix] 7 commits: Move def warn() from plugin.py to _context.py

[Phillip Smyth <gitlab mg gitlab com>]

Title: GitLab

Phillip Smyth pushed to branch mandatory_suffix at BuildStream / buildstream

Commits:

• 83394c98
  by Phillip Smyth at 2018-11-27T15:07:19Z

  Move def warn() from plugin.py to _context.py
  

• 9e2941b3
  by Phillip Smyth at 2018-11-27T15:07:28Z

  plugin.py: Add BAD_ELEMENT_SUFFIX warning
  

• e632fd94
  by Phillip Smyth at 2018-11-27T15:07:28Z

  _project.py: Check for fatal_warnings earlier
  

• 79b0ed8c
  by Phillip Smyth at 2018-11-27T15:07:29Z

  loadelement.py  & loader: Added file suffix check
  
  _loader/loader.py: Added Suffix check and warning for named elements
  _loader/loadelement.py: Added Suffix check and warning for dependencies
  

• c95919da
  by Phillip Smyth at 2018-11-27T15:07:29Z

  cli.py: no autocomplete on files without .bst
  

• ab18dc12
  by Phillip Smyth at 2018-11-27T15:07:29Z

  tests: build checkout and completions tests
  
  buildcheckout.py: Add mandatory .bst suffix tests
  completions.py: Add test for fail on invalid suffix
  Added required files for testing
  integration/source-determinism.py: renamed test elements to end with .bst
  

• f0e50898
  by Phillip Smyth at 2018-11-27T15:07:29Z

  NEWS: Added update about the Mandatory .bst change
  

14 changed files:

• NEWS
• buildstream/_context.py
• buildstream/_frontend/cli.py
• buildstream/_loader/loadelement.py
• buildstream/_loader/loader.py
• buildstream/_project.py
• buildstream/plugin.py
• tests/completions/completions.py
• + tests/frontend/.buildcheckout.py.swp
• tests/frontend/buildcheckout.py
• + tests/frontend/project/elements/target.foo
• + tests/frontend/project/elements/target2.bst
• tests/frontend/project/project.conf
• tests/integration/source-determinism.py

Changes:

NEWS

---

...	...	@@ -2,6 +2,10 @@
2	2	buildstream 1.3.1
3	3	=================
4	4
	5	+ o BREAKING CHANGE: Attempting to use an element that does not have the `.bst`
	6	+ extension, will result in an error message.
	7	+ All elements must now be suffixed with `.bst`
	8	+
5	9	o BREAKING CHANGE: The 'manual' element lost its default 'MAKEFLAGS' and 'V'
6	10	environment variables. There is already a 'make' element with the same
7	11	variables. Note that this is a breaking change, it will require users to

buildstream/_context.py

---

...	...	@@ -27,7 +27,7 @@ from . import _cachekey
27	27	from . import _signals
28	28	from . import _site
29	29	from . import _yaml
30		-from ._exceptions import LoadError, LoadErrorReason, BstError
	30	+from ._exceptions import LoadError, LoadErrorReason, BstError, PluginError
31	31	from ._message import Message, MessageType
32	32	from ._profile import Topics, profile_start, profile_end
33	33	from ._artifactcache import ArtifactCache
...	...	@@ -142,6 +142,37 @@ class Context():
142	142	self._log_handle = None
143	143	self._log_filename = None
144	144
	145	+ # Print a warning message, checks warning_token against project configuration
	146	+ #
	147	+ # Args:
	148	+ # project (Project): The related project
	149	+ # brief (str): The brief message
	150	+ # detail (str): An optional detailed message, can be multiline output
	151	+ # warning_token (str): An optional configurable warning assosciated with this warning,
	152	+ # this will cause PluginError to be raised if this warning is configured as fatal.
	153	+ # (*Since 1.4*)
	154	+ #
	155	+ # Raises:
	156	+ # (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
	157	+ #
	158	+ def warn(self, project, plugin, brief, *, detail=None, warning_token=None):
	159	+ # TODO _prefix_warning should probably move somewhere else
	160	+ from .plugin import _prefix_warning
	161	+ if warning_token:
	162	+ warning_token = _prefix_warning(plugin, warning_token)
	163	+ brief = "[{}]: {}".format(warning_token, brief)
	164	+
	165	+ if project._warning_is_fatal(warning_token):
	166	+ detail = detail if detail else ""
	167	+ raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
	168	+
	169	+ unique_id = None
	170	+ if plugin:
	171	+ unique_id = plugin._get_unique_id()
	172	+
	173	+ message = Message(unique_id, MessageType.WARN, brief, detail=detail)
	174	+ self.message(message)
	175	+
145	176	# load()
146	177	#
147	178	# Loads the configuration files

buildstream/_frontend/cli.py

---

...	...	@@ -109,7 +109,11 @@ def complete_target(args, incomplete):
109	109	if element_directory:
110	110	base_directory = os.path.join(base_directory, element_directory)
111	111
112		- return complete_path("File", incomplete, base_directory=base_directory)
	112	+ complete_list = []
	113	+ for p in complete_path("File", incomplete, base_directory=base_directory):
	114	+ if p.endswith(".bst ") or p.endswith("/") or p.endswith(".conf "):
	115	+ complete_list.append(p)
	116	+ return complete_list
113	117
114	118
115	119	def override_completions(cmd, cmd_param, args, incomplete):

buildstream/_loader/loadelement.py

---

...	...	@@ -145,11 +145,14 @@ def _extract_depends_from_node(node, *, key=None):
145	145
146	146	depends = _yaml.node_get(node, list, key, default_value=[])
147	147	output_deps = []
	148	+ invalid_elements = []
148	149
149	150	for index, dep in enumerate(depends):
150	151	dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
151	152
152	153	if isinstance(dep, str):
	154	+ if not dep.endswith(".bst"):
	155	+ invalid_elements.append(dep)
153	156	dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type)
154	157
155	158	elif isinstance(dep, Mapping):

buildstream/_loader/loader.py

---

...	...	@@ -97,6 +97,7 @@ class Loader():
97	97	# Returns: The toplevel LoadElement
98	98	def load(self, targets, rewritable=False, ticker=None, fetch_subprojects=False):
99	99
	100	+ invalid_elements = []
100	101	for filename in targets:
101	102	if os.path.isabs(filename):
102	103	# XXX Should this just be an assertion ?
...	...	@@ -106,6 +107,18 @@ class Loader():
106	107	"path to the base project directory: {}"
107	108	.format(filename, self._basedir))
108	109
	110	+ if not filename.endswith(".bst") and not filename.endswith("/") and not filename.endswith(".conf"):
	111	+ invalid_elements.append(filename)
	112	+
	113	+ if invalid_elements:
	114	+ # TODO CoreWarnings should probably move somewhere else
	115	+ from ..plugin import CoreWarnings
	116	+
	117	+ self._context.warn(self.project, None,
	118	+ "Target elements '{}' do not have expected file extension `.bst` "
	119	+ "Improperly named elements will not be discoverable by commands"
	120	+ .format(invalid_elements),
	121	+ warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
109	122	# First pass, recursively load files and populate our table of LoadElements
110	123	#
111	124	deps = []
...	...	@@ -124,6 +137,15 @@ class Loader():
124	137	deps.append(Dependency(name, junction=junction))
125	138	profile_end(Topics.LOAD_PROJECT, target)
126	139
	140	+ if invalid_elements:
	141	+ # TODO CoreWarnings should probably move somewhere else
	142	+ from ..plugin import CoreWarnings
	143	+
	144	+ self._context.warn(self.project, None,
	145	+ "Target elements '{}' do not have expected file extension `.bst` "
	146	+ "Improperly named elements will not be discoverable by commands"
	147	+ .format(invalid_elements),
	148	+ warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
127	149	#
128	150	# Now that we've resolve the dependencies, scan them for circular dependencies
129	151	#
...	...	@@ -214,7 +236,7 @@ class Loader():
214	236	# Returns:
215	237	# (LoadElement): A loaded LoadElement
216	238	#
217		- def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None):
	239	+ def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None, suffix_check=False):
218	240
219	241	# Silently ignore already loaded files
220	242	if filename in self._elements:
...	...	@@ -270,6 +292,12 @@ class Loader():
270	292
271	293	# Load all dependency files for the new LoadElement
272	294	for dep in element.deps:
	295	+ print("DEP: {}".format(dep.name))
	296	+ invalid_elements = []
	297	+ if not dep.name.endswith(".bst") and not dep.name.endswith("/") and not dep.name.endswith(".conf"):
	298	+ invalid_elements.append(dep.name)
	299	+ continue
	300	+
273	301	if dep.junction:
274	302	self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
275	303	loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
...	...	@@ -278,12 +306,19 @@ class Loader():
278	306	loader = self
279	307
280	308	dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects, yaml_cache)
281		-
282	309	if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
283	310	raise LoadError(LoadErrorReason.INVALID_DATA,
284	311	"{}: Cannot depend on junction"
285	312	.format(dep.provenance))
286		-
	313	+ if invalid_elements:
	314	+ # TODO CoreWarnings should probably move somewhere else
	315	+ from ..plugin import CoreWarnings
	316	+
	317	+ self._context.warn(self.project, None,
	318	+ "Target elements '{}' do not have expected file extension `.bst` "
	319	+ "Improperly named elements will not be discoverable by commands"
	320	+ .format(invalid_elements),
	321	+ warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
287	322	return element
288	323
289	324	# _check_circular_deps():
...	...	@@ -516,7 +551,7 @@ class Loader():
516	551	return loader
517	552
518	553	try:
519		- self._load_file(filename, rewritable, ticker, fetch_subprojects)
	554	+ self._load_file(filename, rewritable, ticker, fetch_subprojects, suffix_check=True)
520	555	except LoadError as e:
521	556	if e.reason != LoadErrorReason.MISSING_FILE:
522	557	# other load error
...	...	@@ -621,15 +656,24 @@ class Loader():
621	656	# - (str): name of the element
622	657	# - (Loader): loader for sub-project
623	658	#
624		- def _parse_name(self, name, rewritable, ticker, fetch_subprojects=False):
	659	+ def _parse_name(self, name, rewritable, ticker, fetch_subprojects=False, suffix_check=False):
625	660	# We allow to split only once since deep junctions names are forbidden.
626	661	# Users who want to refer to elements in sub-sub-projects are required
627	662	# to create junctions on the top level project.
628	663	junction_path = name.rsplit(':', 1)
629	664	if len(junction_path) == 1:
	665	+ if suffix_check:
	666	+ return None, junction_path[-1], self, "Pass"
630	667	return None, junction_path[-1], self
631	668	else:
632	669	self._load_file(junction_path[-2], rewritable, ticker, fetch_subprojects)
633	670	loader = self._get_loader(junction_path[-2], rewritable=rewritable, ticker=ticker,
634	671	fetch_subprojects=fetch_subprojects)
	672	+ if suffix_check:
	673	+ if not junction_path[-2].endswith(".bst") and not junction_path[-2].endswith("/") and not junction_path[-2].endswith(".conf"):
	674	+ result = "Fail"
	675	+ else:
	676	+ result = "Pass"
	677	+ return junction_path[-2], junction_path[-1], loader, result
	678	+
635	679	return junction_path[-2], junction_path[-1], loader

buildstream/_project.py

---

...	...	@@ -445,6 +445,9 @@ class Project():
445	445	self.config.options = OptionPool(self.element_path)
446	446	self.first_pass_config.options = OptionPool(self.element_path)
447	447
	448	+ # Early initialise fatal warnings
	449	+ self._fatal_warnings = _yaml.node_get(pre_config_node, list, 'fatal-warnings', default_value=[])
	450	+
448	451	self.loader = Loader(self._context, self,
449	452	parent=parent_loader,
450	453	tempdir=tempdir)

buildstream/plugin.py

---

...	...	@@ -492,28 +492,8 @@ class Plugin():
492	492	self.__message(MessageType.INFO, brief, detail=detail)
493	493
494	494	def warn(self, brief, *, detail=None, warning_token=None):
495		- """Print a warning message, checks warning_token against project configuration
496		-
497		- Args:
498		- brief (str): The brief message
499		- detail (str): An optional detailed message, can be multiline output
500		- warning_token (str): An optional configurable warning assosciated with this warning,
501		- this will cause PluginError to be raised if this warning is configured as fatal.
502		- (*Since 1.4*)
503		-
504		- Raises:
505		- (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
506		- """
507		- if warning_token:
508		- warning_token = _prefix_warning(self, warning_token)
509		- brief = "[{}]: {}".format(warning_token, brief)
510		- project = self._get_project()
511		-
512		- if project._warning_is_fatal(warning_token):
513		- detail = detail if detail else ""
514		- raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
515		-
516		- self.__message(MessageType.WARN, brief=brief, detail=detail)
	495	+ context = self._get_context()
	496	+ context.warn(self._get_project(), self, brief, detail=detail, warning_token=warning_token)
517	497
518	498	def log(self, brief, *, detail=None):
519	499	"""Log a message into the plugin's log file
...	...	@@ -784,6 +764,12 @@ class CoreWarnings():
784	764	which is found to be invalid based on the configured track
785	765	"""
786	766
	767	+ BAD_ELEMENT_SUFFIX = "bad-element-suffix"
	768	+ """
	769	+ This warning will be produced when an element whose name doesn not end in .bst
	770	+ is referenced either on the command line or by another element
	771	+ """
	772	+
787	773
788	774	__CORE_WARNINGS = [
789	775	value

tests/completions/completions.py

---

...	...	@@ -66,6 +66,13 @@ PROJECT_ELEMENTS = [
66	66	"target.bst"
67	67	]
68	68
	69	+INVALID_ELEMENTS = [
	70	+ "target.foo"
	71	+ "target.bst.bar"
	72	+]
	73	+
	74	+MIXED_ELEMENTS = PROJECT_ELEMENTS + INVALID_ELEMENTS
	75	+
69	76
70	77	def assert_completion(cli, cmd, word_idx, expected, cwd=None):
71	78	result = cli.run(cwd=cwd, env={
...	...	@@ -85,6 +92,24 @@ def assert_completion(cli, cmd, word_idx, expected, cwd=None):
85	92	assert words == expected
86	93
87	94
	95	+def assert_completion_failed(cli, cmd, word_idx, expected, cwd=None):
	96	+ result = cli.run(cwd=cwd, env={
	97	+ '_BST_COMPLETION': 'complete',
	98	+ 'COMP_WORDS': cmd,
	99	+ 'COMP_CWORD': str(word_idx)
	100	+ })
	101	+ words = []
	102	+ if result.output:
	103	+ words = result.output.splitlines()
	104	+
	105	+ # The order is meaningless, bash will
	106	+ # take the results and order it by its
	107	+ # own little heuristics
	108	+ words = sorted(words)
	109	+ expected = sorted(expected)
	110	+ assert words != expected
	111	+
	112	+
88	113	@pytest.mark.parametrize("cmd,word_idx,expected", [
89	114	('bst', 0, []),
90	115	('bst ', 1, MAIN_COMMANDS),
...	...	@@ -226,6 +251,19 @@ def test_argument_element(datafiles, cli, project, cmd, word_idx, expected, subd
226	251	assert_completion(cli, cmd, word_idx, expected, cwd=cwd)
227	252
228	253
	254	+@pytest.mark.datafiles(DATA_DIR)
	255	+@pytest.mark.parametrize("project,cmd,word_idx,expected,subdir", [
	256	+
	257	+ # When element has invalid suffix
	258	+ ('project', 'bst --directory ../ show ', 4, [e + ' ' for e in MIXED_ELEMENTS], 'files')
	259	+])
	260	+def test_argument_element_invalid(datafiles, cli, project, cmd, word_idx, expected, subdir):
	261	+ cwd = os.path.join(str(datafiles), project)
	262	+ if subdir:
	263	+ cwd = os.path.join(cwd, subdir)
	264	+ assert_completion_failed(cli, cmd, word_idx, expected, cwd=cwd)
	265	+
	266	+
229	267	@pytest.mark.parametrize("cmd,word_idx,expected", [
230	268	('bst he', 1, ['help ']),
231	269	('bst help ', 2, MAIN_COMMANDS),

tests/frontend/.buildcheckout.py.swp

---

No preview for this file type

tests/frontend/buildcheckout.py

---

...	...	@@ -60,9 +60,34 @@ def test_build_checkout(datafiles, cli, strict, hardlinks):
60	60	assert os.path.exists(filename)
61	61
62	62
	63	+@pytest.mark.datafiles(DATA_DIR)
	64	+@pytest.mark.parametrize("strict,hardlinks", [
	65	+ ("non-strict", "hardlinks"),
	66	+])
	67	+def test_build_invalid_suffix(datafiles, cli, strict, hardlinks):
	68	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	69	+ checkout = os.path.join(cli.directory, 'checkout')
	70	+
	71	+ result = cli.run(project=project, args=strict_args(['build', 'target.foo'], strict))
	72	+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
	73	+
	74	+
	75	+@pytest.mark.datafiles(DATA_DIR)
	76	+@pytest.mark.parametrize("strict,hardlinks", [
	77	+ ("non-strict", "hardlinks"),
	78	+])
	79	+def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
	80	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	81	+ checkout = os.path.join(cli.directory, 'checkout')
	82	+
	83	+ # target2.bst depends on an element called target.foo
	84	+ result = cli.run(project=project, args=strict_args(['build', 'target2.bst'], strict))
	85	+ result.assert_main_error(ErrorDomain.PLUGIN, "bad-element-suffix")
	86	+
	87	+
63	88	@pytest.mark.datafiles(DATA_DIR)
64	89	@pytest.mark.parametrize("deps", [("run"), ("none")])
65		-def test_build_checkout_deps(datafiles, cli, deps):
	90	+def test_build_checkoue_deps(datafiles, cli, deps):
66	91	project = os.path.join(datafiles.dirname, datafiles.basename)
67	92	checkout = os.path.join(cli.directory, 'checkout')
68	93

tests/frontend/project/elements/target.foo

---

	1	+kind: stack
	2	+description: |
	3	+
	4	+ Main stack target for the bst build test

tests/frontend/project/elements/target2.bst

---

	1	+kind: stack
	2	+description: |
	3	+
	4	+ Main stack target for the bst build test
	5	+
	6	+depends:
	7	+- target.foo

tests/frontend/project/project.conf

---

...	...	@@ -2,3 +2,6 @@
2	2	name: test
3	3
4	4	element-path: elements
	5	+
	6	+fatal-warnings:
	7	+- bad-element-suffix

tests/integration/source-determinism.py

---

...	...	@@ -33,7 +33,7 @@ def create_test_directory(*path, mode=0o644):
33	33	@pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
34	34	def test_deterministic_source_umask(cli, tmpdir, datafiles, kind, integration_cache):
35	35	project = str(datafiles)
36		- element_name = 'list'
	36	+ element_name = 'list.bst'
37	37	element_path = os.path.join(project, 'elements', element_name)
38	38	repodir = os.path.join(str(tmpdir), 'repo')
39	39	sourcedir = os.path.join(project, 'source')
...	...	@@ -108,7 +108,7 @@ def test_deterministic_source_local(cli, tmpdir, datafiles, integration_cache):
108	108	"""Only user rights should be considered for local source.
109	109	"""
110	110	project = str(datafiles)
111		- element_name = 'test'
	111	+ element_name = 'test.bst'
112	112	element_path = os.path.join(project, 'elements', element_name)
113	113	sourcedir = os.path.join(project, 'source')
114	114

—
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.