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:16 UTC
[buildstream] branch tristan/strict-element-names created (now
c9bcda6)
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a change to branch tristan/strict-element-names
in repository https://gitbox.apache.org/repos/asf/buildstream.git.
at c9bcda6 tests/format/elementnames.py: Added format test for element names
This branch includes the following new commits:
new d4dd918 tests/frontend/buildcheckout.py: Remove element name tests from here
new eb97ef9 Strict element naming format.
new c9bcda6 tests/format/elementnames.py: Added format test for element names
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[buildstream] 01/03: tests/frontend/buildcheckout.py: Remove
element name tests from here
Posted by tv...@apache.org.
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 d4dd918cbdc0f678348e7464f3a0ac7e8a0506ae
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sun Oct 10 14:33:52 2021 +0900
tests/frontend/buildcheckout.py: Remove element name tests from here
This is the wrong place to be testing for invalid element names, we should
be testing that in the format tests, not in the tests which have to do
with build & checkout functionality.
---
tests/frontend/buildcheckout.py | 54 ----------------------
.../project/elements/invalid-chars-in-dep.bst | 8 ----
tests/frontend/project/elements/target2.bst | 7 ---
tests/frontend/project/project.conf | 4 --
4 files changed, 73 deletions(-)
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index 6021ff0..9e615c6 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -170,60 +170,6 @@ def test_non_strict_checkout_uncached(datafiles, cli, tmpdir):
@pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("strict,hardlinks", [("non-strict", "hardlinks"),])
-def test_build_invalid_suffix(datafiles, cli, strict, hardlinks):
- project = str(datafiles)
-
- result = cli.run(project=project, args=strict_args(["build", "target.foo"], strict))
- result.assert_main_error(ErrorDomain.LOAD, "bad-element-suffix")
-
-
-@pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("strict,hardlinks", [("non-strict", "hardlinks"),])
-def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
- project = str(datafiles)
-
- # target2.bst depends on an element called target.foo
- result = cli.run(project=project, args=strict_args(["build", "target2.bst"], strict))
- result.assert_main_error(ErrorDomain.LOAD, "bad-element-suffix")
-
-
-@pytest.mark.skipif(IS_WINDOWS, reason="Not available on Windows")
-@pytest.mark.datafiles(DATA_DIR)
-def test_build_invalid_filename_chars(datafiles, cli):
- project = str(datafiles)
- element_name = "invalid-chars|<>-in-name.bst"
-
- # The name of this file contains characters that are not allowed by
- # BuildStream, using it should raise a warning.
- element = {
- "kind": "stack",
- }
- _yaml.roundtrip_dump(element, os.path.join(project, "elements", element_name))
-
- result = cli.run(project=project, args=strict_args(["build", element_name], "non-strict"))
- result.assert_main_error(ErrorDomain.LOAD, "bad-characters-in-name")
-
-
-@pytest.mark.skipif(IS_WINDOWS, reason="Not available on Windows")
-@pytest.mark.datafiles(DATA_DIR)
-def test_build_invalid_filename_chars_dep(datafiles, cli):
- project = str(datafiles)
- element_name = "invalid-chars|<>-in-name.bst"
-
- # The name of this file contains characters that are not allowed by
- # BuildStream, and is listed as a dependency of 'invalid-chars-in-dep.bst'.
- # This should also raise a warning.
- element = {
- "kind": "stack",
- }
- _yaml.roundtrip_dump(element, os.path.join(project, "elements", element_name))
-
- result = cli.run(project=project, args=strict_args(["build", "invalid-chars-in-dep.bst"], "non-strict"))
- result.assert_main_error(ErrorDomain.LOAD, "bad-characters-in-name")
-
-
-@pytest.mark.datafiles(DATA_DIR)
@pytest.mark.parametrize("deps", [("run"), ("none"), ("build"), ("all")])
def test_build_checkout_deps(datafiles, cli, deps):
project = str(datafiles)
diff --git a/tests/frontend/project/elements/invalid-chars-in-dep.bst b/tests/frontend/project/elements/invalid-chars-in-dep.bst
deleted file mode 100644
index 6a5ec30..0000000
--- a/tests/frontend/project/elements/invalid-chars-in-dep.bst
+++ /dev/null
@@ -1,8 +0,0 @@
-kind: stack
-description: |
-
- This element itself has a valid name, but depends on elements that have
- invalid names. This should also result in a warning.
-
-depends:
-- invalid-chars|<>-in-name.bst
diff --git a/tests/frontend/project/elements/target2.bst b/tests/frontend/project/elements/target2.bst
deleted file mode 100644
index 259819f..0000000
--- a/tests/frontend/project/elements/target2.bst
+++ /dev/null
@@ -1,7 +0,0 @@
-kind: stack
-description: |
-
- Main stack target for the bst build test
-
-depends:
-- target.foo
diff --git a/tests/frontend/project/project.conf b/tests/frontend/project/project.conf
index c82f85b..5ba3168 100644
--- a/tests/frontend/project/project.conf
+++ b/tests/frontend/project/project.conf
@@ -2,7 +2,3 @@
name: test
min-version: 2.0
element-path: elements
-
-fatal-warnings:
-- bad-element-suffix
-- bad-characters-in-name
[buildstream] 02/03: Strict element naming format.
Posted by tv...@apache.org.
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
[buildstream] 03/03: tests/format/elementnames.py: Added format
test for element names
Posted by tv...@apache.org.
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 c9bcda68f21d74086cd4886c7c2c9d89e1ca2255
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sun Oct 10 14:39:02 2021 +0900
tests/format/elementnames.py: Added format test for element names
Ensure that we emit the correct errors for bad element names, either on
the command line or as specified as dependencies in the yaml.
---
tests/format/elementnames.py | 29 ++++++++++++++++++++++++++++
tests/format/elementnames/bad-chars-dep.bst | 3 +++
tests/format/elementnames/bad-suffix-dep.bst | 3 +++
tests/format/elementnames/project.conf | 4 ++++
4 files changed, 39 insertions(+)
diff --git a/tests/format/elementnames.py b/tests/format/elementnames.py
new file mode 100644
index 0000000..f059d54
--- /dev/null
+++ b/tests/format/elementnames.py
@@ -0,0 +1,29 @@
+# Pylint doesn't play well with fixtures and dependency injection from pytest
+# pylint: disable=redefined-outer-name
+
+import os
+import pytest
+
+from buildstream.exceptions import ErrorDomain, LoadErrorReason
+from buildstream.testing import cli # pylint: disable=unused-import
+
+DATA_DIR = os.path.dirname(os.path.realpath(__file__))
+
+
+@pytest.mark.parametrize(
+ "target,reason,provenance",
+ [
+ ("farm.pony", LoadErrorReason.BAD_ELEMENT_SUFFIX, None),
+ ('The "quoted" pony.bst', LoadErrorReason.BAD_CHARACTERS_IN_NAME, None),
+ ("bad-suffix-dep.bst", LoadErrorReason.BAD_ELEMENT_SUFFIX, "bad-suffix-dep.bst [line 3 column 2]"),
+ ("bad-chars-dep.bst", LoadErrorReason.BAD_CHARACTERS_IN_NAME, "bad-chars-dep.bst [line 3 column 2]"),
+ ],
+ ids=["toplevel-bad-suffix", "toplevel-bad-chars", "dependency-bad-suffix", "dependency-bad-chars"],
+)
+@pytest.mark.datafiles(DATA_DIR)
+def test_invalid_element_names(cli, datafiles, target, reason, provenance):
+ project = os.path.join(str(datafiles), "elementnames")
+ result = cli.run(project=project, silent=True, args=["show", target])
+ result.assert_main_error(ErrorDomain.LOAD, reason)
+ if provenance:
+ assert provenance in result.stderr
diff --git a/tests/format/elementnames/bad-chars-dep.bst b/tests/format/elementnames/bad-chars-dep.bst
new file mode 100644
index 0000000..d637799
--- /dev/null
+++ b/tests/format/elementnames/bad-chars-dep.bst
@@ -0,0 +1,3 @@
+kind: stack
+depends:
+- <invalid>.bst
diff --git a/tests/format/elementnames/bad-suffix-dep.bst b/tests/format/elementnames/bad-suffix-dep.bst
new file mode 100644
index 0000000..c96604d
--- /dev/null
+++ b/tests/format/elementnames/bad-suffix-dep.bst
@@ -0,0 +1,3 @@
+kind: stack
+depends:
+- rainbow.pony
diff --git a/tests/format/elementnames/project.conf b/tests/format/elementnames/project.conf
new file mode 100644
index 0000000..b0110aa
--- /dev/null
+++ b/tests/format/elementnames/project.conf
@@ -0,0 +1,4 @@
+# Basic project configuration that doesnt override anything
+#
+name: test
+min-version: 2.0