From 67610496f4a4e3ded0e23b1e5b28918be10f9e0e Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Thu, 8 Jan 2026 16:38:23 -0600 Subject: [PATCH 01/10] Use find_package arguments to mandate NumPy This change allows a CMake-only replacement to introducing numpy build dependencies. Also included is the replacement of one find_package(Python3 ...) command with find_package(Python ...) as Python2 is not supported. --- Modules/private/CreateCoverageTargets.cmake | 30 ++-- plugins/python/CMakeLists.txt | 95 +++-------- plugins/python/src/modulewrap.cpp | 14 +- plugins/python/src/pymodule.cpp | 8 - test/python/CMakeLists.txt | 176 ++++++++++---------- 5 files changed, 123 insertions(+), 200 deletions(-) diff --git a/Modules/private/CreateCoverageTargets.cmake b/Modules/private/CreateCoverageTargets.cmake index d4360c8a..f8844e64 100644 --- a/Modules/private/CreateCoverageTargets.cmake +++ b/Modules/private/CreateCoverageTargets.cmake @@ -27,8 +27,8 @@ find_program(LCOV_EXECUTABLE lcov) find_program(GENHTML_EXECUTABLE genhtml) find_program(GCOVR_EXECUTABLE gcovr) -# Find Python3 and normalization scripts -find_package(Python3 COMPONENTS Interpreter) +# Find Python and normalization scripts +find_package(Python COMPONENTS Interpreter) # Find CTest coverage tool find_program(LLVM_COV_EXECUTABLE NAMES llvm-cov-21 llvm-cov DOC "LLVM coverage tool") @@ -183,7 +183,7 @@ function(_create_coverage_targets_impl) ${CMAKE_COMMAND} -E echo "[Coverage] Exporting LLVM coverage data to LCOV (${LLVM_COV_LCOV_OUTPUT})" COMMAND - ${Python3_EXECUTABLE} "${LLVM_COV_EXPORT_SCRIPT}" ${LLVM_COV_LCOV_OUTPUT} + ${Python_EXECUTABLE} "${LLVM_COV_EXPORT_SCRIPT}" ${LLVM_COV_LCOV_OUTPUT} ${LLVM_COV_EXECUTABLE} export ${LLVM_COV_OBJECTS} -instr-profile=${LLVM_PROFDATA_OUTPUT} "-ignore-filename-regex=${LLVM_COV_EXCLUDE_REGEX}" --format=lcov COMMENT "Exporting LLVM coverage data to LCOV" @@ -196,12 +196,12 @@ function(_create_coverage_targets_impl) # Normalization target for llvm-cov output (if Python script exists) set(_normalize_llvm_script "${PROJECT_SOURCE_DIR}/scripts/normalize_coverage_lcov.py") set(LLVM_COV_NORMALIZED_STAMP ${CMAKE_BINARY_DIR}/coverage-llvm-normalized.stamp) - if(Python3_FOUND AND EXISTS "${_normalize_llvm_script}") + if(Python_FOUND AND EXISTS "${_normalize_llvm_script}") add_custom_command( OUTPUT ${LLVM_COV_NORMALIZED_STAMP} DEPENDS ${LLVM_COV_LCOV_OUTPUT} COMMAND - ${Python3_EXECUTABLE} "${_normalize_llvm_script}" --repo-root "${PROJECT_SOURCE_DIR}" + ${Python_EXECUTABLE} "${_normalize_llvm_script}" --repo-root "${PROJECT_SOURCE_DIR}" --coverage-root "${PROJECT_SOURCE_DIR}" --coverage-alias "${PROJECT_SOURCE_DIR}" "${LLVM_COV_LCOV_OUTPUT}" COMMAND ${CMAKE_COMMAND} -E touch ${LLVM_COV_NORMALIZED_STAMP} @@ -216,7 +216,7 @@ function(_create_coverage_targets_impl) coverage-llvm-normalize COMMAND ${CMAKE_COMMAND} -E echo - "ERROR: Python3 or normalize_coverage_lcov.py not found. Cannot normalize LLVM coverage report." + "ERROR: Python or normalize_coverage_lcov.py not found. Cannot normalize LLVM coverage report." COMMAND exit 1 WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Failed to normalize LLVM coverage report" @@ -287,11 +287,11 @@ function(_create_coverage_targets_impl) set(_coverage_symlink_script "${PROJECT_SOURCE_DIR}/scripts/create_coverage_symlinks.py") set(_coverage_symlink_root "${PROJECT_SOURCE_DIR}/.coverage-generated") - if(Python3_FOUND AND EXISTS "${_coverage_symlink_script}") + if(Python_FOUND AND EXISTS "${_coverage_symlink_script}") add_custom_target( coverage-symlink-prepare COMMAND - ${Python3_EXECUTABLE} "${_coverage_symlink_script}" --build-root "${CMAKE_BINARY_DIR}" + ${Python_EXECUTABLE} "${_coverage_symlink_script}" --build-root "${CMAKE_BINARY_DIR}" --output-root "${_coverage_symlink_root}" WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Preparing generated symlink tree (.coverage-generated)" @@ -304,7 +304,7 @@ function(_create_coverage_targets_impl) COMMAND ${CMAKE_COMMAND} -E make_directory "${_coverage_symlink_root}" COMMAND ${CMAKE_COMMAND} -E echo - "WARNING: Python3 or create_coverage_symlinks.py missing; generated symlink tree will be empty." + "WARNING: Python or create_coverage_symlinks.py missing; generated symlink tree will be empty." WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Preparing generated symlink tree (.coverage-generated)" ) @@ -383,11 +383,11 @@ function(_create_coverage_targets_impl) ) # HTML normalization target (depends on symlink preparation) - if(Python3_FOUND AND EXISTS "${_normalize_lcov_script}") + if(Python_FOUND AND EXISTS "${_normalize_lcov_script}") add_custom_target( coverage-html-normalize COMMAND - ${Python3_EXECUTABLE} "${_normalize_lcov_script}" --repo-root "${PROJECT_SOURCE_DIR}" + ${Python_EXECUTABLE} "${_normalize_lcov_script}" --repo-root "${PROJECT_SOURCE_DIR}" --coverage-root "${PROJECT_SOURCE_DIR}" --coverage-alias "${PROJECT_SOURCE_DIR}" "${CMAKE_BINARY_DIR}/coverage.info.final" WORKING_DIRECTORY ${CMAKE_BINARY_DIR} @@ -400,7 +400,7 @@ function(_create_coverage_targets_impl) coverage-html-normalize COMMAND ${CMAKE_COMMAND} -E echo - "ERROR: Python3 or normalize_coverage_lcov.py not found. Cannot normalize HTML coverage report." + "ERROR: Python or normalize_coverage_lcov.py not found. Cannot normalize HTML coverage report." COMMAND exit 1 WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Failed to normalize LCOV HTML coverage report" @@ -469,11 +469,11 @@ function(_create_coverage_targets_impl) ) # XML normalization target (depends on symlink preparation) - if(Python3_FOUND AND EXISTS "${_normalize_xml_script}") + if(Python_FOUND AND EXISTS "${_normalize_xml_script}") add_custom_target( coverage-xml-normalize COMMAND - ${Python3_EXECUTABLE} "${_normalize_xml_script}" --repo-root "${PROJECT_SOURCE_DIR}" + ${Python_EXECUTABLE} "${_normalize_xml_script}" --repo-root "${PROJECT_SOURCE_DIR}" --source-dir "${PROJECT_SOURCE_DIR}" --path-map "${CMAKE_BINARY_DIR}=${PROJECT_SOURCE_DIR}/.coverage-generated" "${CMAKE_BINARY_DIR}/coverage.xml" @@ -487,7 +487,7 @@ function(_create_coverage_targets_impl) coverage-xml-normalize COMMAND ${CMAKE_COMMAND} -E echo - "ERROR: Python3 or normalize_coverage_xml.py not found. Cannot normalize XML coverage report." + "ERROR: Python or normalize_coverage_xml.py not found. Cannot normalize XML coverage report." COMMAND exit 1 WORKING_DIRECTORY ${CMAKE_BINARY_DIR} COMMENT "Failed to normalize XML coverage report" diff --git a/plugins/python/CMakeLists.txt b/plugins/python/CMakeLists.txt index 9b76f2a4..d7fb220d 100644 --- a/plugins/python/CMakeLists.txt +++ b/plugins/python/CMakeLists.txt @@ -1,75 +1,24 @@ -find_package(Python 3.12 COMPONENTS Interpreter Development QUIET) +find_package(Python 3.12 COMPONENTS Interpreter Development NumPy REQUIRED) -if(Python_FOUND) - # Verify installation of necessary python modules for specific tests - - function(check_python_module_version MODULE_NAME MIN_VERSION OUT_VAR) - execute_process( - COMMAND - ${Python_EXECUTABLE} -c - "import sys -try: - import ${MODULE_NAME} - from packaging.version import parse as parse_version - installed_version = getattr(${MODULE_NAME}, '__version__', None) - if parse_version(installed_version) >= parse_version('${MIN_VERSION}'): - sys.exit(0) - else: - sys.exit(2) # Version too low -except ImportError: - sys.exit(1)" - RESULT_VARIABLE _module_check_result - ) - - if(_module_check_result EQUAL 0) - set(${OUT_VAR} TRUE PARENT_SCOPE) - elseif(_module_check_result EQUAL 1) - set(${OUT_VAR} FALSE PARENT_SCOPE) # silent b/c common - elseif(_module_check_result EQUAL 2) - message( - WARNING - "Python module '${MODULE_NAME}' found but version too low (min required: ${MIN_VERSION})." - ) - set(${OUT_VAR} FALSE PARENT_SCOPE) - else() - message(WARNING "Unknown error while checking Python module '${MODULE_NAME}'.") - set(${OUT_VAR} FALSE PARENT_SCOPE) - endif() - endfunction() - - check_python_module_version("numpy" "2.0.0" HAS_NUMPY) - - # Phlex module to run Python algorithms - add_library( - pymodule - MODULE - src/pymodule.cpp - src/modulewrap.cpp - src/configwrap.cpp - src/lifelinewrap.cpp - src/errorwrap.cpp +if(Python_NumPy_VERSION VERSION_LESS "2.0.0") + message( + FATAL_ERROR + "NumPy version is too low: ${Python_NumPy_VERSION} found, at least 2.0.0 required" ) - include_directories(pymodule, ${Python_INCLUDE_DIRS}) - target_link_libraries(pymodule PRIVATE phlex::module ${Python_LIBRARIES} PUBLIC Python::Python) - - install(TARGETS pymodule LIBRARY DESTINATION lib) - - # numpy support if installed - if(HAS_NUMPY) - # locate numpy's header directory - execute_process( - COMMAND "${Python_EXECUTABLE}" -c "import numpy; print(numpy.get_include(), end='')" - RESULT_VARIABLE NUMPY_RESULT - OUTPUT_VARIABLE NUMPY_INCLUDE_DIR - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - - if(NUMPY_RESULT EQUAL 0) - include_directories(pymodule PRIVATE ${NUMPY_INCLUDE_DIR}) - target_compile_definitions( - pymodule - PRIVATE PHLEX_HAVE_NUMPY=1 NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION - ) - endif() - endif() -endif() # Python available +endif() + +# Phlex module to run Python algorithms +add_library( + pymodule + MODULE + src/pymodule.cpp + src/modulewrap.cpp + src/configwrap.cpp + src/lifelinewrap.cpp + src/errorwrap.cpp +) +target_link_libraries(pymodule PRIVATE phlex::module Python::Python Python::NumPy) + +target_compile_definitions(pymodule PRIVATE NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION) + +install(TARGETS pymodule LIBRARY DESTINATION lib) diff --git a/plugins/python/src/modulewrap.cpp b/plugins/python/src/modulewrap.cpp index e1525a42..00b123d6 100644 --- a/plugins/python/src/modulewrap.cpp +++ b/plugins/python/src/modulewrap.cpp @@ -7,11 +7,9 @@ #include #include -#ifdef PHLEX_HAVE_NUMPY #define NO_IMPORT_ARRAY #define PY_ARRAY_UNIQUE_SYMBOL phlex_ARRAY_API #include -#endif using namespace phlex::experimental; using phlex::concurrency; @@ -316,7 +314,6 @@ namespace { BASIC_CONVERTER(float, float, PyFloat_FromDouble, PyFloat_AsDouble) BASIC_CONVERTER(double, double, PyFloat_FromDouble, PyFloat_AsDouble) -#ifdef PHLEX_HAVE_NUMPY #define VECTOR_CONVERTER(name, cpptype, nptype) \ static intptr_t name##_to_py(std::shared_ptr> const& v) \ { \ @@ -394,7 +391,6 @@ namespace { NUMPY_ARRAY_CONVERTER(vulong, unsigned long, NPY_ULONG) NUMPY_ARRAY_CONVERTER(vfloat, float, NPY_FLOAT) NUMPY_ARRAY_CONVERTER(vdouble, double, NPY_DOUBLE) -#endif } // unnamed namespace @@ -549,7 +545,6 @@ static bool insert_input_converters(py_phlex_module* mod, INSERT_INPUT_CONVERTER(float, cname, inp); else if (inp_type == "double") INSERT_INPUT_CONVERTER(double, cname, inp); -#ifdef PHLEX_HAVE_NUMPY else if (inp_type.compare(0, 13, "numpy.ndarray") == 0) { // TODO: these are hard-coded std::vector <-> numpy array mappings, which is // way too simplistic for real use. It only exists for demonstration purposes, @@ -595,9 +590,7 @@ static bool insert_input_converters(py_phlex_module* mod, PyErr_Format(PyExc_TypeError, "unsupported array input type \"%s\"", inp_type.c_str()); return false; } - } -#endif - else { + } else { PyErr_Format(PyExc_TypeError, "unsupported input type \"%s\"", inp_type.c_str()); return false; } @@ -676,7 +669,6 @@ static PyObject* md_transform(py_phlex_module* mod, PyObject* args, PyObject* kw INSERT_OUTPUT_CONVERTER(float, cname, output); else if (output_type == "double") INSERT_OUTPUT_CONVERTER(double, cname, output); -#ifdef PHLEX_HAVE_NUMPY else if (output_type.compare(0, 13, "numpy.ndarray") == 0) { // TODO: just like for input types, these are hard-coded, but should be handled by // an IDL instead. @@ -721,9 +713,7 @@ static PyObject* md_transform(py_phlex_module* mod, PyObject* args, PyObject* kw PyErr_Format(PyExc_TypeError, "unsupported array output type \"%s\"", output_type.c_str()); return nullptr; } - } -#endif - else { + } else { PyErr_Format(PyExc_TypeError, "unsupported output type \"%s\"", output_type.c_str()); return nullptr; } diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index ef86de63..e193b3bd 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -7,10 +7,8 @@ #include "wrap.hpp" -#ifdef PHLEX_HAVE_NUMPY #define PY_ARRAY_UNIQUE_SYMBOL phlex_ARRAY_API #include -#endif using namespace phlex::experimental; @@ -48,7 +46,6 @@ PHLEX_REGISTER_ALGORITHMS(m, config) } } -#ifdef PHLEX_HAVE_NUMPY static void import_numpy(bool control_interpreter) { static std::atomic numpy_imported{false}; @@ -61,14 +58,11 @@ static void import_numpy(bool control_interpreter) } } } -#endif static bool initialize() { if (Py_IsInitialized()) { -#ifdef PHLEX_HAVE_NUMPY import_numpy(false); -#endif return true; } @@ -122,9 +116,7 @@ static bool initialize() return false; // load numpy (see also above, if already initialized) -#ifdef PHLEX_HAVE_NUMPY import_numpy(true); -#endif // TODO: the GIL should first be released on the main thread and this seems // to be the only place to do it. However, there is no equivalent place to diff --git a/test/python/CMakeLists.txt b/test/python/CMakeLists.txt index fb1cbdca..9a0c559f 100644 --- a/test/python/CMakeLists.txt +++ b/test/python/CMakeLists.txt @@ -1,13 +1,10 @@ -find_package(Python 3.12 COMPONENTS Interpreter Development QUIET) +# Verify installation of necessary python modules for specific tests -if(Python_FOUND) - # Verify installation of necessary python modules for specific tests - - function(check_python_module_version MODULE_NAME MIN_VERSION OUT_VAR) - execute_process( - COMMAND - ${Python_EXECUTABLE} -c - "import sys +function(check_python_module_version MODULE_NAME MIN_VERSION OUT_VAR) + execute_process( + COMMAND + ${Python_EXECUTABLE} -c + "import sys try: import ${MODULE_NAME} from packaging.version import parse as parse_version @@ -18,97 +15,92 @@ try: sys.exit(2) # Version too low except ImportError: sys.exit(1)" - RESULT_VARIABLE _module_check_result - ) + RESULT_VARIABLE _module_check_result + ) - if(_module_check_result EQUAL 0) - set(${OUT_VAR} TRUE PARENT_SCOPE) - elseif(_module_check_result EQUAL 1) - set(${OUT_VAR} FALSE PARENT_SCOPE) # silent b/c common - elseif(_module_check_result EQUAL 2) - message( - WARNING - "Python module '${MODULE_NAME}' found but version too low (min required: ${MIN_VERSION})." - ) - set(${OUT_VAR} FALSE PARENT_SCOPE) - else() - message(WARNING "Unknown error while checking Python module '${MODULE_NAME}'.") - set(${OUT_VAR} FALSE PARENT_SCOPE) - endif() - endfunction() - - check_python_module_version("cppyy" "3.6.0" HAS_CPPYY) - check_python_module_version("numba" "0.61.0" HAS_NUMBA) - check_python_module_version("numpy" "2.0.0" HAS_NUMPY) - - if(HAS_CPPYY) - # Explicitly define Python executable variable to ensure it is visible in - # the test environment - set(PYTHON_TEST_EXECUTABLE ${Python_EXECUTABLE}) - - # Export the location of phlex include headers - if(DEFINED ENV{PHLEX_INSTALL}) - set(PYTHON_TEST_PHLEX_INSTALL $ENV{PHLEX_INSTALL}) - else() - set(PYTHON_TEST_PHLEX_INSTALL ${CMAKE_SOURCE_DIR}) - endif() - - # tests of the python support modules (relies on cppyy) - add_test( - NAME py:phlex - COMMAND ${PYTHON_TEST_EXECUTABLE} -m pytest test_phlex.py - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + if(_module_check_result EQUAL 0) + set(${OUT_VAR} TRUE PARENT_SCOPE) + elseif(_module_check_result EQUAL 1) + set(${OUT_VAR} FALSE PARENT_SCOPE) # silent b/c common + elseif(_module_check_result EQUAL 2) + message( + WARNING + "Python module '${MODULE_NAME}' found but version too low (min required: ${MIN_VERSION})." ) - - set_property(TEST py:phlex PROPERTY ENVIRONMENT "PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}") + set(${OUT_VAR} FALSE PARENT_SCOPE) + else() + message(WARNING "Unknown error while checking Python module '${MODULE_NAME}'.") + set(${OUT_VAR} FALSE PARENT_SCOPE) endif() +endfunction() - set(ACTIVE_PY_CPHLEX_TESTS "") +check_python_module_version("cppyy" "3.6.0" HAS_CPPYY) +check_python_module_version("numba" "0.61.0" HAS_NUMBA) - # C++ helper to provide a driver - add_library(cppsource4py MODULE source.cpp) - target_link_libraries(cppsource4py PRIVATE phlex::module) +if(HAS_CPPYY) + # Explicitly define Python executable variable to ensure it is visible in + # the test environment + set(PYTHON_TEST_EXECUTABLE ${Python_EXECUTABLE}) - # numpy support if installed - if(HAS_NUMPY) - # phlex-based tests that require numpy support - add_test(NAME py:vec COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyvec.jsonnet) - list(APPEND ACTIVE_PY_CPHLEX_TESTS py:vec) + # Export the location of phlex include headers + if(DEFINED ENV{PHLEX_INSTALL}) + set(PYTHON_TEST_PHLEX_INSTALL $ENV{PHLEX_INSTALL}) + else() + set(PYTHON_TEST_PHLEX_INSTALL ${CMAKE_SOURCE_DIR}) endif() - # phlex-based tests (no cppyy dependency) - add_test(NAME py:add COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyadd.jsonnet) - list(APPEND ACTIVE_PY_CPHLEX_TESTS py:add) - - add_test(NAME py:config COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyconfig.jsonnet) - list(APPEND ACTIVE_PY_CPHLEX_TESTS py:config) - - add_test(NAME py:reduce COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyreduce.jsonnet) - list(APPEND ACTIVE_PY_CPHLEX_TESTS py:reduce) - - # "failing" tests for checking error paths + # tests of the python support modules (relies on cppyy) add_test( - NAME py:failure - COMMAND - ${CMAKE_CURRENT_SOURCE_DIR}/failing_test_wrap.sh ${PROJECT_BINARY_DIR}/bin/phlex -c - ${CMAKE_CURRENT_SOURCE_DIR}/pyfailure.jsonnet - ) - set_tests_properties( - py:failure - PROPERTIES PASS_REGULAR_EXPRESSION "property \"input\" does not exist" + NAME py:phlex + COMMAND ${PYTHON_TEST_EXECUTABLE} -m pytest test_phlex.py + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} ) - list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) - set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}) - if(DEFINED ENV{VIRTUAL_ENV}) - set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${Python_SITELIB}:${Python_SITEARCH}) - endif() - set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:$ENV{PYTHONPATH}) - - set_tests_properties( - ${ACTIVE_PY_CPHLEX_TESTS} - PROPERTIES - ENVIRONMENT - "SPDLOG_LEVEL=debug;PHLEX_PLUGIN_PATH=${PROJECT_BINARY_DIR};PYTHONPATH=${TEST_PYTHONPATH};PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}" - ) -endif() # Python available + set_property(TEST py:phlex PROPERTY ENVIRONMENT "PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}") +endif() + +set(ACTIVE_PY_CPHLEX_TESTS "") + +# C++ helper to provide a driver +add_library(cppsource4py MODULE source.cpp) +target_link_libraries(cppsource4py PRIVATE phlex::module) + +# phlex-based tests that require numpy support +add_test(NAME py:vec COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyvec.jsonnet) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:vec) + +# phlex-based tests (no cppyy dependency) +add_test(NAME py:add COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyadd.jsonnet) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:add) + +add_test(NAME py:config COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyconfig.jsonnet) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:config) + +add_test(NAME py:reduce COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyreduce.jsonnet) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:reduce) + +# "failing" tests for checking error paths +add_test( + NAME py:failure + COMMAND + ${CMAKE_CURRENT_SOURCE_DIR}/failing_test_wrap.sh ${PROJECT_BINARY_DIR}/bin/phlex -c + ${CMAKE_CURRENT_SOURCE_DIR}/pyfailure.jsonnet +) +set_tests_properties( + py:failure + PROPERTIES PASS_REGULAR_EXPRESSION "property \"input\" does not exist" +) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) + +set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}) +if(DEFINED ENV{VIRTUAL_ENV}) + set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${Python_SITELIB}:${Python_SITEARCH}) +endif() +set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:$ENV{PYTHONPATH}) + +set_tests_properties( + ${ACTIVE_PY_CPHLEX_TESTS} + PROPERTIES + ENVIRONMENT + "SPDLOG_LEVEL=debug;PHLEX_PLUGIN_PATH=${PROJECT_BINARY_DIR};PYTHONPATH=${TEST_PYTHONPATH};PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}" +) From 57fa9f2250fae8ad0ce2551bd7d1770e7453a188 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 11:50:13 -0600 Subject: [PATCH 02/10] Accommodate VIRTUAL_ENV in C++ and Spack infelicities --- plugins/python/src/pymodule.cpp | 50 +++++++++++++++++++++++++++++++++ test/python/CMakeLists.txt | 6 +--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index e193b3bd..860cd4be 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -90,6 +90,16 @@ static bool initialize() PyConfig config; PyConfig_InitPythonConfig(&config); PyConfig_SetString(&config, &config.program_name, L"phlex"); + + // Ensure Python finds site-packages from a virtual environment if it exists + if (char const* python_home = std::getenv("VIRTUAL_ENV")) { + wchar_t* whome = Py_DecodeLocale(python_home, nullptr); + if (whome) { + PyConfig_SetString(&config, &config.home, whome); + PyMem_RawFree(whome); + } + } + Py_InitializeFromConfig(&config); #endif #if PY_VERSION_HEX >= 0x03020000 @@ -115,6 +125,46 @@ static bool initialize() if (PyType_Ready(&PhlexLifeline_Type) < 0) return false; + // FIXME: Spack does not set PYTHONPATH or VIRTUAL_ENV, but it does set + // CMAKE_PREFIX_PATH. Add site-packages directories from CMAKE_PREFIX_PATH + // to sys.path so Python can find packages in Spack views. + if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { + std::string prefix_path_str(cmake_prefix_path); + std::istringstream iss(prefix_path_str); + std::string path_entry; + std::vector site_package_paths; + + // Build site-packages paths from each CMAKE_PREFIX_PATH token + while (std::getline(iss, path_entry, ':')) { + if (!path_entry.empty()) { + std::string version_str = + std::to_string(PY_MAJOR_VERSION) + "." + std::to_string(PY_MINOR_VERSION); + std::string site_packages = path_entry + "/lib/python" + version_str + "/site-packages"; + site_package_paths.push_back(site_packages); + } + } + + // Add each site-packages path to sys.path + if (!site_package_paths.empty()) { + PyObject* sys = PyImport_ImportModule("sys"); + if (sys) { + PyObject* sys_path = PyObject_GetAttrString(sys, "path"); + if (sys_path && PyList_Check(sys_path)) { + for (auto const& sp_path : site_package_paths) { + PyObject* py_path = PyUnicode_FromString(sp_path.c_str()); + if (py_path) { + // Insert at beginning to prioritize these paths + PyList_Insert(sys_path, 0, py_path); + Py_DECREF(py_path); + } + } + } + Py_XDECREF(sys_path); + Py_DECREF(sys); + } + } + } + // load numpy (see also above, if already initialized) import_numpy(true); diff --git a/test/python/CMakeLists.txt b/test/python/CMakeLists.txt index 9a0c559f..19aebd5c 100644 --- a/test/python/CMakeLists.txt +++ b/test/python/CMakeLists.txt @@ -92,11 +92,7 @@ set_tests_properties( ) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) -set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}) -if(DEFINED ENV{VIRTUAL_ENV}) - set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${Python_SITELIB}:${Python_SITEARCH}) -endif() -set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:$ENV{PYTHONPATH}) +set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}:$ENV{PYTHONPATH}) set_tests_properties( ${ACTIVE_PY_CPHLEX_TESTS} From 43d6eca78a115907e75b936cb259c6546ae85507 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 12:01:59 -0600 Subject: [PATCH 03/10] Refactor code --- plugins/python/src/pymodule.cpp | 74 ++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index 860cd4be..0f06ec22 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -59,6 +59,45 @@ static void import_numpy(bool control_interpreter) } } +static void add_cmake_prefix_paths_to_syspath(char const* cmake_prefix_path) +{ + std::string prefix_path_str(cmake_prefix_path); + std::istringstream iss(prefix_path_str); + std::string path_entry; + std::vector site_package_paths; + + // Build site-packages paths from each CMAKE_PREFIX_PATH token + while (std::getline(iss, path_entry, ':')) { + if (!path_entry.empty()) { + std::string version_str = + std::to_string(PY_MAJOR_VERSION) + "." + std::to_string(PY_MINOR_VERSION); + std::string site_packages = path_entry + "/lib/python" + version_str + "/site-packages"; + site_package_paths.push_back(site_packages); + } + } + + // Add each site-packages path to sys.path + if (site_package_paths.empty()) { + return; + } + + if (PyObject* sys = PyImport_ImportModule("sys")) { + if (PyObject* sys_path = PyObject_GetAttrString(sys, "path")) { + if (PyList_Check(sys_path)) { + for (auto const& sp_path : site_package_paths) { + if (PyObject* py_path = PyUnicode_FromString(sp_path.c_str())) { + // Insert at beginning to prioritize these paths + PyList_Insert(sys_path, 0, py_path); + Py_DECREF(py_path); + } + } + } + Py_DECREF(sys_path); + } + Py_DECREF(sys); + } +} + static bool initialize() { if (Py_IsInitialized()) { @@ -129,40 +168,7 @@ static bool initialize() // CMAKE_PREFIX_PATH. Add site-packages directories from CMAKE_PREFIX_PATH // to sys.path so Python can find packages in Spack views. if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { - std::string prefix_path_str(cmake_prefix_path); - std::istringstream iss(prefix_path_str); - std::string path_entry; - std::vector site_package_paths; - - // Build site-packages paths from each CMAKE_PREFIX_PATH token - while (std::getline(iss, path_entry, ':')) { - if (!path_entry.empty()) { - std::string version_str = - std::to_string(PY_MAJOR_VERSION) + "." + std::to_string(PY_MINOR_VERSION); - std::string site_packages = path_entry + "/lib/python" + version_str + "/site-packages"; - site_package_paths.push_back(site_packages); - } - } - - // Add each site-packages path to sys.path - if (!site_package_paths.empty()) { - PyObject* sys = PyImport_ImportModule("sys"); - if (sys) { - PyObject* sys_path = PyObject_GetAttrString(sys, "path"); - if (sys_path && PyList_Check(sys_path)) { - for (auto const& sp_path : site_package_paths) { - PyObject* py_path = PyUnicode_FromString(sp_path.c_str()); - if (py_path) { - // Insert at beginning to prioritize these paths - PyList_Insert(sys_path, 0, py_path); - Py_DECREF(py_path); - } - } - } - Py_XDECREF(sys_path); - Py_DECREF(sys); - } - } + add_cmake_prefix_paths_to_syspath(cmake_prefix_path); } // load numpy (see also above, if already initialized) From 14cc26af8ace08e3709b4c49cd49463659ca465e Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 12:07:27 -0600 Subject: [PATCH 04/10] Remove code intended for unsupported Python versions --- plugins/python/src/pymodule.cpp | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index 0f06ec22..0f96b0ba 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -120,12 +120,6 @@ static bool initialize() dlopen(info.dli_fname, RTLD_GLOBAL | RTLD_NOW); } -#if PY_VERSION_HEX < 0x03020000 - PyEval_InitThreads(); -#endif -#if PY_VERSION_HEX < 0x03080000 - Py_Initialize(); -#else PyConfig config; PyConfig_InitPythonConfig(&config); PyConfig_SetString(&config, &config.program_name, L"phlex"); @@ -140,22 +134,11 @@ static bool initialize() } Py_InitializeFromConfig(&config); -#endif -#if PY_VERSION_HEX >= 0x03020000 -#if PY_VERSION_HEX < 0x03090000 - PyEval_InitThreads(); -#endif -#endif + // try again to see if the interpreter is now initialized if (!Py_IsInitialized()) throw std::runtime_error("Python can not be initialized"); -#if PY_VERSION_HEX < 0x03080000 - // set the command line arguments on python's sys.argv - wchar_t* argv[] = {const_cast(L"phlex")}; - PySys_SetArgv(sizeof(argv) / sizeof(argv[0]), argv); -#endif - // add custom types if (PyType_Ready(&PhlexConfig_Type) < 0) return false; From 39f8d2da28f84443dc126b815f3ad226143fa7e5 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 12:11:28 -0600 Subject: [PATCH 05/10] Slight cleanup --- plugins/python/src/pymodule.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index 0f96b0ba..e99ed3e6 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -126,8 +126,7 @@ static bool initialize() // Ensure Python finds site-packages from a virtual environment if it exists if (char const* python_home = std::getenv("VIRTUAL_ENV")) { - wchar_t* whome = Py_DecodeLocale(python_home, nullptr); - if (whome) { + if (wchar_t* whome = Py_DecodeLocale(python_home, nullptr)) { PyConfig_SetString(&config, &config.home, whome); PyMem_RawFree(whome); } From 6d339310177a414235ace8e571b6a4d632f01460 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 13:38:18 -0600 Subject: [PATCH 06/10] Use encouraged API --- plugins/python/src/pymodule.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index e99ed3e6..e8568370 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -126,10 +126,7 @@ static bool initialize() // Ensure Python finds site-packages from a virtual environment if it exists if (char const* python_home = std::getenv("VIRTUAL_ENV")) { - if (wchar_t* whome = Py_DecodeLocale(python_home, nullptr)) { - PyConfig_SetString(&config, &config.home, whome); - PyMem_RawFree(whome); - } + PyConfig_SetBytesString(&config, &config.home, python_home); } Py_InitializeFromConfig(&config); From dfe9bcd891fa23877bc5ba8983c367d59383ba53 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 13:38:36 -0600 Subject: [PATCH 07/10] Consult CMAKE_PREFIX_PATH only when VIRTUAL_ENV is empty --- plugins/python/src/pymodule.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index e8568370..1238ef84 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -146,8 +146,10 @@ static bool initialize() // FIXME: Spack does not set PYTHONPATH or VIRTUAL_ENV, but it does set // CMAKE_PREFIX_PATH. Add site-packages directories from CMAKE_PREFIX_PATH // to sys.path so Python can find packages in Spack views. - if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { - add_cmake_prefix_paths_to_syspath(cmake_prefix_path); + if (!std::getenv("VIRTUAL_ENV")) { + if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { + add_cmake_prefix_paths_to_syspath(cmake_prefix_path); + } } // load numpy (see also above, if already initialized) From 575be3fb2943ff4046d494e4e7d1e2ac76bc9cd0 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 15:10:18 -0600 Subject: [PATCH 08/10] Test that Python virtual environments are treated correctly --- plugins/python/src/pymodule.cpp | 9 ++++----- test/python/CMakeLists.txt | 23 ++++++++++++++++++++--- test/python/check_sys_path.py | 32 ++++++++++++++++++++++++++++++++ test/python/pysyspath.jsonnet.in | 20 ++++++++++++++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 test/python/check_sys_path.py create mode 100644 test/python/pysyspath.jsonnet.in diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index 1238ef84..b1c6e029 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -124,13 +124,12 @@ static bool initialize() PyConfig_InitPythonConfig(&config); PyConfig_SetString(&config, &config.program_name, L"phlex"); - // Ensure Python finds site-packages from a virtual environment if it exists - if (char const* python_home = std::getenv("VIRTUAL_ENV")) { - PyConfig_SetBytesString(&config, &config.home, python_home); + PyStatus status = Py_InitializeFromConfig(&config); + PyConfig_Clear(&config); + if (PyStatus_Exception(status)) { + throw std::runtime_error("Python initialization failed"); } - Py_InitializeFromConfig(&config); - // try again to see if the interpreter is now initialized if (!Py_IsInitialized()) throw std::runtime_error("Python can not be initialized"); diff --git a/test/python/CMakeLists.txt b/test/python/CMakeLists.txt index 19aebd5c..7af3b297 100644 --- a/test/python/CMakeLists.txt +++ b/test/python/CMakeLists.txt @@ -1,3 +1,5 @@ +find_package(Python 3.12 COMPONENTS Interpreter REQUIRED) + # Verify installation of necessary python modules for specific tests function(check_python_module_version MODULE_NAME MIN_VERSION OUT_VAR) @@ -92,11 +94,26 @@ set_tests_properties( ) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) +# Environment variables required: set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}:$ENV{PYTHONPATH}) +set( + PYTHON_TEST_ENVIRONMENT + "SPDLOG_LEVEL=debug;PHLEX_PLUGIN_PATH=${PROJECT_BINARY_DIR};PYTHONPATH=${TEST_PYTHONPATH};PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}" +) + +set_tests_properties(${ACTIVE_PY_CPHLEX_TESTS} PROPERTIES ENVIRONMENT "${PYTHON_TEST_ENVIRONMENT}") + +# phlex-based test to ensure that sys.path is sanely set for virtual environment +set(PY_VIRTUAL_ENV_DIR ${CMAKE_CURRENT_BINARY_DIR}/py_virtual_env) +execute_process(COMMAND python -m venv ${PY_VIRTUAL_ENV_DIR}) +configure_file(pysyspath.jsonnet.in pysyspath.jsonnet @ONLY) +add_test(NAME py:syspath COMMAND phlex -c ${CMAKE_CURRENT_BINARY_DIR}/pysyspath.jsonnet) +# Activate the Python virtual environment "by hand". Requires setting the VIRTUAL_ENV +# environment variable and prepending to the PATH environment variable. set_tests_properties( - ${ACTIVE_PY_CPHLEX_TESTS} + py:syspath PROPERTIES - ENVIRONMENT - "SPDLOG_LEVEL=debug;PHLEX_PLUGIN_PATH=${PROJECT_BINARY_DIR};PYTHONPATH=${TEST_PYTHONPATH};PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}" + ENVIRONMENT "${PYTHON_TEST_ENVIRONMENT};VIRTUAL_ENV=${PY_VIRTUAL_ENV_DIR}" + ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:${PY_VIRTUAL_ENV_DIR}/bin" ) diff --git a/test/python/check_sys_path.py b/test/python/check_sys_path.py new file mode 100644 index 00000000..88d7be1a --- /dev/null +++ b/test/python/check_sys_path.py @@ -0,0 +1,32 @@ +"""An observer to check for output in tests. + +Test algorithms produce outputs. To ensure that a test is run correctly, +this observer verifies its result against the expected value. +""" + +import sys + +class Checker: + + __name__ = 'checker' + + def __init__(self, venv_path: str): + self._venv_path = venv_path + + def __call__(self, i: int) -> None: + assert len(sys.path) > 0 + venv_site_packages = f"{sys.prefix}/lib/python{sys.version_info.major}.{sys.version_info.minor}/site-packages" + assert any(p == venv_site_packages for p in sys.path) + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register check_sys_path for checking sys.path. + + Args: + m (internal): Phlex registrar representation. + config (internal): Phlex configuration representation. + + Returns: + None + """ + m.observe(Checker(config["venv"]), input_family = config["input"]) diff --git a/test/python/pysyspath.jsonnet.in b/test/python/pysyspath.jsonnet.in new file mode 100644 index 00000000..4330b780 --- /dev/null +++ b/test/python/pysyspath.jsonnet.in @@ -0,0 +1,20 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 10, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + pysyspath: { + py: 'check_sys_path', + input: ['i'], + venv: '@PY_VIRTUAL_ENV_DIR@', + }, + }, +} From 6198357037629f7ebf3ef564bc5251807b87e68b Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 15:51:27 -0600 Subject: [PATCH 09/10] Address ruff issues (courtesy Claude Sonnet 4.5) --- test/python/check_sys_path.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/test/python/check_sys_path.py b/test/python/check_sys_path.py index 88d7be1a..f767c4dd 100644 --- a/test/python/check_sys_path.py +++ b/test/python/check_sys_path.py @@ -6,16 +6,38 @@ import sys + class Checker: + """Observer that verifies virtual environment site-packages is in sys.path. + + This checker ensures that when a virtual environment is active, its + site-packages directory appears in Python's sys.path. + """ __name__ = 'checker' def __init__(self, venv_path: str): + """Initialize the checker with the expected virtual environment path. + + Args: + venv_path: Path to the virtual environment directory. + """ self._venv_path = venv_path def __call__(self, i: int) -> None: + """Verify that the virtual environment's site-packages is in sys.path. + + Args: + i: number provided by upstream provider (unused). + + Raises: + AssertionError: If sys.path is empty or virtual environment + site-packages is not found in sys.path. + """ assert len(sys.path) > 0 - venv_site_packages = f"{sys.prefix}/lib/python{sys.version_info.major}.{sys.version_info.minor}/site-packages" + venv_site_packages = ( + f"{sys.prefix}/lib/python{sys.version_info.major}.{sys.version_info.minor}/site-packages" + ) assert any(p == venv_site_packages for p in sys.path) From e626d52e9519102e203ab45474b909bc1a9acc5c Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Tue, 13 Jan 2026 15:51:49 -0600 Subject: [PATCH 10/10] Remove explicit check on VIRTUAL_ENV --- plugins/python/src/pymodule.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/python/src/pymodule.cpp b/plugins/python/src/pymodule.cpp index b1c6e029..c3dac985 100644 --- a/plugins/python/src/pymodule.cpp +++ b/plugins/python/src/pymodule.cpp @@ -145,10 +145,8 @@ static bool initialize() // FIXME: Spack does not set PYTHONPATH or VIRTUAL_ENV, but it does set // CMAKE_PREFIX_PATH. Add site-packages directories from CMAKE_PREFIX_PATH // to sys.path so Python can find packages in Spack views. - if (!std::getenv("VIRTUAL_ENV")) { - if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { - add_cmake_prefix_paths_to_syspath(cmake_prefix_path); - } + if (char const* cmake_prefix_path = std::getenv("CMAKE_PREFIX_PATH")) { + add_cmake_prefix_paths_to_syspath(cmake_prefix_path); } // load numpy (see also above, if already initialized)