Refactor wrapping to use target properties and address bug building remote modules against an ITK build directory by blowekamp · Pull Request #5842 · InsightSoftwareConsortium/ITK

blowekamp · 2026-02-26T22:04:32Z

Use generator expression to convert target property into properly formatted string for include file. Change to file with GENERATE option to support generator expressions.

There were redundant calls to configure the inc file for each wrapping file. An additional prefix is added to the inc file to create multiple.

This addresses a bug where remote modules could not be wrapped against a build ITK due to missing include paths and definitions.

COMPATIBILITY NOTE: Removes support for directory level properties for includes and defines.

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)
Added test (or behavior not changed)
Updated API documentation (or API not changed)
Added license to new files (if any)
Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

blowekamp · 2026-02-26T22:06:32Z

Sorry for short message, issue mostly redundant include with ITK_INCLUDE_DIRS, and target's transitive properties. Needed to keep ITK_INCLUDE_DIRS because of itkMeshRegion.h in Core/Common.

The extra inc files are redundant.

blowekamp · 2026-02-27T16:51:32Z

@@ -1,3 +1,4 @@
+set(ITK_MODULE_ITKCommon_WRAPPING_DEPENDS ITK::ITKMeshModule)


@thewtex There needs to be a way to express that the wrapping for common depends on the Mesh module, or the mesh code needs to be moved to a better place. This follows the convention of (internal?) variable used in ModuleMacro... I wasn't sure if other models had un-described dependencies for wrapping, but it appears not. I am still pondering better ways to specify this for wrapping; I am open to suggestions?

Yes, I don't recall the history or the reason for this. Maybe we can ask an agent to try to simplify the dependencies?

I have asked my local agent many questions regarding the wrapping code it figure things out. I ended up just hard coding the dep in the TypedefMacros.cmake file. Perhaps this can be revisited after this is merged and remotes are working again for wrapping.

Apparently, it has been there since pre-modularization before 2011, and it was trivial to relocate.

dzenanz

Looks good on a glance.

blowekamp · 2026-02-27T21:00:20Z

+      set(
+        CONFIG_CASTXML_INC_CONTENTS
+        "${CONFIG_CASTXML_INC_CONTENTS}$<LIST:JOIN,$<LIST:TRANSFORM,$<TARGET_PROPERTY:${_depend},INTERFACE_COMPILE_DEFINITIONS>,REPLACE,^(.+)$,\"-D\\1\">,\n>\n"
+      )


@zivy @bradking @thewtex This is some pretty cool code going on with CMake here.

thewtex · 2026-02-27T21:58:33Z

Cool! 😎

I recall that we had code to remove redundant include directories -- hopefully this helps with that, too.

Moves MeshRegion wrapping from ITKCommon to ITKMesh module and removes the workaround dependency linkage in TypedefMacros.cmake. Related to PR InsightSoftwareConsortium#5842

Changes VTK Bridge module dependency from PRIVATE_DEPENDS to DEPENDS for ITKCommon, making it a public dependency as required. Related to PR InsightSoftwareConsortium#5842

blowekamp · 2026-03-03T21:05:38Z

/azp.run ITK.macOS.Python

blowekamp · 2026-03-04T13:12:04Z

I don't think any of the current test failures and build issue are related to this PR.

dzenanz

I tested this locally on Windows + VS2026. MCI built successfully against the build dir.

blowekamp · 2026-03-04T19:33:33Z

@thewtex All the builds came back green, shall we merged this change with the wrapping infrastructure?

hjmjohnson · 2026-03-05T00:41:05Z

@dzenanz CDash has been hanging. Do you know if there is a problem with the server?

dzenanz · 2026-03-05T00:47:24Z

I am not aware of any problems. Maybe this CDash GHA check got broken somehow?

thewtex

@blowekamp awesome!!

Yes, let's merge. Could you please change the base to release?

I need to re-build the Python packages with the Windows patch, and I will include this one, too. Are there others that we need to include?

blowekamp · 2026-03-05T11:34:20Z

@blowekamp awesome!!

Yes, let's merge. Could you please change the base to release?

I need to re-build the Python packages with the Windows patch, and I will include this one, too. Are there others that we need to include?

There are 3 other PR's that are link to this one that did minor fixes needed #5850 #5851 #5852. So this make the rebase onto release a little more difficult.

ADDITION: I think the best course of action would the to rebase this onto release. Then merge into release even though the branch is missing those components listed. This will allow testing in main. Then you can cherry pick the listed PRs, and merge this brach into release? Or just do another tag, but there still seems to be a bit of churn with other issues. Please let me know how to proceed.

Use generator expression to convert target property into properly formatted string for include file. Change to file with GENERATE option to support generator expressions. There were redundant calls to configure the inc file for each wrapping file. An additional prefix is added to the inc file to create multiple.

Compile definitions and include directories are no longer obtained from the directory. This is replaced by usage of target level properties. Retain support for WRAPPER_LIBRARY_INCLUDE_DIRECTORIES and WRAPPER_LIBRARY_LINK_LIBRARIES to control behavior.

For modules that use Python.h, explicitly include Python's path for cast xml.

thewtex · 2026-03-05T17:52:33Z

@blowekamp ok, thanks. I will cherry-pick all four patches on release. The ITK tag will stay the same, but the ITKPythonPackage will use the patched release. We will use this updated release hash for the Remote Module CI. You did a great job addressing most issues. I expect people will find a few more :-). The tag will be available, and we can suggest using main for the most up-to-date version.

Moves MeshRegion wrapping from ITKCommon to ITKMesh module and removes the workaround dependency linkage in TypedefMacros.cmake. Related to PR #5842

Changes VTK Bridge module dependency from PRIVATE_DEPENDS to DEPENDS for ITKCommon, making it a public dependency as required. Related to PR #5842

thewtex · 2026-03-06T20:17:14Z

Backported to release.

…tSoftwareConsortium#6191) PR InsightSoftwareConsortium#5842 introduced $<LIST:JOIN,$<LIST:TRANSFORM,…>> generator expressions in itk_auto_load_submodules.cmake. Those operators landed in CMake 3.27 (2023-07) but ITK's cmake_minimum_required is 3.22.1; on older CMake they silently expand to empty at file(GENERATE) time, producing castxml.inc files with no -I/-isystem/-D flags and breaking Python wrapping. Rewrite the three affected sites with $<JOIN:list,glue> (CMake 3.0+), carrying the per-element wrapping in the glue string and guarded by $<$<BOOL:property>:…> so an empty property emits nothing. Generated castxml.inc bytes are byte-identical to the 3.27+ form for both populated and empty target properties. Revert when ITK's CMake floor reaches 3.27 (block comment marks the lines).

blowekamp requested a review from thewtex February 26, 2026 22:06

blowekamp force-pushed the fix_wrap_external branch from a643386 to 7722e15 Compare February 27, 2026 16:44

github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class area:Core Issues affecting the Core module area:IO Issues affecting the IO module labels Feb 27, 2026

blowekamp commented Feb 27, 2026

View reviewed changes

dzenanz reviewed Feb 27, 2026

View reviewed changes

blowekamp force-pushed the fix_wrap_external branch from 7722e15 to 12250d9 Compare February 27, 2026 20:57

github-actions Bot removed the area:Python wrapping Python bindings for a class label Feb 27, 2026

blowekamp commented Feb 27, 2026

View reviewed changes

github-actions Bot added area:Python wrapping Python bindings for a class area:Bridge Issues affecting the Bridge module labels Feb 27, 2026

blowekamp mentioned this pull request Mar 2, 2026

COMP: VTKImageIO publicly depends on ImageIOBase #5850

Merged

7 tasks

blowekamp mentioned this pull request Mar 2, 2026

ENH: Move MeshRegion wrapping to Mesh module #5851

Merged

blowekamp mentioned this pull request Mar 2, 2026

BUG: VTK Bridge publicly depends on ITKCommon #5852

Merged

blowekamp force-pushed the fix_wrap_external branch from 846da46 to 9176640 Compare March 2, 2026 21:14

github-actions Bot removed type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Bridge Issues affecting the Bridge module area:IO Issues affecting the IO module labels Mar 2, 2026

blowekamp mentioned this pull request Mar 3, 2026

Windows Python CMake configuration fixes #5854

Merged

blowekamp force-pushed the fix_wrap_external branch from 9176640 to 786e51d Compare March 3, 2026 15:53

github-actions Bot added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Mar 3, 2026

dzenanz reviewed Mar 3, 2026

View reviewed changes

Comment thread Wrapping/TypedefMacros.cmake Outdated

Comment thread Wrapping/TypedefMacros.cmake Outdated

blowekamp force-pushed the fix_wrap_external branch from 786e51d to 6d469c8 Compare March 3, 2026 16:54

blowekamp force-pushed the fix_wrap_external branch from 6d469c8 to 12d1e58 Compare March 3, 2026 21:35

github-actions Bot added the area:Bridge Issues affecting the Bridge module label Mar 3, 2026

blowekamp marked this pull request as ready for review March 3, 2026 21:36

dzenanz approved these changes Mar 4, 2026

View reviewed changes

blowekamp force-pushed the fix_wrap_external branch from 12d1e58 to 9ecba99 Compare March 4, 2026 15:28

blowekamp changed the title ~~Address bug building remote modules against an ITK build directory~~ Refactor wrapping to use target properties and address bug building remote modules against an ITK build directory Mar 4, 2026

blowekamp requested a review from hjmjohnson March 4, 2026 19:32

thewtex approved these changes Mar 5, 2026

View reviewed changes

blowekamp added 3 commits March 5, 2026 16:28

BUG: Include python paths some wrapped modules

d63f5df

For modules that use Python.h, explicitly include Python's path for cast xml.

blowekamp force-pushed the fix_wrap_external branch from 9ecba99 to d63f5df Compare March 5, 2026 16:28

blowekamp merged commit a322c2d into InsightSoftwareConsortium:main Mar 5, 2026
2 of 4 checks passed

blowekamp mentioned this pull request Mar 5, 2026

COMP: Remove extraneous linking for VtkGlue Python #5861

Merged

7 tasks

thewtex pushed a commit that referenced this pull request Mar 6, 2026

ENH: Move MeshRegion wrapping to Mesh module

632b297

Moves MeshRegion wrapping from ITKCommon to ITKMesh module and removes the workaround dependency linkage in TypedefMacros.cmake. Related to PR #5842

thewtex pushed a commit that referenced this pull request Mar 6, 2026

BUG: VTK Bridge publicly depends on ITKCommon

05a6613

Changes VTK Bridge module dependency from PRIVATE_DEPENDS to DEPENDS for ITKCommon, making it a public dependency as required. Related to PR #5842

blowekamp mentioned this pull request Mar 12, 2026

Refactor third-party Eigen3 CMake code #5831

Merged

7 tasks

This was referenced May 2, 2026

Wrapping requires CMake ≥ 3.27 ($<LIST:JOIN>) but project declares minimum 3.22.1 #6191

Open

COMP: Backport wrapping castxml genexes to CMake 3.22.1 (issue #6191) #6193

Open

		@@ -1,3 +1,4 @@
		set(ITK_MODULE_ITKCommon_WRAPPING_DEPENDS ITK::ITKMeshModule)

Uh oh!

Conversation

blowekamp commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

blowekamp commented Feb 26, 2026

Uh oh!

blowekamp Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thewtex Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

blowekamp Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

blowekamp Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

blowekamp Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thewtex commented Feb 27, 2026

Uh oh!

Uh oh!

Uh oh!

blowekamp commented Mar 3, 2026

Uh oh!

blowekamp commented Mar 4, 2026

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

blowekamp commented Mar 4, 2026

Uh oh!

hjmjohnson commented Mar 5, 2026

Uh oh!

dzenanz commented Mar 5, 2026

Uh oh!

thewtex left a comment

Choose a reason for hiding this comment

Uh oh!

blowekamp commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

thewtex commented Mar 5, 2026

Uh oh!

thewtex commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

blowekamp commented Feb 26, 2026 •

edited

Loading

blowekamp Feb 27, 2026 •

edited

Loading

blowekamp Feb 27, 2026 •

edited

Loading

blowekamp commented Mar 5, 2026 •

edited

Loading