Skip to content

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

Open
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix/wrapping-castxml-genex-cmake-3.22-backport
Open

COMP: Backport wrapping castxml genexes to CMake 3.22.1 (issue #6191)#6193
hjmjohnson wants to merge 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix/wrapping-castxml-genex-cmake-3.22-backport

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Restores Python wrapping on CMake 3.22.1–3.26.x by rewriting three $<LIST:JOIN,$<LIST:TRANSFORM,…>> genexes (CMake 3.27+) in Wrapping/macro_files/itk_auto_load_submodules.cmake using $<JOIN:list,glue> (CMake 3.0+) plus a $<$<BOOL:property>:…> empty-list guard. Fixes the floor mismatch reported in #6191.

Why this is needed

PR #5842 (commit 7a59c6da41, 2026-02-26) silently raised the effective CMake floor for ITK Python wrapping to 3.27 by introducing $<LIST:JOIN,…> and $<LIST:TRANSFORM,…> generator expressions. The project-wide cmake_minimum_required(VERSION 3.22.1) was not updated. On CMake 3.22.1..3.26.x these unrecognized genexes silently expand to empty at file(GENERATE) time, producing castxml.inc files with no -I / -isystem / -D flags. Symptom: castxml fails to find module headers when wrapping any module that depends on ITK module includes.

list(JOIN …) and list(TRANSFORM …) commands have existed since CMake 3.12 (2018), but the generator-expression variants $<LIST:JOIN> / $<LIST:TRANSFORM> were first released in CMake 3.27 (2023-07). PR #5842 needs the genex form because target-property values can themselves contain generator expressions ($<BUILD_INTERFACE:…>, $<INSTALL_INTERFACE:…>), so configure-time list commands cannot substitute.

How the backport works

The standard pre-3.27 trick: $<JOIN:list,glue> carries the per-element wrapping inside the glue string. To wrap each element of [a;b;c] in "-Ix":

"-I$<JOIN:list,"\n"-I>"

evaluates to "-Ia"\n"-Ib"\n"-Ic". The first element's prefix and the last element's suffix sit outside the JOIN. An outer $<$<BOOL:property>:…> guard suppresses the whole expression when the property is empty so we don't emit a stray "-I" (which would become an empty include directory passed to castxml).

Three call sites updated for INTERFACE_INCLUDE_DIRECTORIES, INTERFACE_SYSTEM_INCLUDE_DIRECTORIES, and INTERFACE_COMPILE_DEFINITIONS.

Verification — byte-identical output to the 3.27+ form

Standalone file(GENERATE) round-trip test using the original 3.27+ form and the backport form side-by-side, on (a) a target with three include dirs and two compile definitions and (b) an empty target:

$ diff -u build/original.txt build/backport.txt
$ echo $?
0

diff reports no differences. The backport is a behavior-preserving rewrite, not a semantic change.

Reverting once the CMake floor is bumped to 3.27+

The commit message documents the mechanical revert. When ITK's cmake_minimum_required is raised to 3.27 or newer, this backport should be reverted in favor of the more expressive $<LIST:TRANSFORM> / $<LIST:JOIN> form because:

  • The 3.27+ form makes the per-element transform explicit (REPLACE regex on each element).
  • LIST:TRANSFORM supports APPEND / PREPEND / TOLOWER / TOUPPER / STRIP / GENEX_STRIP, none of which have a JOIN-glue equivalent — so future evolution of this code is easier in the modern form.
  • Empty-property handling is automatic in LIST:JOIN (no $<BOOL:> guard needed).
  • The "shove the wrapping into the glue string" trick is correct but harder to read than a regex transform.

The block comment in the file marks the backport region for revert.

@github-actions github-actions Bot added the type:Compiler Compiler support or related warnings label May 2, 2026
@dzenanz dzenanz requested a review from blowekamp May 2, 2026 13:43
@hjmjohnson hjmjohnson self-assigned this May 2, 2026
Comment thread Wrapping/macro_files/itk_auto_load_submodules.cmake Outdated
@blowekamp
Copy link
Copy Markdown
Member

blowekamp commented May 2, 2026

This generator expression is cleaner than the current implementation too, so if it works it is a keeper. This cmake code was challenging to get it working.

P.S. IMHO the commit message is too log. The AI is being too verbose in many situations.

@hjmjohnson hjmjohnson force-pushed the fix/wrapping-castxml-genex-cmake-3.22-backport branch from 5bf71a0 to ae21bdb Compare May 2, 2026 14:30
…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).
@hjmjohnson hjmjohnson force-pushed the fix/wrapping-castxml-genex-cmake-3.22-backport branch from ae21bdb to c465015 Compare May 2, 2026 14:34
@hjmjohnson
Copy link
Copy Markdown
Member Author

Thanks @blowekamp. The commit message has been tightened from 65 → 17 body lines (now: subject + root-cause paragraph + what-it-does paragraph + a one-line note marking the revert trigger). Inline comment also rewritten to describe only what the code is.

@hjmjohnson hjmjohnson requested a review from blowekamp May 2, 2026 14:36
@hjmjohnson hjmjohnson marked this pull request as ready for review May 2, 2026 14:36
@greptile-apps

This comment was marked as resolved.

@hjmjohnson hjmjohnson added this to the ITK 6.0.0 milestone May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants