[cmake-developers] FindBacktrace.cmake

[Alexander Neundorf]

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 "

Thanks
Alex