[cmake-developers] FindBacktrace.cmake

[Vadim Zhukov]

2013/7/29 Alexander Neundorf <neundorf at kde.org>:
> Hi Vadim,
>
> On Sunday 28 July 2013, you wrote:
>> 2013/7/23 Alexander Neundorf <neundorf at kde.org>:
>> > On Tuesday 09 July 2013, Brad King wrote:
>> >> On 07/08/2013 05:51 PM, Vadim Zhukov wrote:
>> >> > 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?
>> >>
>> >> I would like Alex's opinion on this, but we must either use
>> >> CMAKE_REQUIRED_* to report the results (as you did originally)
>> >> or not use the caller's CMAKE_REQUIRED_* to perform the test.
>> >> Otherwise the results will not be consistent.
>> >>
>> >> We have never (intentionally) defined CMAKE_REQUIRED_* as the
>> >> input to a _find_ module before, only to _check_ macros.  The
>> >> check state stack module helps a project handle its own check
>> >> accumulation
>> >
>> > Yes, that was the intention.
>> > Adding a cmake_reset_check_state() sounds good.
>> >
>> >> and AFAIK is not meant as an API between a project
>> >> and CMake find modules.
>> >>
>> >> It looks like a few find modules already use check macros and
>> >> do not pay any attention to whether CMAKE_REQUIRED_* is defined.
>> >> These may also need to be cleaned up based on the results of this
>> >> discussion.
>> >
>> > I found it only in FindGIF.cmake right now. There the reset() could be
>> > used then.
>>
>> Brad and Alexander,
>>
>> With all input from you and after setting up local development repo, I
>> created and pushed two branches to stage:
>>
>> 1. add-cmake_reset_check_state, adding CMAKE_RESET_CHECK_STATE() macro
>> and optional resetting functionality in CMAKE_PUSH_CHECK_STATE():
>> http://cmake.org/gitweb?p=stage/cmake.git;a=shortlog;h=refs/heads/add-cmake
>> _reset_check_state
>
>
> Looks good, but two comments:
>
> The documentation does not mention the new optional RESET argument of
> cmake_push_check_state(). It looks like there is something missing.
>
> "These macros can be used to save, restore and reset the state "
> maybe to make clear that "reset" here means setting empty:
> "These macros can be used to save, restore and reset (i.e. clear) the state "

Thank you, Alexander. I've fixed both points you've mentioned and
merged the "add-cmake_reset_check_state" branch into the "next".

--
  WBR,
  Vadim Zhukov