You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by sz...@apache.org on 2022/08/22 20:13:04 UTC
[nifi-minifi-cpp] 02/05: MINIFICPP-1900 Make python scripting extension compile on Windows
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 <fg...@gmail.com>
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 <sz...@apache.org>
---
.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/setup-msbuild@v1.0.2
+ - 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];