search

BioMedIA / irtk-refactored-deprecated

Apache License 2.0
8 stars 7 forks source link

Order of include_directories in CMake files #70

Closed kevin-keraudren closed 8 years ago

kevin-keraudren commented 8 years ago

The order in which include_directories appear in CMake files matters as we do not want previously installed versions of IRTK to interfere with the current build.

The proposed fix was obtained with the following perl one-liner:

perl -pi -e 's/include_directories\(\"/include_directories\(BEFORE \"/g'  `find Modules -name CMakeLists.txt`

This issue occured while compiling in an Anaconda environment and is highlighted below:

In file included from ~/anaconda/conda-bld/work/Modules/Common/include/irtkMath.h:21:
~/anaconda/include/cutil_math.h:26:10: fatal error: 'cuda_runtime.h' file not found
#include <cuda_runtime.h>
         ^
1 error generated.

Note that the old ~/anaconda/include/cutil_math.h is picked up instead of the development version at ~/anaconda/conda-bld/work/Modules/Common/include/cutil_math.h

ghisvail commented 8 years ago

A few questions first:

kevin-keraudren commented 8 years ago

Hi @ghisvail , I made my deductions from the output of:

make VERBOSE=1

See before the proposed fix:

[ 0%] Building CXX object Modules/Common/src/CMakeFiles/Common.dir/irtkBSpline.cc.o cd
/Users/kevin/anaconda/conda-bld/work/build/Modules/Common/src &&
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
-DHAS_EIGEN -DHAS_NIFTI -DHAS_PNG -DHAS_TBB -DHAS_VTK -DHAS_ZLIB -DHAVE_EIGEN -DHAVE_NIFTI
-DHAVE_PNG -DHAVE_TBB -DHAVE_VTK -DHAVE_ZLIB -DNO_BOUNDS -stdlib=libc++ -std=c++11 -O3 -DNDEBUG
-I/Users/kevin/anaconda/include -I/Users/kevin/anaconda/envs/_build/include
-I/Users/kevin/anaconda/envs/_build/include/vtk-6.3 -I/usr/local/include/eigen3
-I/usr/local/include -I/Users/kevin/anaconda/envs/py34/include
-I/Users/kevin/anaconda/conda-bld/work/Modules/Common/include
-I/Users/kevin/anaconda/conda-bld/work/build/Modules/Common/include -o
CMakeFiles/Common.dir/irtkBSpline.cc.o -c
/Users/kevin/anaconda/conda-bld/work/Modules/Common/src/irtkBSpline.cc In file included from
/Users/kevin/anaconda/conda-bld/work/Modules/Common/src/irtkBSpline.cc:17: In file included from
/Users/kevin/anaconda/conda-bld/work/Modules/Common/include/irtkBSpline.h:20: In file included from
/Users/kevin/anaconda/conda-bld/work/Modules/Common/include/irtkCommon.h:41: In file included from
/Users/kevin/anaconda/conda-bld/work/Modules/Common/include/irtkMath.h:21:
/Users/kevin/anaconda/include/cutil_math.h:26:10: fatal error: 'cuda_runtime.h' file not found
#include <cuda_runtime.h> ^ 1 error generated. make[2]: ***
[Modules/Common/src/CMakeFiles/Common.dir/irtkBSpline.cc.o] Error 1 make[1]: ***
[Modules/Common/src/CMakeFiles/Common.dir/all] Error 2 make: *** [all] Error 2 Command failed:
/bin/bash -x -e /Users/kevin/Work/github/conda-recipes/irtk/build.sh

and after:

[ 0%] Building CXX object Modules/Common/src/CMakeFiles/Common.dir/irtkBSpline.cc.o cd
/Users/kevin/anaconda/conda-bld/work/build/Modules/Common/src &&
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DHAS_EIGEN -DHAS_NIFTI -DHAS_PNG
-DHAS_TBB -DHAS_VTK -DHAS_ZLIB -DHAVE_EIGEN -DHAVE_NIFTI -DHAVE_PNG -DHAVE_TBB -DHAVE_VTK -DHAVE_ZLIB -DNO_BOUNDS -stdlib=libc++
-std=c++11 -O3 -DNDEBUG -I/Users/kevin/anaconda/conda-bld/work/build/Modules/Common/include
-I/Users/kevin/anaconda/conda-bld/work/Modules/Common/include -I/Users/kevin/anaconda/include
-I/Users/kevin/anaconda/envs/_build/include -I/Users/kevin/anaconda/envs/_build/include/vtk-6.3 -I/usr/local/include/eigen3
-I/usr/local/include -I/Users/kevin/anaconda/envs/py34/include -o CMakeFiles/Common.dir/irtkBSpline.cc.o -c
/Users/kevin/anaconda/conda-bld/work/Modules/Common/src/irtkBSpline.cc

Please note that -I/Users/kevin/anaconda/conda-bld/work/build/Modules/Common/include used to come last and it now comes first.

The issue here is unrelated to CUDA. I had different error messages before that complained about IRTK headers, which I could get easily rid off with:

rm ~/anaconda/include/irtk*

but there is no way I also start running commands like:

rm ~/anaconda/include/cuda*

What you mention regarding CUDA should be addressed in a separate pull request, and I do not know what the plans are regarding the work @bkainz did on irtkCuda. I guess there are plans to include it in IRTK as you cite his work in the list of authors: https://github.com/BioMedIA/IRTK/blob/master/AUTHORS.txt#L19

I believe this fix is of interest to anyone who plans to compile IRTK and type make install more than once in the lifetime of their package manager. A typical case would be compiling IRTK-LEGACY, installing it then trying to compile the new IRTK in the same environment where the old one is installed. Then you will end up with the same conflicts between headers.

Lastly, this issue is not new to me as I already adressed it for the old IRTK in this commit: https://gitlab.doc.ic.ac.uk/sk1712/irtk/commit/b3dd8e6c32d508ca4424c1cfb25e86960c2e752e I believe the use of the keyword BEFORE is a cleaner and safer solution than moving around the include_directories.

ghisvail commented 8 years ago

Thanks for clarifying @kevin-keraudren. My comments below:

I believe this fix is of interest to anyone who plans to compile IRTK and type make install more than once in the lifetime of their package manager.

So, this issue only happens if you have an existing installation of IRTK or IRTK-legacy under a system include directory such as /usr/include or /usr/local/include? Am I right ? What is your current setup actually?

Just trying to figure out what would make this reproducible on my machine. I have an install on $HOME/local and I don't get that problem when compiling another instance of IRTK for example.

kevin-keraudren commented 8 years ago

The short answer is: if none of the IRTK dependencies are installed in the same place as IRTK, then the problem will not occur.

The long answer is:

Imagine you are using Anaconda: it is a package manager that can install any prepackaged stuff into an environment with ${ANACONDA_HOME}/lib, ${ANACONDA_HOME}/include and ${ANACONDA_HOME}/bin. Anaconda is particularly aimed at Python, and very handy to install non-Python dependencies of Python packages. For instance, python-irtk expects IRTK to be already installed.

Anaconda tends to put everything in the same place, as if you had CMAKE_PEFIX=${ANACONDA_HOME}, so for instance:

include_directories(${PNG_INCLUDE_DIRS})

will add a

-I${ANACONDA_HOME}/include

to your compilation command (cf ls ~/anaconda/include/png.h).

My old install of IRTK headers also landed in ${ANACONDA_HOME}/include hence the origin of the conflict.

Because line 37 (https://github.com/BioMedIA/IRTK/blob/master/CMakeLists.txt#L37) comes before the lines 51 (https://github.com/BioMedIA/IRTK/blob/master/CMakeLists.txt#L51) and 62 (https://github.com/BioMedIA/IRTK/blob/master/CMakeLists.txt#L62), then the include_directories come in the wrong order.

However, I would not recommend moving line 31 after the lines 51 and 62 as it might break the definition of all the build options (such as include this_directory only if HAS_VTK), hence the choice of the BEFORE solution.

If you were to install IRTK-LEGACY into /usr/include, then the problem will occur as soon as eigen, png, tbb or any of the dependencies is also installed there.

ghisvail commented 8 years ago

My old install of IRTK headers also landed in ${ANACONDA_HOME}/include hence the origin of the conflict.

So if you remove them, this issue goes away doesn't it?

kevin-keraudren commented 8 years ago

The problem is resolved by a clean uninstall of IRTK-LEGACY. As the new IRTK installs headers in include/irtk there should not be conflicts unless other libraries have header files with the same name. Apart from cuda_math.h, this is highly unlikely, so we can close this issue.