You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by tv...@apache.org on 2021/10/10 05:40:18 UTC
[buildstream] 02/03: Strict element naming format.
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/strict-element-names
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit eb97ef9de2d874580c5262b77865c7ce9d0dc510
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sun Oct 10 14:35:34 2021 +0900
Strict element naming format.
This patch removes the fatal(able) warnings for bad element names,
and instead makes BuildStream strict about element naming all around
such that we can now rely on elements being suffixed with ".bst" and
not contain invalid characters that are unsupported as filenames on some
platforms.
Summary of changes:
* types.py: Removed associated CoreWarning entries
* exceptions.py: Added corresponding LoadErrorReason entries
* _loader/loader.py: Raise an error instead of a configurable warning
upon encountering invalid element names.
We also now provide the provenance of the reference to the
invalid element name in error messages (if it is not toplevel).
---
src/buildstream/_loader/loader.py | 63 ++++++++++++++++-----------------------
src/buildstream/exceptions.py | 12 ++++++++
src/buildstream/types.py | 12 --------
3 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index df4df84..12b461e 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -125,8 +125,6 @@ class Loader:
LoadErrorReason.INVALID_DATA,
)
- self._warn_invalid_elements(targets)
-
# First pass, recursively load files and populate our table of LoadElements
#
target_elements = []
@@ -267,6 +265,9 @@ class Loader:
# (LoadElement): A partially-loaded LoadElement
#
def _load_file_no_deps(self, filename, provenance_node=None):
+
+ self._assert_element_name(filename, provenance_node)
+
# Load the data and process any conditional statements therein
fullpath = os.path.join(self._basedir, filename)
try:
@@ -530,9 +531,6 @@ class Loader:
dep.set_element(dep_element)
current_element[0].dependencies.append(dep) # pylint: disable=no-member
else:
- # We do not have any more dependencies to load for this
- # element on the queue, report any invalid dep names
- self._warn_invalid_elements(loader_queue[-1][2])
# And pop the element off the queue
loader_queue.pop()
@@ -1000,6 +998,8 @@ class Loader:
loader = self.get_loader(junction_path[-2], provenance_node, load_subprojects=load_subprojects)
return junction_path[-2], junction_path[-1], loader
+ # _warn():
+ #
# Print a warning message, checks warning_token against project configuration
#
# Args:
@@ -1017,47 +1017,34 @@ class Loader:
self.load_context.context.messenger.warn(brief)
- # Print warning messages if any of the specified elements have invalid names.
+ # _assert_element_name():
+ #
+ # Raises an error if any of the specified elements have invalid names.
#
# Valid filenames should end with ".bst" extension.
#
# Args:
- # elements (list): List of element names
+ # filename (str): The element name
+ # provenance_node (Node): The provenance node, or None
#
# Raises:
- # (:class:`.LoadError`): When warning_token is considered fatal by the project configuration
+ # (:class:`.LoadError`): If the element name is invalid
#
- def _warn_invalid_elements(self, elements):
+ def _assert_element_name(self, filename, provenance_node):
+ error_message = None
+ error_reason = None
- # invalid_elements
- #
- # A dict that maps warning types to the matching elements.
- invalid_elements = {
- CoreWarnings.BAD_ELEMENT_SUFFIX: [],
- CoreWarnings.BAD_CHARACTERS_IN_NAME: [],
- }
-
- for filename in elements:
- if not filename.endswith(".bst"):
- invalid_elements[CoreWarnings.BAD_ELEMENT_SUFFIX].append(filename)
- if not valid_chars_name(filename):
- invalid_elements[CoreWarnings.BAD_CHARACTERS_IN_NAME].append(filename)
-
- if invalid_elements[CoreWarnings.BAD_ELEMENT_SUFFIX]:
- self._warn(
- "Target elements '{}' do not have expected file extension `.bst` "
- "Improperly named elements will not be discoverable by commands".format(
- invalid_elements[CoreWarnings.BAD_ELEMENT_SUFFIX]
- ),
- warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX,
- )
- if invalid_elements[CoreWarnings.BAD_CHARACTERS_IN_NAME]:
- self._warn(
- "Target elements '{}' have invalid characerts in their name.".format(
- invalid_elements[CoreWarnings.BAD_CHARACTERS_IN_NAME]
- ),
- warning_token=CoreWarnings.BAD_CHARACTERS_IN_NAME,
- )
+ if not filename.endswith(".bst"):
+ error_message = "Element '{}' does not have expected file extension `.bst`".format(filename)
+ error_reason = LoadErrorReason.BAD_ELEMENT_SUFFIX
+ elif not valid_chars_name(filename):
+ error_message = "Element '{}' has invalid characters.".format(filename)
+ error_reason = LoadErrorReason.BAD_CHARACTERS_IN_NAME
+
+ if error_message:
+ if provenance_node is not None:
+ error_message = "{}: {}".format(provenance_node.get_provenance(), error_message)
+ raise LoadError(error_message, error_reason)
# _clean_caches()
#
diff --git a/src/buildstream/exceptions.py b/src/buildstream/exceptions.py
index 4b91189..8d828d0 100644
--- a/src/buildstream/exceptions.py
+++ b/src/buildstream/exceptions.py
@@ -146,3 +146,15 @@ class LoadErrorReason(Enum):
CIRCULAR_REFERENCE = 26
"""A circular element reference was detected"""
+
+ BAD_ELEMENT_SUFFIX = 27
+ """
+ This warning will be produced when an element whose name does not end in .bst
+ is referenced either on the command line or by another element
+ """
+
+ BAD_CHARACTERS_IN_NAME = 28
+ """
+ This warning will be produced when a filename for a target contains invalid
+ characters in its name.
+ """
diff --git a/src/buildstream/types.py b/src/buildstream/types.py
index ac3b180..8c87b4f 100644
--- a/src/buildstream/types.py
+++ b/src/buildstream/types.py
@@ -113,18 +113,6 @@ class CoreWarnings:
which is found to be invalid based on the configured track
"""
- BAD_ELEMENT_SUFFIX = "bad-element-suffix"
- """
- This warning will be produced when an element whose name does not end in .bst
- is referenced either on the command line or by another element
- """
-
- BAD_CHARACTERS_IN_NAME = "bad-characters-in-name"
- """
- This warning will be produced when a filename for a target contains invalid
- characters in its name.
- """
-
UNALIASED_URL = "unaliased-url"
"""
A URL used for fetching a sources was specified without specifying any