search

strasdat / Sophus

C++ implementation of Lie Groups using Eigen.
Other
2.07k stars 599 forks source link

CMakeLists.txt: various fixes #535

Closed DasRoteSkelett closed 5 months ago

DasRoteSkelett commented 7 months ago

Bumped minimal version of cmake to 3.5, because versions below 3.5 will be removed soon from cmake. Removed manual setting of optimization flags. These are already set to decent defaults, and it should be possible to set them from the outside if need be.

Also made an option SOPHUS_INSANE_CFLAGS which defaults to off. The issue is, that (more) modern compilers will not compile the tests / code with -Werror on. Werror is therefore harmful to production, albeit useful for developers. Hence, it is an option to be set.

strasdat commented 5 months ago

@DasRoteSkelett Thanks for your PR!

I did merge larger PR (#538) today which bumps cmake to 3.16.

the issue is, that (more) modern compilers will not compile the tests / code with -Werror on.

Hm, I'm a fan of -Werror and rather have those issues fixed or specific problematic warnings disabled (globally or in the corresponding file using https://stackoverflow.com/a/48426846). #538 moves to more modern compiler toolchain and hopefully fixes the issues you saw.

I'm closing your PR for now, but feel free to reopen an updated PR if you still see issues.

DasRoteSkelett commented 5 months ago

Hi @strasdat ,

I see your point from the side of a Developer. Please bear in mind, that there are also so called users.

Regards

strasdat commented 5 months ago

@DasRoteSkelett I do see your point, and I'm also reading up on the topic e.g.

https://discourse.cmake.org/t/how-do-i-replace-the-compiler-options-for-one-target/7815

My main objective is to use Werror etc in CI. I think I like to come up with a clean solution which does not set any of those CMAKE_<LANG>_FLAGS inside the CMakeLists.txt.

Possibly cmake presets are the way to go:

https://cmake.org/cmake/help/latest/manual/cmake-presets.7.html

Thanks for pointing out that issue...