2013/7/29 Alexander Neundorf <neund...@kde.org>: > Hi Vadim, > > On Sunday 28 July 2013, you wrote: >> 2013/7/23 Alexander Neundorf <neund...@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 -- 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