[cmake-developers] FindBacktrace.cmake

Vadim Zhukov persgray at gmail.com
Mon Jul 8 17:51:21 EDT 2013


2013/7/8 Brad King <brad.king at kitware.com>:
> 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?

I'm not sure whether resetting CMAKE_REQUIRED_* is the desired
behavior. The example in the CMakePushCheckState.cmake documentation
tells the opposite, and I think that appending values have a point by
adding some sort of a flexibility for writers of other CMake project
files and other CMake modules. Also, I see the same logic (append
instead of rewrite) is used in other CMake modules (in KDE at least,
where CMakePushCheckState.cmake did came from).

And if CMAKE_REQUIRED_* should be generally reset within module, then
I propose adding a third macro in CMakePushCheckState.cmake, say,
cmake_reset_check_state(), which will do this.

I'm adding Alexander Neundorf to the thread to make things more clear,
as he developed CMakePushCheckState.cmake module. Alexander, could
you, please, explain the way CMakePushCheckState macros should be
used?

>>> 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

Fixed in my tree, thanks.

> 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.

On the way, send access request.

--
  WBR,
  Vadim Zhukov



More information about the cmake-developers mailing list