This is an automated email from the ASF dual-hosted git repository. szaszm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit c20fd9128772112facd15cd12b9ee97043806c89 Author: Ferenc Gerlits <[email protected]> AuthorDate: Mon Aug 22 21:10:36 2022 +0200 MINIFICPP-1900 Make python scripting extension compile on Windows - Fix compile errors: * do the sol2 patch in binary mode * put gcc-specific pragmas and attributes in #def's * undef COMPILER as some include #defines COMPILER as the compiler version * fix the usual Windows filesystem::path -> string stuff - Fix some warnings - Enable the script extension in the Windows CI build - Set up Python in the Windows CI job - Fix the unit tests Closes #1392 Signed-off-by: Marton Szasz <[email protected]> --- .github/workflows/ci.yml | 8 ++++++-- cmake/Sol2.cmake | 2 +- extensions/script/python/ExecutePythonProcessor.h | 4 ++++ extensions/script/python/PyProcessSession.h | 4 ++++ extensions/script/python/PythonCreator.h | 4 +++- extensions/script/python/PythonObjectFactory.h | 4 ++++ extensions/script/python/PythonScriptEngine.cpp | 4 ++-- extensions/script/python/PythonScriptEngine.h | 8 ++++++++ extensions/script/tests/CMakeLists.txt | 2 +- extensions/script/tests/ExecutePythonProcessorTests.cpp | 6 +++--- .../script/tests/TestExecuteScriptProcessorWithPythonScript.cpp | 8 ++++++-- libminifi/include/agent/agent_version.h | 5 +++++ libminifi/include/io/CRCStream.h | 4 ++-- libminifi/include/io/StreamPipe.h | 2 +- libminifi/include/utils/file/FileUtils.h | 2 +- libminifi/src/utils/StringUtils.cpp | 6 +++--- 16 files changed, 54 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eafe39a8e..ffd79921f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,8 +50,12 @@ jobs: run: git config --system core.longpaths true - id: checkout uses: actions/checkout@v2 - - name: Setup PATH + - name: Set up MSBuild uses: microsoft/[email protected] + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.10' - id: install-sqliteodbc-driver run: | Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe"; -OutFile "sqliteodbc_w64.exe" @@ -61,7 +65,7 @@ jobs: run: | PATH %PATH%;C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64 PATH %PATH%;C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Roslyn - win_build_vs.bat ..\b /64 /CI /S /A /PDH /SPLUNK /GCP /ELASTIC /K /L /R /Z /N /RO /PR + win_build_vs.bat ..\b /64 /CI /S /A /PDH /SPLUNK /GCP /ELASTIC /K /L /R /Z /N /RO /PR /SCRIPTING shell: cmd - name: test run: cd ..\b && ctest --timeout 300 --parallel %NUMBER_OF_PROCESSORS% -C Release --output-on-failure diff --git a/cmake/Sol2.cmake b/cmake/Sol2.cmake index 9ba518f9d..aa8042cc7 100644 --- a/cmake/Sol2.cmake +++ b/cmake/Sol2.cmake @@ -28,7 +28,7 @@ if(NOT EXISTS "${SOL2_INCLUDE_DIR}/sol.hpp") file(DOWNLOAD "https://github.com/ThePhD/sol2/releases/download/v3.2.2/forward.hpp"; "${SOL2_INCLUDE_DIR}/sol/forward.hpp" EXPECTED_HASH SHA256=491c59790c242f8ec766deb35a18a5aae34772da3393c1b8f0719f5c50d01fdf) - set(PC "${Patch_EXECUTABLE}" -p1 -i "${CMAKE_SOURCE_DIR}/thirdparty/sol2/add-missing-include.patch" "${SOL2_INCLUDE_DIR}/sol/sol.hpp") + set(PC "${Patch_EXECUTABLE}" --binary -p1 -i "${CMAKE_SOURCE_DIR}/thirdparty/sol2/add-missing-include.patch" "${SOL2_INCLUDE_DIR}/sol/sol.hpp") execute_process(COMMAND ${PC} RESULT_VARIABLE patch_result_code) if(NOT patch_result_code EQUAL "0") diff --git a/extensions/script/python/ExecutePythonProcessor.h b/extensions/script/python/ExecutePythonProcessor.h index 2258580d4..c6384bf00 100644 --- a/extensions/script/python/ExecutePythonProcessor.h +++ b/extensions/script/python/ExecutePythonProcessor.h @@ -32,7 +32,9 @@ #include "PythonScriptEngine.h" #include "core/PropertyBuilder.h" +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility push(hidden) +#endif namespace org { namespace apache { @@ -142,4 +144,6 @@ class ExecutePythonProcessor : public core::Processor { } /* namespace apache */ } /* namespace org */ +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility pop +#endif diff --git a/extensions/script/python/PyProcessSession.h b/extensions/script/python/PyProcessSession.h index 5637159b9..2aa5473da 100644 --- a/extensions/script/python/PyProcessSession.h +++ b/extensions/script/python/PyProcessSession.h @@ -27,7 +27,9 @@ #include "../ScriptFlowFile.h" #include "PyBaseStream.h" +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility push(hidden) +#endif namespace org::apache::nifi::minifi::python { @@ -61,4 +63,6 @@ class PyProcessSession { } // namespace org::apache::nifi::minifi::python +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility pop +#endif diff --git a/extensions/script/python/PythonCreator.h b/extensions/script/python/PythonCreator.h index db79a541b..def743bbd 100644 --- a/extensions/script/python/PythonCreator.h +++ b/extensions/script/python/PythonCreator.h @@ -129,7 +129,9 @@ class PythonCreator : public minifi::core::CoreComponent { return ""; } const auto python_package_path = std::filesystem::relative(pythonscript, basePath).parent_path(); - std::vector<std::string> path_elements{python_package_path.begin(), python_package_path.end()}; + std::vector<std::string> path_elements; + path_elements.reserve(std::distance(python_package_path.begin(), python_package_path.end())); + std::transform(python_package_path.begin(), python_package_path.end(), std::back_inserter(path_elements), [](const auto& path) { return path.string(); }); std::string python_package = minifi::utils::StringUtils::join(".", path_elements); if (python_package.length() > 1 && python_package.at(0) == '.') { python_package = python_package.substr(1); diff --git a/extensions/script/python/PythonObjectFactory.h b/extensions/script/python/PythonObjectFactory.h index d6e84cf41..7f2bd422c 100644 --- a/extensions/script/python/PythonObjectFactory.h +++ b/extensions/script/python/PythonObjectFactory.h @@ -27,7 +27,9 @@ #include "ExecutePythonProcessor.h" #include "utils/StringUtils.h" +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility push(hidden) +#endif class PythonObjectFactory : public org::apache::nifi::minifi::core::DefautObjectFactory<org::apache::nifi::minifi::python::processors::ExecutePythonProcessor> { public: @@ -77,4 +79,6 @@ class PythonObjectFactory : public org::apache::nifi::minifi::core::DefautObject std::string name_; }; +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility pop +#endif diff --git a/extensions/script/python/PythonScriptEngine.cpp b/extensions/script/python/PythonScriptEngine.cpp index 301f53f4e..173be8e93 100644 --- a/extensions/script/python/PythonScriptEngine.cpp +++ b/extensions/script/python/PythonScriptEngine.cpp @@ -48,9 +48,9 @@ void PythonScriptEngine::evaluateModuleImports() { std::string path; std::string filename; utils::file::getFileNameAndPath(module_path, path, filename); - py::eval<py::eval_statements>("sys.path.append('" + path + "')", *bindings_, *bindings_); + py::eval<py::eval_statements>("sys.path.append(r'" + path + "')", *bindings_, *bindings_); } else { - py::eval<py::eval_statements>("sys.path.append('" + module_path + "')", *bindings_, *bindings_); + py::eval<py::eval_statements>("sys.path.append(r'" + module_path + "')", *bindings_, *bindings_); } } } diff --git a/extensions/script/python/PythonScriptEngine.h b/extensions/script/python/PythonScriptEngine.h index 25e48b8e0..dea19a2d8 100644 --- a/extensions/script/python/PythonScriptEngine.h +++ b/extensions/script/python/PythonScriptEngine.h @@ -32,7 +32,9 @@ #include "PyProcessSession.h" #include "../ScriptException.h" +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility push(hidden) +#endif namespace org::apache::nifi::minifi::python { @@ -53,7 +55,11 @@ struct Interpreter { Interpreter *getInterpreter(); +#if defined(__GNUC__) || defined(__GNUG__) class __attribute__((visibility("default"))) PythonScriptEngine : public script::ScriptEngine { +#else +class PythonScriptEngine : public script::ScriptEngine { +#endif public: PythonScriptEngine(); virtual ~PythonScriptEngine() { @@ -231,4 +237,6 @@ class __attribute__((visibility("default"))) PythonScriptEngine : public script: } // namespace org::apache::nifi::minifi::python +#if defined(__GNUC__) || defined(__GNUG__) #pragma GCC visibility pop +#endif diff --git a/extensions/script/tests/CMakeLists.txt b/extensions/script/tests/CMakeLists.txt index e980638cd..23e2cf6f7 100644 --- a/extensions/script/tests/CMakeLists.txt +++ b/extensions/script/tests/CMakeLists.txt @@ -48,7 +48,7 @@ FOREACH(testfile ${EXECUTESCRIPT_PYTHON_TESTS}) target_link_libraries(${testfilename} ${CATCH_MAIN_LIB}) createTests("${testfilename}") MATH(EXPR EXTENSIONS_TEST_COUNT "${EXTENSIONS_TEST_COUNT}+1") - add_test(NAME "${testfilename}" COMMAND "${testfilename}" WORKING_DIRECTORY ${TEST_DIR}) + add_test(NAME "${testfilename}" COMMAND "${testfilename}" WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) ENDFOREACH() FOREACH(testfile ${EXECUTEPYTHONPROCESSOR_UNIT_TESTS}) diff --git a/extensions/script/tests/ExecutePythonProcessorTests.cpp b/extensions/script/tests/ExecutePythonProcessorTests.cpp index ed0d4d216..71c030762 100644 --- a/extensions/script/tests/ExecutePythonProcessorTests.cpp +++ b/extensions/script/tests/ExecutePythonProcessorTests.cpp @@ -145,10 +145,10 @@ class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase { std::shared_ptr<core::Processor> addExecutePythonProcessorToPlan(const std::string& used_as_script_file, const std::string& used_as_script_body) { auto executePythonProcessor = plan_->addProcessor("ExecutePythonProcessor", "executePythonProcessor", core::Relationship("success", "description"), true); - if ("" != used_as_script_file) { + if (!used_as_script_file.empty()) { plan_->setProperty(executePythonProcessor, "Script File", getScriptFullPath(used_as_script_file)); } - if ("" != used_as_script_body) { + if (!used_as_script_body.empty()) { plan_->setProperty(executePythonProcessor, "Script Body", getFileContent(getScriptFullPath(used_as_script_body))); } return executePythonProcessor; @@ -289,7 +289,7 @@ TEST_CASE_METHOD(SimplePythonFlowFileTransferTest, "Test module load of processo addGetFileProcessorToPlan(input_dir); auto execute_python_processor = addExecutePythonProcessorToPlan("foo_bar_processor.py", ""); - plan_->setProperty(execute_python_processor, "Module Directory", getScriptFullPath("foo_modules/foo.py") + "," + getScriptFullPath("bar_modules")); + plan_->setProperty(execute_python_processor, "Module Directory", getScriptFullPath(concat_path("foo_modules", "foo.py")) + "," + getScriptFullPath("bar_modules")); auto success_putfile = plan_->addProcessor("PutFile", "SuccessPutFile", { {"success", "d"} }, false); plan_->addConnection(execute_python_processor, {"success", "d"}, success_putfile); diff --git a/extensions/script/tests/TestExecuteScriptProcessorWithPythonScript.cpp b/extensions/script/tests/TestExecuteScriptProcessorWithPythonScript.cpp index ec53b3ee2..db68cf192 100644 --- a/extensions/script/tests/TestExecuteScriptProcessorWithPythonScript.cpp +++ b/extensions/script/tests/TestExecuteScriptProcessorWithPythonScript.cpp @@ -28,6 +28,7 @@ #include "processors/GetFile.h" #include "processors/PutFile.h" #include "utils/file/FileUtils.h" +#include "utils/file/PathUtils.h" #include "utils/TestUtils.h" TEST_CASE("Script engine is not set", "[executescriptMisconfiguration]") { @@ -357,7 +358,10 @@ TEST_CASE("Python: Test Module Directory property", "[executescriptPythonModuleD logTestController.setDebug<TestPlan>(); logTestController.setDebug<minifi::processors::ExecuteScript>(); - const std::string SCRIPT_FILES_DIRECTORY = concat_path(get_executable_dir(), "test_python_scripts"); + std::string path; + std::string filename; + minifi::utils::file::getFileNameAndPath(__FILE__, path, filename); + const std::string SCRIPT_FILES_DIRECTORY = minifi::utils::file::getFullPath(concat_path(path, "test_python_scripts")); auto getScriptFullPath = [&SCRIPT_FILES_DIRECTORY](const std::string& script_file_name) { return concat_path(SCRIPT_FILES_DIRECTORY, script_file_name); @@ -372,7 +376,7 @@ TEST_CASE("Python: Test Module Directory property", "[executescriptPythonModuleD true); plan->setProperty(executeScript, minifi::processors::ExecuteScript::ScriptFile.getName(), getScriptFullPath("foo_bar_processor.py")); - plan->setProperty(executeScript, minifi::processors::ExecuteScript::ModuleDirectory.getName(), getScriptFullPath("foo_modules/foo.py") + "," + getScriptFullPath("bar_modules")); + plan->setProperty(executeScript, minifi::processors::ExecuteScript::ModuleDirectory.getName(), getScriptFullPath(concat_path("foo_modules", "foo.py")) + "," + getScriptFullPath("bar_modules")); auto getFileDir = testController.createTempDirectory(); plan->setProperty(getFile, minifi::processors::GetFile::Directory.getName(), getFileDir); diff --git a/libminifi/include/agent/agent_version.h b/libminifi/include/agent/agent_version.h index ac38cf281..f1aac9f10 100644 --- a/libminifi/include/agent/agent_version.h +++ b/libminifi/include/agent/agent_version.h @@ -18,6 +18,11 @@ #ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_VERSION_H_ #define LIBMINIFI_INCLUDE_AGENT_AGENT_VERSION_H_ +// The Windows version of pyconfig.h (https://github.com/python/cpython/blob/3.10/PC/pyconfig.h, also on main as of 2022-08-18) +// rather unhelpfully #define's COMPILER as the detected compiler version. Since we have a COMPILER variable below, we need to #undef it +// to make anything which requires cpython (e.g. the script extension) compile on Windows. +#undef COMPILER + #include <string> #include <vector> #include "utils/Export.h" diff --git a/libminifi/include/io/CRCStream.h b/libminifi/include/io/CRCStream.h index 2f2e76696..9afd66da0 100644 --- a/libminifi/include/io/CRCStream.h +++ b/libminifi/include/io/CRCStream.h @@ -87,7 +87,7 @@ class InputCRCStream : public virtual CRCStreamBase<StreamType>, public InputStr size_t read(gsl::span<std::byte> buf) override { const auto ret = child_stream_->read(buf); if (ret > 0 && !io::isError(ret)) { - crc_ = crc32(crc_, reinterpret_cast<const unsigned char*>(buf.data()), ret); + crc_ = crc32(crc_, reinterpret_cast<const unsigned char*>(buf.data()), gsl::narrow<uInt>(ret)); } return ret; } @@ -107,7 +107,7 @@ class OutputCRCStream : public virtual CRCStreamBase<StreamType>, public OutputS size_t write(const uint8_t *value, size_t size) override { const auto ret = child_stream_->write(value, size); if (ret > 0 && !io::isError(ret)) { - crc_ = crc32(crc_, value, ret); + crc_ = crc32(crc_, value, gsl::narrow<uInt>(ret)); } return ret; } diff --git a/libminifi/include/io/StreamPipe.h b/libminifi/include/io/StreamPipe.h index 1e3a23a37..25364ec1a 100644 --- a/libminifi/include/io/StreamPipe.h +++ b/libminifi/include/io/StreamPipe.h @@ -37,7 +37,7 @@ inline int64_t pipe(io::InputStream& src, io::OutputStream& dst) { if (io::isError(readRet)) return -1; if (readRet == 0) break; auto remaining = readRet; - int transferred = 0; + size_t transferred = 0; while (remaining > 0) { const auto writeRet = dst.write(gsl::make_span(buffer).subspan(transferred, remaining)); // TODO(adebreceni): diff --git a/libminifi/include/utils/file/FileUtils.h b/libminifi/include/utils/file/FileUtils.h index 92a6884b2..fa8f02a64 100644 --- a/libminifi/include/utils/file/FileUtils.h +++ b/libminifi/include/utils/file/FileUtils.h @@ -149,7 +149,7 @@ inline int64_t delete_dir(const std::string &path, bool delete_files_recursively std::filesystem::remove(path); } } - } catch (std::filesystem::filesystem_error const &e) { + } catch (std::filesystem::filesystem_error const &) { return -1; // display error message } diff --git a/libminifi/src/utils/StringUtils.cpp b/libminifi/src/utils/StringUtils.cpp index f3a59a081..f18450733 100644 --- a/libminifi/src/utils/StringUtils.cpp +++ b/libminifi/src/utils/StringUtils.cpp @@ -476,9 +476,7 @@ std::string StringUtils::escapeUnprintableBytes(gsl::span<const std::byte> data) std::string result; for (auto byte : data) { char ch = static_cast<char>(byte); - if (std::isprint(static_cast<unsigned char>(ch))) { - result += ch; - } else if (ch == '\n') { + if (ch == '\n') { result += "\\n"; } else if (ch == '\t') { result += "\\t"; @@ -488,6 +486,8 @@ std::string StringUtils::escapeUnprintableBytes(gsl::span<const std::byte> data) result += "\\v"; } else if (ch == '\f') { result += "\\f"; + } else if (std::isprint(static_cast<unsigned char>(byte))) { + result += ch; } else { result += "\\x"; result += hex_digits[(std::to_integer<int>(byte) >> 4) & 0xf];
