Finish pybind11 dependence removal - #227
Conversation
|
This PR helps the work started by @Leengit, if you'd also like to take a look? |
There was a problem hiding this comment.
Pull request overview
This PR updates the CMake build to better support configurations where the Python development environment is disabled (DISABLE_PYTHON=ON), by conditionally excluding Python-related dependencies/targets and adjusting one C++ source include.
Changes:
- Gate inclusion of the
extern/pybind11subdirectory and headers onDISABLE_PYTHON. - Avoid linking
pybind11::embed(and avoid building/linking thetestEmptest utility) when Python is disabled. - Add a missing
<cassert>include inexputil/realize_model.cc.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| CMakeLists.txt | Adds DISABLE_PYTHON handling and conditionally includes/adds extern/pybind11. |
| exputil/CMakeLists.txt | Conditionally links pybind11::embed only when Python is not disabled. |
| utils/Test/CMakeLists.txt | Conditionally builds/links testEmp only when Python is not disabled. |
| exputil/realize_model.cc | Adds <cassert> include to support existing assert(...) usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Do we have a use case for the resulting build that can be tested and verified? @Leengit may have already done this? |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I've built this version of the code on tursa, which has effective no Python (a version so old it can't handle |
|
Agreed. |
This pull request improves support for disabling the Python development environment by making the build system respect the
DISABLE_PYTHONoption throughout the CMake configuration. The changes ensure that Python-related dependencies and targets are only included when Python support is enabled, resulting in a cleaner and more robust build process when Python is not required.Build system improvements for Python support:
CMakeLists.txtto setENABLE_PYEXPtoOFFand print a status message whenDISABLE_PYTHONis enabled, preventing the build of Python-related components.pybind11headers and subdirectory in the mainCMakeLists.txtand only add them if Python is not disabled.exputil/CMakeLists.txtto only link againstpybind11::embedif Python support is enabled, avoiding unnecessary dependencies.Test utilities adjustments:
utils/Test/CMakeLists.txtto only add and link thetestEmpexecutable (and its dependencies) if Python is enabled, ensuring test targets are consistent with the build configuration.Minor code improvement:
#include <cassert>inexputil/realize_model.ccfor improved code safety and correctness.