Hi Vadim, On Sunday 28 July 2013, you wrote: > 2013/7/23 Alexander Neundorf <[email protected]>: > > 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 -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
