COMP: CMake, add INCLUDE_DIRS for swig to work by phcerdan · Pull Request #5 · InsightSoftwareConsortium/ITKTotalVariation

phcerdan · 2019-03-27T22:02:28Z

WIP: Fix TotalVariation_INCLUDE_DIRS

Add itk-module-init.cmake
WIP: Remove debug CMake messages

SWIG is not reading the TotalVariationTargets.cmake file, where proxTV
library is set, with include dirs, interface libraries and compile
definitions.
This creates erorrs, the first sign is missing the INCLUDE_DIRSs.
(TVopt.h not found).
This can be solved hacking the INCLUDE_DIRS of the ITK Module TotalVariation_INCLUDE_DIRS.

Once that's fixed, the next error is missing mex.h file, this is because
the compile definition NO_MATLAB is not applied.

In Eigen3 I faced the same problems, but because it is header only,
fixing INCLUDE_DIRS and adding the compile definitions in the
Eigen3-ITK Header (itkEigen3.h) was enough.

In this case, the compile definitions of proxTV (NO_MATLAB, USE_LAPACK=0)
are ignored.

Also re-arranges to avoid duplication

Add itk-module-init.cmake WIP: Remove debug CMake messages SWIG is not reading the TotalVariationTargets.cmake file, where proxTV library is set, with include dirs, interface libraries and compile definitions. This creates erorrs, the first sign is missing the INCLUDE_DIRSs. (TVopt.h not found). This can be solved hacking the INCLUDE_DIRS of the ITK Module TotalVariation_INCLUDE_DIRS. Once that's fixed, the next error is missing mex.h file, this is because the compile definition NO_MATLAB is not applied. In Eigen3 I faced the same problems, but because it is header only, fixing INCLUDE_DIRS and adding the compile definitions in the Eigen3-ITK Header (itkEigen3.h) was enough. In this case, the compile definitions of proxTV (NO_MATLAB, USE_LAPACK=0) are ignored.

phcerdan · 2019-03-29T14:38:22Z

Related: InsightSoftwareConsortium/ITK#650

phcerdan · 2019-03-29T14:59:29Z

Example error:

[1/8] Generating ../../../../../../ITK-cp37-cp37m-manylinux1_x64/Wrapping/itkProxTVImageFilter.xml
FAILED: ../../../ITK-cp37-cp37m-manylinux1_x64/Wrapping/itkProxTVImageFilter.xml
cd /work/_skbuild/linux-x86_64-3.7/cmake-build/Wrapping/Modules/TotalVariation && /work/_skbuild/linux-x86_64-3.7/cmake-build/Wrapping/Generators/CastXML/castxml/bin/castxml -o /work/ITK-cp37-cp37m-manylinux1_x64/Wrapping/itkProxTVImageFilter.xml --castxml-gccxml --target=x86_64-linux-gnu --castxml-start _wrapping_ --castxml-cc-gnu "(" /opt/rh/devtoolset-2/root/usr/bin/g++ -Wno-array-bounds -w -Wno-array-bounds -std=c++11 ")" -w -c @/work/ITK-cp37-cp37m-manylinux1_x64/Wrapping/TotalVariation.castxml.inc /work/ITK-cp37-cp37m-manylinux1_x64/Wrapping/itkProxTVImageFilter.cxx
In file included from /work/ITK-cp37-cp37m-manylinux1_x64/Wrapping/itkProxTVImageFilter.cxx:16:
In file included from /work/include/itkProxTVImageFilter.h:22:
In file included from /work/_skbuild/linux-x86_64-3.7/cmake-build/_deps/proxtv_fetch-build/src/include/TVopt.h:15:
In file included from /work/_skbuild/linux-x86_64-3.7/cmake-build/_deps/proxtv_fetch-build/src/include/general.h:22:
/work/_skbuild/linux-x86_64-3.7/cmake-build/_deps/proxtv_fetch-build/src/include/lapackFunctionsWrap.h:16:10: fatal error: 'mex.h' file not found
#include "mex.h"

thewtex

Looks great!

phcerdan · 2019-04-01T13:52:17Z

Merging but the problem where castxml does not read the ModuleTargets.cmake is still there.

thewtex · 2019-04-02T02:01:54Z

I will take a look... 👀

bpaniagua · 2019-07-09T09:27:07Z

Hi @phcerdan @thewtex

I am trying to compile this external package and I am experiencing this error:

In file included from /work/Dev/ITK/ITK-build/_deps/proxtv_fetch-build/src/include/general.h:22: /work/Dev/ITK/ITK-build/_deps/proxtv_fetch-build/src/include/lapackFunctionsWrap.h:16:10: fatal error: 'mex.h' file not found #include "mex.h" ^~~~~~~ 1 error generated. make[2]: *** [Wrapping/itkProxTVImageFilter.xml] Error 1 make[1]: *** [Wrapping/Modules/TotalVariation/CMakeFiles/TotalVariationCastXML.dir/all] Error 2 make: *** [all] Error 2

I see that the most recent commit of the TV sources that get pulled at /path-to-ITKbuild/_deps/proxtv_fetch-src is from April 11th

commit 187f868861f53b74fd00eeb40eb7ba83c2b7809d (HEAD -> use_eigen, origin/use_eigen)
Author: Pablo Hernandez-Cerdan pablo.hernandez.cerdan@outlook.com
Date: Thu Apr 11 14:59:10 2019 -0400

COMP: Remove ifdef DEBUG for pragma omp

When I look for these changes by typing git log --all --grep='swig to work' I dont have any commits coming up.

Has this commit been fully integrated into ITK?

Thank you! 🙏 ☺️

phcerdan · 2019-07-09T09:39:06Z

Hi @bpaniagua,
What version of ITK are you using? Does it have this PR Add COMPILE_DEFINITIONS to castxml?

bpaniagua · 2019-07-09T09:43:18Z

Yes, it looks like ITK does (If i grep the log in my ITKsources - not the tv sources - I can see that commit)
Maybe I am not doing things right?

bpaniagua · 2019-07-09T09:43:44Z

ITK version:
commit e7e78524e4ef9e86d828c7a053fdecdd3b603d33 (HEAD -> master, origin/python-builds, origin/nightly-master, origin/master, origin/HEAD)
Merge: c8fde5fe20 0c7d03f86a
Author: Dženan Zukić dzenan.zukic@kitware.com
Date: Mon Jun 24 09:32:41 2019 -0400

Merge branch 'release'

phcerdan · 2019-07-09T10:02:54Z

yeah, I can reproduce it, it seems that the compile definition is not added to castxml.
A hacky workaround to keep you going meanwhile:
In your ITK build folder, find the file: TotalVariation.castxml.inc
and append a new line with -DNOMATLAB

The compile definition is added here: https://github.com/InsightSoftwareConsortium/ITKTotalVariation/blob/master/CMakeLists.txt#L26 . But castxml is ignoring it, even though compile definitions are added there.

I will have a look.

bpaniagua · 2019-07-09T10:37:03Z

The hacky solution works, @phcerdan Thank you so so much!
Please, let me know if can I help to integrate a patch into ITK.

itk_wrap_module_castxml now gets the COMPILE_DEFINITIONS of the module top directory, instead of the wrapping directory. The old behavior wasn't respecting the add_compile_definitions set in the CMakeLists of the module. Triggered by: InsightSoftwareConsortium/ITKTotalVariation#5 This is a follow on from: e3a908b

phcerdan · 2019-07-09T12:44:30Z

@bpaniagua I created a PR in ITK to fix this 1079, not sure why the add_compile_definitions set in the top CMakeLists.txt of the modules are not propagated to the wrapping subdirectory.

That PR just gets the COMPILE_DEFINITIONS of the parent module directory (instead of the wrapping folder).

It would be great if you are able to give it a test!

itk_wrap_module_castxml now gets the COMPILE_DEFINITIONS of the module top directory, instead of the wrapping directory. The old behavior wasn't respecting the add_compile_definitions set in the CMakeLists of the module. Triggered by: InsightSoftwareConsortium/ITKTotalVariation#5 This is a follow on from: e3a908b

itk_wrap_module_castxml now gets the COMPILE_DEFINITIONS of the module top directory, instead of the wrapping directory. The old behavior wasn't respecting the add_compile_definitions set in the CMakeLists of the module. Triggered by: InsightSoftwareConsortium/ITKTotalVariation#5 This is a follow on from: 8c922bb

itk_wrap_module_castxml now gets the COMPILE_DEFINITIONS of the module top directory, instead of the wrapping directory. The old behavior wasn't respecting the add_compile_definitions set in the CMakeLists of the module. Triggered by: InsightSoftwareConsortium/ITKTotalVariation#5 This is a follow on from: 5d2f0ed

COMP: CMake, add INCLUDE_DIRS for swig to work

9a4a689

Also re-arranges to avoid duplication

phcerdan force-pushed the fix_cmake_for_swig branch from 7e8a942 to 9a4a689 Compare March 27, 2019 22:06

phcerdan requested a review from thewtex March 29, 2019 14:37

thewtex approved these changes Mar 29, 2019

View reviewed changes

phcerdan merged commit 3a1f99e into InsightSoftwareConsortium:master Apr 1, 2019

phcerdan mentioned this pull request Jul 9, 2019

BUG: Fix COMPILE_DEFINITIONS of castxml InsightSoftwareConsortium/ITK#1079

Merged

phcerdan deleted the fix_cmake_for_swig branch July 9, 2019 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

COMP: CMake, add INCLUDE_DIRS for swig to work#5

COMP: CMake, add INCLUDE_DIRS for swig to work#5
phcerdan merged 2 commits into
InsightSoftwareConsortium:masterfrom
phcerdan:fix_cmake_for_swig

phcerdan commented Mar 27, 2019 •

edited

Loading

Uh oh!

phcerdan commented Mar 29, 2019

Uh oh!

phcerdan commented Mar 29, 2019

Uh oh!

thewtex left a comment

Uh oh!

phcerdan commented Apr 1, 2019

Uh oh!

thewtex commented Apr 2, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019 •

edited

Loading

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

phcerdan commented Mar 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phcerdan commented Mar 29, 2019

Uh oh!

phcerdan commented Mar 29, 2019

Uh oh!

thewtex left a comment

Choose a reason for hiding this comment

Uh oh!

phcerdan commented Apr 1, 2019

Uh oh!

thewtex commented Apr 2, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpaniagua commented Jul 9, 2019

Uh oh!

phcerdan commented Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phcerdan commented Mar 27, 2019 •

edited

Loading

phcerdan commented Jul 9, 2019 •

edited

Loading

phcerdan commented Jul 9, 2019 •

edited

Loading