[cmake-developers] FindBacktrace.cmake

Brad King brad.king at kitware.com
Mon Jul 8 09:55:03 EDT 2013


On 7/3/2013 5:10 PM, Vadim Zhukov wrote:
> 2. Just make sure BACKTRACE_LIBRARIES is empty: if the developer wants
> to shoot himself in the foot with -nostdlib, then he's responsible

Yes, I agree with this approach.  However,

 cmake_push_check_state()

will not erase any existing CMAKE_REQUIRED_* settings so your
check could report success due to the context.  Inside the
push/pop pair shouldn't you clear the values except for the
one you need?

>> Otherwise, the module looks like a great start!
> 
> Thank you for review. I'm sending updated module now. Aside tweaks
> based on your input, I realized that there should be a way for the
> user to specify BACKTRACE_HEADER, too. I hope that I did not
> overcomplicate things.

That looks fine to me.

A few minor style comments:

* Please use 2 spaces for indentation instead of hard TABs
* Please remove trailing whitespace

Please follow steps 5 and 6 of these instructions:

 http://www.cmake.org/Wiki/CMake:Module_Maintainers#New_Maintainer

to get access to the repo.  The SetupForDevelopment.sh
step in the Git instructions will install local hooks for
commits that will catch things like trailing whitespace.
There are equivalent checks when pushing to the server so
it is easier to fix them up front rather than rebasing later.

Thanks,
-Brad



More information about the cmake-developers mailing list