search

InsightSoftwareConsortium / ITKModuleTemplate

A template to start an ITK Module
https://itk.org/ITKSoftwareGuide/html/Book1/ITKSoftwareGuide-Book1ch9.html#x50-1430009
Apache License 2.0
12 stars 16 forks source link

ENH: Use itkOverrideGetNameOfClassMacro as per ITK PR 4373 #155

Closed dzenanz closed 2 weeks ago

dzenanz commented 5 months ago

See related ITK PRs: https://github.com/InsightSoftwareConsortium/ITK/pull/4373 https://github.com/InsightSoftwareConsortium/ITK/pull/4378

We should probably wait until ITK 5.4 final is tagged, before merging this.

dzenanz commented 5 months ago

@jhlegarreta @n-dekker @thewtex

dzenanz commented 5 months ago

As-is, this breaks the python builds with GitHub action for remote modules:

[9/17] Generating /work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/itkANTSRegistration.xml
FAILED: /work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/itkANTSRegistration.xml 
cd /work/_skbuild/linux-x86_64-3.8/cmake-build/Wrapping/Modules/ANTsWasm && /work/_skbuild/linux-x86_64-3.8/cmake-build/Wrapping/Generators/CastXML/castxml/bin/castxml -o /work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/itkANTSRegistration.xml --castxml-gccxml --castxml-start _wrapping_ --castxml-cc-gnu "(" /opt/rh/gcc-toolset-11/root/bin//g++ -std=c++14 ")" -w -c @/work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/.castxml.inc /work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/itkANTSRegistration.cxx
In file included from /work/ITK-cp38-cp38-manylinux_2_28_x64/Wrapping/castxml_inputs/itkANTSRegistration.cxx:16:
/work/include/itkANTSRegistration.h:66:3: error: unknown type name 'itkOverrideGetNameOfClassMacro'
  itkOverrideGetNameOfClassMacro(ANTSRegistration);
  ^
1 error generated.
ninja: build stopped: subcommand failed.
jhlegarreta commented 5 months ago

I assume we would get a better sense of what's going on after the ITK commit hashes are updated in the CI config file: https://github.com/InsightSoftwareConsortium/ITKModuleTemplate/blob/main/.github/workflows/build-test-package.yml#L6-L8

So:

We should probably wait until ITK 5.4 final is tagged, before merging this.

Looks the way forward to me.

jhlegarreta commented 1 month ago

Rebased on main. Tthe Win 2022 failure reported on main is unrelated: https://github.com/InsightSoftwareConsortium/ITKModuleTemplate/actions/runs/9455350750 https://github.com/InsightSoftwareConsortium/ITKModuleTemplate/actions/runs/9455350750/job/26044951161#step:13:71

So this can be safely merged once CIs come green.

Edit: Unless this is still a problem: https://github.com/InsightSoftwareConsortium/ITKModuleTemplate/pull/155#issuecomment-1910857435