search

jhu-cisst / cisst

JHU ERC CISST Library
http://github.com/jhu-cisst/cisst/wiki
Other
64 stars 47 forks source link

cisst build fails on feature-cxx-compatibility branch #55

Closed vincent-hui closed 6 years ago

vincent-hui commented 6 years ago

I tried to build the latest cisst on feature-cxx-compatibility branch with catkin on Ubuntu 16.04 with gcc 5.4 catkin config --cmake-args -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=14 but cisst build fails. The error mesage is `src/cisst-saw/cisst/cisstCommon/cmnPortability.h:395:33: error: ‘isnan’ was not declared in this scope

define CMN_ISNAN(x) isnan(x)`

because CISST_USE_STD_ISNAN, CISST_USE_STD_ISFINITE and CISST_USE_CMATH are not defined. The definitions of CISST_USE_STD_ISNAN, CISST_USE_STD_ISFINITE and CISST_USE_CMATH are removed by this commit

In order to build dvrk-ros with C++ 14 standard on Ubuntu 16.04 with gcc 5.4, we need to

  1. include instead of
  2. use std::isnan instead of isnan
  3. use std::isfinite instead of isfinite
adeguet1 commented 6 years ago

I definitively didn't claim this branch was ready. I'll let you know when it is ready for testing. There are some parts of the library that are likely not needed so I'd rather remove them than add multiple conditional compilation depending on OS and compiler.

adeguet1 commented 6 years ago

I think I've ported most of your changes now. The main differences are:

@vincent-hui , can you give this a try? I tested on 17.04 with gcc 6.3.0 and clang 4.0.0 with and without the CMAKE_CXX_STANDARD 14. In all cases it found std::isnan and std::isfinite in . When you try on your settings, can you also turn ON the cisst CMake setting CISST_BUILD_TESTS and once everything is build, launch cisstCommonTests -r as well as cisstVectorTests -r?

vincent-hui commented 6 years ago

I tried again, I can build cisst with and without the CMAKE_CXX_STANDARD 14 on Ubuntu 16.04 with gcc 5.4. I turned on CISST_BUILD_TESTS and run cisstCommonTests -r as well as cisstVectorTests -r. All tests are passed with and without the CMAKE_CXX_STANDARD 14.

vincent@vincent-VirtualBox:~$ cisstCommonTests -r
.........................................
Cisst operating system code: 2
Cisst operating system string: Linux
Cisst compiler code: 1
Cisst compiler string: gcc
CMake system name: Linux
CMake c++ compiler: c++
CMake generator: Unix Makefiles
.................

OK (58 tests)

vincent@vincent-VirtualBox:~$ cisstVectorTests -r
...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

OK (907 tests)

However I encountered a build issue after CISST_BUILD_TESTS is turned on with catkin

catkin config --cmake-args -DCMAKE_BUILD_TYPE=Release -DCISST_BUILD_TESTS=1

I need to build cisst by running catkin build twice. the first build fails always

src/cisst-saw/cisst/cisstCommon/cmnGenericObject.h:40:23: fatal error: json/json.h: No such file or directory compilation terminated.

I need to run catkin build again after the first build fails, cisst can be built sucessfully after running catkin build twice.

adeguet1 commented 6 years ago

I will test this code asap on Windows with different compilers and see if it passes the unit tests for cisstCommon and cisstVector. If it does, I'll merge to the devel branch.

As for the need to build twice, this is a known issue. Our CMakeLists.txt predate catkin build tools. One thing we do is to look for cisstConfig.cmake in sub directories even though this a single CMake project. It made sense when we developed our code but it doesn't work with catkin. Catkin will only copy the generated files at the end of the build, so the cisstConfig.cmake file can't be found until the first build is finished. The proper fix would be to move all the test programs in a different package (e.g. cisst_tests) that would depend on cisst but this would need some major file re-organization. In any case, this is not a regression so this won't stop me from merging this branch.

adeguet1 commented 6 years ago

I tested on Windows 7 with VS 2013 and was able to run cisstCommon and cisstVector unit tests. Merged to devel branch.