diff options
author | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-12-31 20:29:51 +0000 |
---|---|---|
committer | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-12-31 20:29:51 +0000 |
commit | b848172c518d3d52a94feded9e12a0b69a27f1d7 (patch) | |
tree | 1e818daf5ce2e113ff0f23da540c94fe7ffe4301 | |
parent | 560a634256839de4c9e8bcc41215a17c525d0e61 (diff) | |
parent | ce9e9c2dbec71841d5ac5311f6d18ffb78bce9d3 (diff) | |
download | buildstream-b848172c518d3d52a94feded9e12a0b69a27f1d7.tar.gz |
Merge branch 'chandan/element-filename-requirements' into 'master'
Add warnings about invalid characters in filename
See merge request BuildStream/buildstream!1028
-rw-r--r-- | buildstream/_loader/loader.py | 88 | ||||
-rw-r--r-- | buildstream/types.py | 6 | ||||
-rw-r--r-- | doc/source/format_declaring.rst | 24 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 14 | ||||
-rw-r--r-- | tests/frontend/project/elements/invalid-chars-in-dep.bst | 8 | ||||
-rw-r--r-- | tests/frontend/project/elements/invalid-chars|<>-in-name.bst | 4 | ||||
-rw-r--r-- | tests/frontend/project/project.conf | 1 |
7 files changed, 127 insertions, 18 deletions
diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py index 0de0e2b9c..16e880388 100644 --- a/buildstream/_loader/loader.py +++ b/buildstream/_loader/loader.py @@ -99,7 +99,6 @@ class Loader(): # Returns: The toplevel LoadElement def load(self, targets, rewritable=False, ticker=None, fetch_subprojects=False): - invalid_elements = [] for filename in targets: if os.path.isabs(filename): # XXX Should this just be an assertion ? @@ -109,14 +108,8 @@ class Loader(): "path to the base project directory: {}" .format(filename, self._basedir)) - if not filename.endswith(".bst"): - invalid_elements.append(filename) + self._warn_invalid_elements(targets) - if invalid_elements: - self._warn("Target elements '{}' do not have expected file extension `.bst` " - "Improperly named elements will not be discoverable by commands" - .format(invalid_elements), - warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX) # First pass, recursively load files and populate our table of LoadElements # deps = [] @@ -280,12 +273,7 @@ class Loader(): self._elements[filename] = element # Load all dependency files for the new LoadElement - invalid_elements = [] for dep in element.deps: - if not dep.name.endswith(".bst"): - invalid_elements.append(dep.name) - continue - if dep.junction: self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache) loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker, @@ -300,11 +288,9 @@ class Loader(): "{}: Cannot depend on junction" .format(dep.provenance)) - if invalid_elements: - self._warn("The following dependencies do not have expected file extension `.bst`: {} " - "Improperly named elements will not be discoverable by commands" - .format(invalid_elements), - warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX) + deps_names = [dep.name for dep in element.deps] + self._warn_invalid_elements(deps_names) + return element # _check_circular_deps(): @@ -679,3 +665,69 @@ class Loader(): message = Message(None, MessageType.WARN, brief) self._context.message(message) + + # Print warning messages if any of the specified elements have invalid names. + # + # Valid filenames should end with ".bst" extension. + # + # Args: + # elements (list): List of element names + # + # Raises: + # (:class:`.LoadError`): When warning_token is considered fatal by the project configuration + # + def _warn_invalid_elements(self, elements): + + # 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 self._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) + + # Check if given filename containers valid characters. + # + # Args: + # name (str): Name of the file + # + # Returns: + # (bool): True if all characters are valid, False otherwise. + # + def _valid_chars_name(self, name): + for char in name: + char_val = ord(char) + + # 0-31 are control chars, 127 is DEL, and >127 means non-ASCII + if char_val <= 31 or char_val >= 127: + return False + + # Disallow characters that are invalid on Windows. The list can be + # found at https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file + # + # Note that although : (colon) is not allowed, we do not raise + # warnings because of that, since we use it as a separator for + # junctioned elements. + # + # We also do not raise warnings on slashes since they are used as + # path separators. + if char in r'<>"|?*': + return False + + return True diff --git a/buildstream/types.py b/buildstream/types.py index 5e6265b4e..23d78b08c 100644 --- a/buildstream/types.py +++ b/buildstream/types.py @@ -105,6 +105,12 @@ class CoreWarnings(): is referenced either on the command line or by another element """ + BAD_CHARACTERS_IN_NAME = "bad-characters-in-name" + """ + This warning will be produces when filename for a target contains invalid + characters in its name. + """ + # _KeyStrength(): # diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst index 57ea4488a..714e1fa33 100644 --- a/doc/source/format_declaring.rst +++ b/doc/source/format_declaring.rst @@ -526,3 +526,27 @@ read-only variables are also dynamically declared by BuildStream: build, support for this is conditional on the element type and the build system used (any element using 'make' can implement this). + + +Naming elements +--------------- +When naming the element files, use the following rules: + +* The name of the file must have ``.bst`` extension. + +* All characters in the name must be printable 7-bit ASCII characters. + +* Following characters are reserved and must not be part of the name: + + - ``<`` (less than) + - ``>`` (greater than) + - ``:`` (colon) + - ``"`` (double quote) + - ``/`` (forward slash) + - ``\`` (backslash) + - ``|`` (vertical bar) + - ``?`` (question mark) + - ``*`` (asterisk) + +BuildStream will attempt to raise warnings when any of these rules are violated +but that may not always be possible. diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index 03bba0b26..dc7ce6847 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -86,6 +86,20 @@ def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks): @pytest.mark.datafiles(DATA_DIR) +def test_build_invalid_filename_chars(datafiles, cli): + project = os.path.join(datafiles.dirname, datafiles.basename) + result = cli.run(project=project, args=strict_args(['build', 'invalid-chars|<>-in-name.bst'], 'non-strict')) + result.assert_main_error(ErrorDomain.LOAD, "bad-characters-in-name") + + +@pytest.mark.datafiles(DATA_DIR) +def test_build_invalid_filename_chars_dep(datafiles, cli): + project = os.path.join(datafiles.dirname, datafiles.basename) + 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")]) def test_build_checkout_deps(datafiles, cli, deps): project = os.path.join(datafiles.dirname, datafiles.basename) diff --git a/tests/frontend/project/elements/invalid-chars-in-dep.bst b/tests/frontend/project/elements/invalid-chars-in-dep.bst new file mode 100644 index 000000000..6a5ec30c8 --- /dev/null +++ b/tests/frontend/project/elements/invalid-chars-in-dep.bst @@ -0,0 +1,8 @@ +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/invalid-chars|<>-in-name.bst b/tests/frontend/project/elements/invalid-chars|<>-in-name.bst new file mode 100644 index 000000000..bc6a13110 --- /dev/null +++ b/tests/frontend/project/elements/invalid-chars|<>-in-name.bst @@ -0,0 +1,4 @@ +kind: stack +description: | + The name of this files contains characters that are not allowed by + BuildStream, using it should raise a warning. diff --git a/tests/frontend/project/project.conf b/tests/frontend/project/project.conf index a7e4a023c..ed18221e4 100644 --- a/tests/frontend/project/project.conf +++ b/tests/frontend/project/project.conf @@ -5,3 +5,4 @@ element-path: elements fatal-warnings: - bad-element-suffix +- bad-characters-in-name |