diff --git a/CMakeLists.txt b/CMakeLists.txt index 51f4e091159..5a0c7d17f3e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ message(STATUS "Using CMake ${CMAKE_VERSION}") -cmake_minimum_required(VERSION 3.19.0) +cmake_minimum_required(VERSION 3.20.0) cmake_policy(SET CMP0048 NEW) set(CMAKE_CXX_STANDARD 20) diff --git a/python/SetupPython.cmake b/python/SetupPython.cmake index 356552175fa..15811293264 100644 --- a/python/SetupPython.cmake +++ b/python/SetupPython.cmake @@ -7,9 +7,9 @@ set(Python_USE_STATIC_LIBS OFF) # find_package(Python) has the problem that on github actions in particular it'll pick up the most recent python (eg 3.9) from the tool cache # even if you have used the setup-python action and set it to 3.8 if (PYTHON_VERSION) - find_package(Python ${PYTHON_VERSION} EXACT COMPONENTS Interpreter Development REQUIRED) + find_package(Python ${PYTHON_VERSION} EXACT REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy) else() - find_package(Python COMPONENTS Interpreter Development REQUIRED) + find_package(Python REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy) endif() execute_process(COMMAND ${Python_EXECUTABLE} -m pytest --version diff --git a/python/engine/PythonEngine.cpp b/python/engine/PythonEngine.cpp index 7ed95f7254d..86a6559da41 100644 --- a/python/engine/PythonEngine.cpp +++ b/python/engine/PythonEngine.cpp @@ -30,9 +30,12 @@ void addToPythonPath(const openstudio::path& includePath) { if (!includePath.empty()) { PyObject* sys = PyImport_ImportModule("sys"); PyObject* sysPath = PyObject_GetAttrString(sys, "path"); + Py_DECREF(sys); // PyImport_ImportModule returns a new reference, decrement it + // fmt::print("Prepending '{}' to sys.path\n", includePath); PyObject* unicodeIncludePath = PyUnicode_FromString(includePath.string().c_str()); PyList_Insert(sysPath, 0, unicodeIncludePath); + Py_DECREF(sysPath); // PyObject_GetAttrString returns a new reference, decrement it } } @@ -41,30 +44,56 @@ void PythonEngine::pyimport(const std::string& importName, const std::string& in PyImport_ImportModule(importName.c_str()); } -void PythonEngine::setupPythonPath(const std::vector& includeDirs, const openstudio::path& pythonHomeDir) { - for (const auto& includeDir : includeDirs) { - addToPythonPath(includeDir); - } - if (!pythonHomeDir.empty()) { - wchar_t* a = Py_DecodeLocale(pythonHomeDir.generic_string().c_str(), nullptr); - Py_SetPythonHome(a); +void PythonEngine::setupPythonPath(const std::vector& includeDirs) { + + // Iterate in reverse order since addToPythonPath always inserts at pos 0 + // --python_path path1 --python_path path2 => includeDirs = ["path1", "path2"] + // std::ranges::reverse_view needs modern compilers + for (auto it = includeDirs.rbegin(); it != includeDirs.rend(); it++) { + addToPythonPath(*it); } } PythonEngine::PythonEngine(int argc, char* argv[]) : ScriptEngine(argc, argv), program(Py_DecodeLocale(pythonProgramName, nullptr)) { + // TODO: modernize and use PyConfig (new in 3.8): https://docs.python.org/3/c-api/init_config.html + // this frozen flag tells Python that the package and library have been frozen for embedding, so it shouldn't warn about missing prefixes Py_FrozenFlag = 1; - // Set the PYTHONPATH / PYTHONHOME to the E+ shipped standard library - // I think we need to set the python path before initializing the library + // Path to the E+ shipped standard library auto pathToPythonPackages = getEnergyPlusDirectory() / "python_standard_lib"; - wchar_t* a = Py_DecodeLocale(pathToPythonPackages.make_preferred().string().c_str(), nullptr); - Py_SetPath(a); - Py_SetPythonHome(a); + + // The PYTHONPATH / PYTHONHOME should be set before initializing Python + // If this Py_SetPath is called before Py_Initialize, then Py_GetPath won't attempt to compute a default search path + // The default search path is affected by the Py_SetPythonHome + // * if the user passed --python_home, we use that as the Python Home, and do not use Py_SetPath. But later we add the E+ standard_lib anyways + // so it takes precedence (to limit incompatibility issues...) + // * If the user didn't pass it, we use Py_SetPath set to the E+ standard_lib + + std::vector args(argv, std::next(argv, static_cast(argc))); + bool pythonHomePassed = false; + auto it = std::find(args.cbegin(), args.cend(), "--python_home"); + if (it != args.cend()) { + openstudio::path pythonHomeDir(*std::next(it)); + wchar_t* h = Py_DecodeLocale(pythonHomeDir.make_preferred().string().c_str(), nullptr); + Py_SetPythonHome(h); + pythonHomePassed = true; + } else { + wchar_t* a = Py_DecodeLocale(pathToPythonPackages.make_preferred().string().c_str(), nullptr); + Py_SetPath(a); + } Py_SetProgramName(program); // optional but recommended Py_Initialize(); + + if (pythonHomePassed) { + addToPythonPath(pathToPythonPackages); + } +#if defined(__APPLE__) || defined(__linux___) || defined(__unix__) + addToPythonPath(pathToPythonPackages / "lib-dynload"); +#endif + PyObject* m = PyImport_AddModule("__main__"); if (m == nullptr) { throw std::runtime_error("Unable to add module __main__ for python script execution"); diff --git a/python/engine/PythonEngine.hpp b/python/engine/PythonEngine.hpp index 8979c08de25..c2a03bad1cb 100644 --- a/python/engine/PythonEngine.hpp +++ b/python/engine/PythonEngine.hpp @@ -30,7 +30,7 @@ class PythonEngine final : public ScriptEngine ScriptObject eval(std::string_view sv) override; void exec(std::string_view sv) override; - virtual void setupPythonPath(const std::vector& includeDirs, const openstudio::path& pythonHomeDir) override; + virtual void setupPythonPath(const std::vector& includeDirs) override; protected: void* getAs_impl(ScriptObject& obj, const std::type_info&) override; diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index 9e2426749df..36c5296ab3e 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -337,6 +337,61 @@ if(BUILD_TESTING) # ============ EndForward a Path properly no matter the slashes employed ============ + # ============ Native Ruby Gems / Python Modules - C extensions ============= + if (Python_NumPy_FOUND) + + if(NOT EXISTS "${Python_ROOT_DIR}") + # Python_STDLIB: we expect it to be + # Unix: ~/.pyenv/versions/3.8.12/lib/python3.8 or + # or on CI: /usr/lib/python3.8/ ... with numpy if install via pip3 and not apt install python3-numpy in `/usr/local/lib/python3.8/dist-packages/` + # Windows C:\Python38\Lib + cmake_path(GET Python_STDLIB PARENT_PATH Python_ROOT_DIR) + if(UNIX) + cmake_path(GET Python_ROOT_DIR PARENT_PATH Python_ROOT_DIR) + endif() + endif() + + if(UNIX) + if(EXISTS "${Python_SITELIB}") + set(PYTHON_PATH "${Python_SITELIB}" "${Python_STDLIB}/lib-dynload") + else() + set(PYTHON_PATH "${Python_STDLIB}/lib-dynload") + endif() + + if(NOT APPLE) + set(EXTRA_LOCAL_DIST "/usr/local/lib/python3.8/dist-packages") + if (EXISTS "${EXTRA_LOCAL_DIST}") + list(APPEND PYTHON_PATH "${EXTRA_LOCAL_DIST}") + endif() + endif() + else() + set(PYTHON_PATH "$") + endif() + + message(DEBUG "PYTHON_PATH=${PYTHON_PATH}") + + add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.explicit_sys_path_insert + COMMAND $ labs execute_python_script execute_python_script_with_numpy.py ${Python_STDLIB} + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/" + ) + + add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.python_path + COMMAND $ labs + "$<$:--python_path;$>" + execute_python_script execute_python_script_with_numpy.py + COMMAND_EXPAND_LISTS + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/" + ) + + add_test(NAME OpenStudioCLI.Labs.execute_python_script.numpy.python_home + COMMAND $ labs + --python_home "$" + execute_python_script execute_python_script_with_numpy.py + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test/" + ) + else() + message(AUTHOR_WARNING "Cannot run the python numpy test, as numpy isn't installed on your system python") + endif() file(GLOB RUBY_TEST_SRC # find all CLI test diff --git a/src/cli/main.cpp b/src/cli/main.cpp index 1d4a6d89839..3834de4caca 100644 --- a/src/cli/main.cpp +++ b/src/cli/main.cpp @@ -136,6 +136,7 @@ int main(int argc, char* argv[]) { experimentalApp ->add_option("-I,--include", includeDirs, "Add additional directory to add to front of Ruby $LOAD_PATH (may be used more than once)") ->option_text("DIR") + ->check(CLI::ExistingDirectory) ->group(rubySpecificOptionsGroupName); std::vector gemPathDirs; @@ -143,21 +144,25 @@ int main(int argc, char* argv[]) { ->add_option("--gem_path", gemPathDirs, "Add additional directory to add to front of GEM_PATH environment variable (may be used more than once)") ->option_text("DIR") + ->check(CLI::ExistingDirectory) ->group(rubySpecificOptionsGroupName); openstudio::path gemHomeDir; experimentalApp->add_option("--gem_home", gemHomeDir, "Set GEM_HOME environment variable") ->option_text("DIR") + ->check(CLI::ExistingDirectory) ->group(rubySpecificOptionsGroupName); openstudio::path bundleGemFilePath; experimentalApp->add_option("--bundle", bundleGemFilePath, "Use bundler for GEMFILE") ->option_text("GEMFILE") + ->check(CLI::ExistingFile) ->group(rubySpecificOptionsGroupName); openstudio::path bundleGemDirPath; experimentalApp->add_option("--bundle_path", bundleGemDirPath, "Use bundler installed gems in BUNDLE_PATH") ->option_text("BUNDLE_PATH") + ->check(CLI::ExistingDirectory) ->group(rubySpecificOptionsGroupName); // std::vector @@ -190,16 +195,19 @@ int main(int argc, char* argv[]) { ->add_option("--python_path", pythonPathDirs, "Add additional directory to add to front of PYTHONPATH environment variable (may be used more than once)") ->option_text("DIR") + ->check(CLI::ExistingDirectory) ->group(pythonSpecificOptionsGroupName); openstudio::path pythonHomeDir; experimentalApp->add_option("--python_home", pythonHomeDir, "Set PYTHONHOME environment variable") ->option_text("DIR") + ->check(CLI::ExistingDirectory) ->group(pythonSpecificOptionsGroupName); // This is a callback that's stored on the ScriptEngineInstance, triggered only the first time - std::function runSetupPythonPath = [&pythonEngine, &pythonPathDirs, &pythonHomeDir]() { - pythonEngine->setupPythonPath(pythonPathDirs, pythonHomeDir); + std::function runSetupPythonPath = [&pythonEngine, &pythonPathDirs]() { + // pythonHomeDir is retrieved from (argc, argv) actually, as Py_SetPythonHome has to be called before Py_Initialize + pythonEngine->setupPythonPath(pythonPathDirs); pythonEngine->registerType("openstudio::measure::ModelMeasure *"); pythonEngine->registerType("openstudio::measure::EnergyPlusMeasure *"); pythonEngine->registerType("openstudio::measure::ReportingMeasure *"); diff --git a/src/cli/test/execute_python_script_with_numpy.py b/src/cli/test/execute_python_script_with_numpy.py new file mode 100644 index 00000000000..8f205edbd3f --- /dev/null +++ b/src/cli/test/execute_python_script_with_numpy.py @@ -0,0 +1,49 @@ +"""Test for importing a python package with a C extension, numpy here.""" + +import sys +from pathlib import Path + + +def test_import_numpy(): + """Common test function.""" + print(f"{sys.path=}") + import numpy as np + + a = np.array([0, 1, 2, 3]) + assert a.sum() == 6 + + +def test_with_sys_path_explicit(std_lib: Path): + """Explicitly insert site-packages and lib-dynload in sys.path.""" + site_pack = std_lib / "site-packages" + if site_pack.exists(): + sys.path.insert(0, str(site_pack)) + # Only for unix + dynload = std_lib / "lib-dynload" + if dynload.exists(): + sys.path.insert(0, str(dynload)) + # Only for debian + dist_pack = std_lib.parent.parent / 'local/lib/python3.8/dist-packages' + if dist_pack.exists(): + sys.path.insert(0, str(dist_pack)) + + test_import_numpy() + + +def test_with_pythonpath_pythonhome(): + """Rely on passing PYTHONPATH/PYTHONHOME.""" + test_import_numpy() + + +if __name__ == "__main__": + if len(sys.argv) == 2: + std_lib = Path(sys.argv[1]) + print("Passed {std_lib=}") + + if std_lib.is_dir(): + test_with_sys_path_explicit(std_lib) + else: + raise ValueError(f"Python std_lib doesn't exist at {std_lib}") + + else: + test_with_pythonpath_pythonhome() diff --git a/src/scriptengine/ScriptEngine.hpp b/src/scriptengine/ScriptEngine.hpp index 60d5cb620be..51dc8b8f395 100644 --- a/src/scriptengine/ScriptEngine.hpp +++ b/src/scriptengine/ScriptEngine.hpp @@ -61,7 +61,7 @@ class ScriptEngine const openstudio::path& gemHomeDir, const openstudio::path& bundleGemFilePath, const openstudio::path& bundleGemDirPath, const std::string& bundleWithoutGroups){}; - virtual void setupPythonPath(const std::vector& includeDirs, const openstudio::path& pythonHomeDir){}; + virtual void setupPythonPath(const std::vector& includeDirs){}; template T getAs(ScriptObject& obj) { diff --git a/src/utilities/core/DynamicLibraryPOSIX.hpp b/src/utilities/core/DynamicLibraryPOSIX.hpp index d9eaa976524..f3043d3e85f 100644 --- a/src/utilities/core/DynamicLibraryPOSIX.hpp +++ b/src/utilities/core/DynamicLibraryPOSIX.hpp @@ -31,8 +31,17 @@ struct DynamicLibrary return symbol; } - explicit DynamicLibrary(openstudio::path location) - : m_location{std::move(location)}, m_handle{dlopen(m_location.c_str(), RTLD_LAZY | RTLD_LOCAL), m_handle_deleter} { + explicit DynamicLibrary(openstudio::path location) : m_location{std::move(location)} { + int flags = RTLD_LAZY | RTLD_LOCAL; // NOLINT(misc-const-correctness, hicpp-signed-bitwise) + + // This seems to work on Mac without RTLD_GLOBAL... +#ifdef __linux__ + if (m_location.filename().generic_string().find("python") != std::string::npos) { + // https://stackoverflow.com/questions/67891197/ctypes-cpython-39-x86-64-linux-gnu-so-undefined-symbol-pyfloat-type-in-embedd + flags = RTLD_LAZY | RTLD_GLOBAL; + } +#endif + m_handle = {dlopen(m_location.c_str(), flags), m_handle_deleter}; if (!m_handle) { throw std::runtime_error(fmt::format("Unable to load library '{}', reason: '{}'", m_location.string(), dlerror())); }