From 66105e8a3a8cfc54410d57a48eb0bfc81715417f Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 09:48:23 +0100 Subject: [PATCH 1/3] Add invalid imports checker Since plugins.* is not available on the PATH for some builds, they should not be used. Relative imports are preferred --- docker/build.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docker/build.sh b/docker/build.sh index 5b035ca08a..9bff78c2a3 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -13,6 +13,19 @@ export PKG_CONFIG_PATH="${CURA_BUILD_ENV_PATH}/lib/pkgconfig:${PKG_CONFIG_PATH}" cd "${PROJECT_DIR}" +# Check for plugins.* import statements. These imports may work when running from source, +# but will fail in some build types (linux and mac) +GREP_OUTPUT=$(grep -Ern "^\s*(from plugins|import plugins)" --include \*.py "${PROJECT_DIR}" || true) +echo "$GREP_OUTPUT" + +if [ -z "$GREP_OUTPUT" ] +then + echo "invalid imports checker: OK" +else + echo "error: sources contain invalid imports. Use relative imports when referencing plugin source files" + exit 1 +fi + # # Clone Uranium and set PYTHONPATH first # From b830a6faa3f83814b795fdca41bb73b90e48ac68 Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 16:22:02 +0100 Subject: [PATCH 2/3] Rewrite invalid imports checker to Python Makes it consistent with other checkers we already have --- cmake/CuraTests.cmake | 7 ++++ docker/build.sh | 11 ------ scripts/check_invalid_imports.py | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 scripts/check_invalid_imports.py diff --git a/cmake/CuraTests.cmake b/cmake/CuraTests.cmake index b1d3e0ddc4..c76019d310 100644 --- a/cmake/CuraTests.cmake +++ b/cmake/CuraTests.cmake @@ -56,6 +56,13 @@ function(cura_add_test) endif() endfunction() +#Add test for whether the shortcut alt-keys are unique in every translation. +add_test( + NAME "invalid-imports" + COMMAND ${Python3_EXECUTABLE} scripts/check_invalid_imports.py + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} +) + cura_add_test(NAME pytest-main DIRECTORY ${CMAKE_SOURCE_DIR}/tests PYTHONPATH "${CMAKE_SOURCE_DIR}|${URANIUM_DIR}") file(GLOB_RECURSE _plugins plugins/*/__init__.py) diff --git a/docker/build.sh b/docker/build.sh index 9bff78c2a3..a500663c64 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -13,18 +13,7 @@ export PKG_CONFIG_PATH="${CURA_BUILD_ENV_PATH}/lib/pkgconfig:${PKG_CONFIG_PATH}" cd "${PROJECT_DIR}" -# Check for plugins.* import statements. These imports may work when running from source, -# but will fail in some build types (linux and mac) -GREP_OUTPUT=$(grep -Ern "^\s*(from plugins|import plugins)" --include \*.py "${PROJECT_DIR}" || true) -echo "$GREP_OUTPUT" -if [ -z "$GREP_OUTPUT" ] -then - echo "invalid imports checker: OK" -else - echo "error: sources contain invalid imports. Use relative imports when referencing plugin source files" - exit 1 -fi # # Clone Uranium and set PYTHONPATH first diff --git a/scripts/check_invalid_imports.py b/scripts/check_invalid_imports.py new file mode 100644 index 0000000000..121184e739 --- /dev/null +++ b/scripts/check_invalid_imports.py @@ -0,0 +1,60 @@ +import os +import re +import sys +from pathlib import Path + +""" +Run this file with the Cura project root as the working directory +""" + +class InvalidImportsChecker: + # compile regex + REGEX = re.compile(r"^\s*(from plugins|import plugins)") + + def check(self): + """ Checks for invalid imports + + :return: True if checks passed, False when the test fails + """ + cwd = os.getcwd() + cura_result = checker.check_dir(os.path.join(cwd, "cura")) + plugins_result = checker.check_dir(os.path.join(cwd, "plugins")) + result = cura_result and plugins_result + if not result: + print("error: sources contain invalid imports. Use relative imports when referencing plugin source files") + + return result + + def check_dir(self, root_dir: str) -> bool: + """ Checks a directory for invalid imports + + :return: True if checks passed, False when the test fails + """ + passed = True + for path_like in Path(root_dir).rglob('*.py'): + if not self.check_file(str(path_like)): + passed = False + + return passed + + def check_file(self, file_path): + """ Checks a file for invalid imports + + :return: True if checks passed, False when the test fails + """ + passed = True + with open(file_path, 'r', encoding = "utf-8") as inputFile: + # loop through each line in file + for line_i, line in enumerate(inputFile, 1): + # check if we have a regex match + match = self.REGEX.search(line) + if match: + path = os.path.relpath(file_path) + print("{path}:{line_i}:{match}".format(path=path, line_i=line_i, match=match.group(1))) + passed = False + return passed + + +if __name__ == "__main__": + checker = InvalidImportsChecker() + sys.exit(0 if checker.check() else 1) From 2e20bf6a983dfd4fcfad56aecb3ce41f53621b31 Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 16:33:03 +0100 Subject: [PATCH 3/3] Update invalid imports checker documentation Makes it consistent with other checkers we already have --- cmake/CuraTests.cmake | 2 +- scripts/check_invalid_imports.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmake/CuraTests.cmake b/cmake/CuraTests.cmake index c76019d310..251bec5781 100644 --- a/cmake/CuraTests.cmake +++ b/cmake/CuraTests.cmake @@ -56,7 +56,7 @@ function(cura_add_test) endif() endfunction() -#Add test for whether the shortcut alt-keys are unique in every translation. +#Add test for import statements which are not compatible with all builds add_test( NAME "invalid-imports" COMMAND ${Python3_EXECUTABLE} scripts/check_invalid_imports.py diff --git a/scripts/check_invalid_imports.py b/scripts/check_invalid_imports.py index 121184e739..ba21b9f822 100644 --- a/scripts/check_invalid_imports.py +++ b/scripts/check_invalid_imports.py @@ -5,8 +5,13 @@ from pathlib import Path """ Run this file with the Cura project root as the working directory +Checks for invalid imports. When importing from plugins, there will be no problems when running from source, +but for some build types the plugins dir is not on the path, so relative imports should be used instead. eg: +from ..UltimakerCloudScope import UltimakerCloudScope <-- OK +import plugins.Toolbox.src ... <-- NOT OK """ + class InvalidImportsChecker: # compile regex REGEX = re.compile(r"^\s*(from plugins|import plugins)")