This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/remote-cli-options in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit d3589e9a578d556381d6b6328250ddab62e72aa4 Author: Tristan van Berkom <[email protected]> AuthorDate: Fri Feb 5 16:42:17 2021 +0900 Moving some cython functions into _utils.pyx This moves a function from _loader/_loader.pyx and a function from _scheduler/jobs/_job.pyx into _utils.pyx instead. The reason for this change is that without it, the following commit causes "Unable to import" pylint error E0401 to occur for job.py and for loader.py respectively. It is unclear what the following commit does to trigger these errors, asides from adding some additional type safety from the typing module, it affects mostly _frontend/cli.py, _context.py and _stream.py. This change seems to be a good tradeoff however since the cython modules each offer a single function that is not intertwined in the loader or scheduler code at all, so it may even be better to just provide these in the same soname (this might have load time benefits by reducing the number of dynamic libraries to load). --- .pylintrc | 2 -- setup.py | 2 -- src/buildstream/_loader/_loader.pyi | 1 - src/buildstream/_loader/_loader.pyx | 52 -------------------------------- src/buildstream/_loader/loader.py | 4 +-- src/buildstream/_scheduler/jobs/_job.pyi | 1 - src/buildstream/_scheduler/jobs/_job.pyx | 15 --------- src/buildstream/_scheduler/jobs/job.py | 2 +- src/buildstream/_utils.pyi | 2 ++ src/buildstream/_utils.pyx | 50 ++++++++++++++++++++++++++++++ 10 files changed, 55 insertions(+), 76 deletions(-) diff --git a/.pylintrc b/.pylintrc index 56aa38c..382c81b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -5,10 +5,8 @@ # run arbitrary code extension-pkg-whitelist= buildstream.node, - buildstream._loader._loader, buildstream._loader.loadelement, buildstream._loader.types, - buildstream._scheduler.jobs._job, buildstream._types, buildstream._utils, buildstream._variables, diff --git a/setup.py b/setup.py index 18b5e42..022ecfb 100755 --- a/setup.py +++ b/setup.py @@ -317,9 +317,7 @@ def register_cython_module(module_name, dependencies=None): BUILD_EXTENSIONS = [] register_cython_module("buildstream.node") -register_cython_module("buildstream._loader._loader") register_cython_module("buildstream._loader.loadelement", dependencies=["buildstream.node"]) -register_cython_module("buildstream._scheduler.jobs._job") register_cython_module("buildstream._yaml", dependencies=["buildstream.node"]) register_cython_module("buildstream._types") register_cython_module("buildstream._utils") diff --git a/src/buildstream/_loader/_loader.pyi b/src/buildstream/_loader/_loader.pyi deleted file mode 100644 index c4281b4..0000000 --- a/src/buildstream/_loader/_loader.pyi +++ /dev/null @@ -1 +0,0 @@ -def valid_chars_name(name: str) -> bool: ... diff --git a/src/buildstream/_loader/_loader.pyx b/src/buildstream/_loader/_loader.pyx deleted file mode 100644 index 258a34a..0000000 --- a/src/buildstream/_loader/_loader.pyx +++ /dev/null @@ -1,52 +0,0 @@ -# -# Copyright (C) 2019 Bloomberg L.P. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library. If not, see <http://www.gnu.org/licenses/>. -# -# Authors: -# Benjamin Schubert <[email protected]> -# - - -# 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(str name): - cdef int char_value - cdef int forbidden_char - - for char_value in name: - # 0-31 are control chars, 127 is DEL, and >127 means non-ASCII - if char_value <= 31 or char_value >= 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. - for forbidden_char in '<>"|?*': - if char_value == forbidden_char: - return False - - return True diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 6ace362..48fb1c4 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -27,12 +27,12 @@ from ..element import Element from ..node import Node from .._profile import Topics, PROFILER from .._includes import Includes +from .._utils import valid_chars_name +from ..types import CoreWarnings, _KeyStrength -from ._loader import valid_chars_name from .types import Symbol from . import loadelement from .loadelement import LoadElement, Dependency, DependencyType, extract_depends_from_node -from ..types import CoreWarnings, _KeyStrength # Loader(): diff --git a/src/buildstream/_scheduler/jobs/_job.pyi b/src/buildstream/_scheduler/jobs/_job.pyi deleted file mode 100644 index fbf3e64..0000000 --- a/src/buildstream/_scheduler/jobs/_job.pyi +++ /dev/null @@ -1 +0,0 @@ -def terminate_thread(thread_id: int): ... diff --git a/src/buildstream/_scheduler/jobs/_job.pyx b/src/buildstream/_scheduler/jobs/_job.pyx deleted file mode 100644 index 82f6ab0..0000000 --- a/src/buildstream/_scheduler/jobs/_job.pyx +++ /dev/null @@ -1,15 +0,0 @@ -from cpython.pystate cimport PyThreadState_SetAsyncExc -from cpython.ref cimport PyObject -from ..._signals import TerminateException - - -# terminate_thread() -# -# Ask a given a given thread to terminate by raising an exception in it. -# -# Args: -# thread_id (int): the thread id in which to throw the exception -# -def terminate_thread(long thread_id): - res = PyThreadState_SetAsyncExc(thread_id, <PyObject*> TerminateException) - assert res == 1 diff --git a/src/buildstream/_scheduler/jobs/job.py b/src/buildstream/_scheduler/jobs/job.py index aa71b6e..c875afe 100644 --- a/src/buildstream/_scheduler/jobs/job.py +++ b/src/buildstream/_scheduler/jobs/job.py @@ -30,10 +30,10 @@ import traceback # BuildStream toplevel imports from ... import utils +from ..._utils import terminate_thread from ..._exceptions import ImplError, BstError, set_last_task_error, SkipJob from ..._message import Message, MessageType, unconditional_messages from ...types import FastEnum -from ._job import terminate_thread from ..._signals import TerminateException diff --git a/src/buildstream/_utils.pyi b/src/buildstream/_utils.pyi index 4584372..1938eec 100644 --- a/src/buildstream/_utils.pyi +++ b/src/buildstream/_utils.pyi @@ -1 +1,3 @@ def url_directory_name(url: str) -> str: ... +def valid_chars_name(name: str) -> bool: ... +def terminate_thread(thread_id: int) -> None: ... diff --git a/src/buildstream/_utils.pyx b/src/buildstream/_utils.pyx index 6605cc8..2386ef4 100644 --- a/src/buildstream/_utils.pyx +++ b/src/buildstream/_utils.pyx @@ -22,6 +22,10 @@ This module contains utilities that have been optimized in Cython """ +from cpython.pystate cimport PyThreadState_SetAsyncExc +from cpython.ref cimport PyObject +from ._signals import TerminateException + def url_directory_name(str url): """Normalizes a url into a directory name @@ -35,6 +39,52 @@ def url_directory_name(str url): return ''.join([_transl(x) for x in url]) + +# terminate_thread() +# +# Ask a given a given thread to terminate by raising an exception in it. +# +# Args: +# thread_id (int): the thread id in which to throw the exception +# +def terminate_thread(long thread_id): + res = PyThreadState_SetAsyncExc(thread_id, <PyObject*> TerminateException) + assert res == 1 + + +# 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(str name): + cdef int char_value + cdef int forbidden_char + + for char_value in name: + # 0-31 are control chars, 127 is DEL, and >127 means non-ASCII + if char_value <= 31 or char_value >= 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. + for forbidden_char in '<>"|?*': + if char_value == forbidden_char: + return False + + return True + + ############################################################# # Module local helper Methods # #############################################################
