fix CMake not finding MPI#179
Open
s-ol wants to merge 2 commits into
Open
Conversation
Member
|
Does fail CI, so not mergable as-is. |
elliejs
added a commit
to elliejs/pywrap
that referenced
this pull request
Jun 25, 2026
Several bugs in the binding generator caused incorrect or missing C++
code in the generated pybind11 bindings. These affect all platforms.
header.py — type resolution (_underlying_type):
Nested types defined inside template classes (typedefs like
value_type, enums like BehaviorOnTransform, POD types like
size_type) were emitted as bare names without their enclosing class
qualification. In generated static_cast expressions, the compiler
cannot resolve a bare "value_type" — it needs the fully qualified
"NCollection_Array1<T>::value_type". Three new blocks handle this:
non-POD typedefs (134 types), POD typedefs (3 types), and nested
enums/records (17 types). The non-POD typedef block also fixes a
corruption pattern where the upstream replace() call could produce
"X<T>::typename X<T>::Y", and prevents double typename prefixes.
header.py — default values (_default_value):
Fix a typo in the ArgInfo dataclass field name: "arg_defualt" was
never matching what Jinja2 templates reference as "arg_default".
This silently dropped every default parameter value for every
function on every platform. Also strip "const" from brace-init
defaults — "const gp_Pnt{ }" is a syntax error (8 parameters).
macros.j2 — default value emission:
The argnames macro had {% if d %} where d is undefined in Jinja2
(always falsy), so default values were never emitted even if
header.py produced them. Fixed to {% if arg.arg_default %}.
New default_value macro handles null pointer defaults correctly:
pybind11 expects py::none() for optional pointer arguments, not
static_cast<T*>(0) which was previously emitted (40 parameters).
Fix arg.name → arg.arg_name in init_outputs_byref — the ArgInfo
dataclass field is arg_name. The undefined arg.name produced empty
variable names in generated C++ for by-reference smart pointer
methods.
macros.j2 — template type qualification:
arg_type_with_template_params had a bug where it checked the raw
input string t instead of ns.t, and only handled types starting
with the bare class name (ClassName::member). Types where the class
name already had template params (ClassName<T>::Iterator) were not
qualified with typename, causing compile failures. Added a second
branch for this case. Same fix applied to template_return_type.
New argtypes_for_template macro applies template type qualification
to py::init<> type lists. Without this, template class constructor
argument types were emitted without typename qualification, causing
dependent name lookup failures.
Removed dead inline type-expansion code in argnames_template that
computed a namespace variable ns.t which was never used — the
actual default emission already called arg_type_with_template_params.
macros.j2 — trampoline class deduplication:
Pure virtual method deduplication compared by short name (m.name),
so overloaded virtuals like Values(Vec&, Real&, Vec&) and
Values(Vec&, Real&, Vec&, Mat&) were treated as duplicates. Changed
to use m.full_name (full signature). Also added the missing
methods.append call for private virtuals, which allowed parent-class
loops to re-emit methods already overridden. 20 classes affected.
CMakeLists.j2 (all-platform changes):
Bump cmake_minimum_required to 3.17. Add enable_language(C) on
non-Windows platforms (required when VTK links MPI::MPI_C). Add
find_package(MPI QUIET) so cmake can resolve MPI::MPI_C and
MPI::MPI_CXX targets referenced by VTK builds that include MPI
support (resolves CadQuery/OCP#179). Add find_package(fmt) and link
fmt::fmt — FreeBSD's OCCT port links against libfmt externally,
while conda builds bundle it statically so this is a no-op there.
Add status messages for Python lib path and VTK version to aid CI
debugging.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
elliejs
added a commit
to elliejs/pywrap
that referenced
this pull request
Jun 25, 2026
Several bugs in the binding generator caused incorrect or missing C++
code in the generated pybind11 bindings. These affect all platforms.
header.py — type resolution (_underlying_type):
Nested types defined inside template classes (typedefs like
value_type, enums like BehaviorOnTransform, POD types like
size_type) were emitted as bare names without their enclosing class
qualification. In generated static_cast expressions, the compiler
cannot resolve a bare "value_type" — it needs the fully qualified
"NCollection_Array1<T>::value_type". Three new blocks handle this:
non-POD typedefs (134 types), POD typedefs (3 types), and nested
enums/records (17 types). The non-POD typedef block also fixes a
corruption pattern where the upstream replace() call could produce
"X<T>::typename X<T>::Y", and prevents double typename prefixes.
header.py — default values (_default_value):
Fix a typo in the ArgInfo dataclass field name: "arg_defualt" was
never matching what Jinja2 templates reference as "arg_default".
This silently dropped every default parameter value for every
function on every platform. Also strip "const" from brace-init
defaults — "const gp_Pnt{ }" is a syntax error (8 parameters).
macros.j2 — default value emission:
The argnames macro had {% if d %} where d is undefined in Jinja2
(always falsy), so default values were never emitted even if
header.py produced them. Fixed to {% if arg.arg_default %}.
New default_value macro handles null pointer defaults correctly:
pybind11 expects py::none() for optional pointer arguments, not
static_cast<T*>(0) which was previously emitted (40 parameters).
Fix arg.name → arg.arg_name in init_outputs_byref — the ArgInfo
dataclass field is arg_name. The undefined arg.name produced empty
variable names in generated C++ for by-reference smart pointer
methods.
macros.j2 — template type qualification:
arg_type_with_template_params had a bug where it checked the raw
input string t instead of ns.t, and only handled types starting
with the bare class name (ClassName::member). Types where the class
name already had template params (ClassName<T>::Iterator) were not
qualified with typename, causing compile failures. Added a second
branch for this case. Same fix applied to template_return_type.
New argtypes_for_template macro applies template type qualification
to py::init<> type lists. Without this, template class constructor
argument types were emitted without typename qualification, causing
dependent name lookup failures.
Removed dead inline type-expansion code in argnames_template that
computed a namespace variable ns.t which was never used — the
actual default emission already called arg_type_with_template_params.
macros.j2 — trampoline class deduplication:
Pure virtual method deduplication compared by short name (m.name),
so overloaded virtuals like Values(Vec&, Real&, Vec&) and
Values(Vec&, Real&, Vec&, Mat&) were treated as duplicates. Changed
to use m.full_name (full signature). Also added the missing
methods.append call for private virtuals, which allowed parent-class
loops to re-emit methods already overridden. 20 classes affected.
CMakeLists.j2:
Bump cmake_minimum_required to 3.17. Enable C language (needed when
VTK links against MPI::MPI_C). Add find_package(MPI QUIET) so cmake
can resolve MPI::MPI_C and MPI::MPI_CXX targets on platforms where
VTK is built with MPI support (resolves CadQuery/OCP#179). Add
find_package(fmt) and link fmt::fmt — FreeBSD's OCCT port links
against libfmt externally, while conda builds bundle it statically
so this is a no-op there. Add status messages for Python lib path
and VTK version to aid CI debugging.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Building OCP 7.8.1.2 on Arch Linux against vtk 9.5.0 currently fails in the CMake Configuration stage:
This patch adds
mpito the VTK components to search for, resolving this issue.