Re: [cmake-developers] FindBacktrace.cmake is slightly too chatty
Am 19.12.2013 16:28, schrieb Brad King: On 12/11/2013 02:17 AM, Vadim Zhukov wrote: It took a bit more thinking than I thought initially, sorrt. Anyway, see the last update to find_backtrace topic. Worked fine for me. I've rebased the new commit and merged the topic to 'next' for testing: FindBacktrace: Search and report only when not already found http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9650c09b Eike, does this work as you expect now? Yes, works fine. Thx! Eike -- 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
Re: [cmake-developers] FindBacktrace.cmake is slightly too chatty
2013/12/6 Rolf Eike Beer e...@sf-mail.de: Hi, I noticed that the message backtrace facility detected in default set of libraries appears every time CMake is run, and not just when the library is searched for, which is not what the modules usually do. Can this be changed? It took a bit more thinking than I thought initially, sorrt. Anyway, see the last update to find_backtrace topic. Worked fine for me. -- 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
Re: [cmake-developers] FindBacktrace.cmake is slightly too chatty
06.12.2013 1:19 пользователь Rolf Eike Beer e...@sf-mail.de написал: Hi, I noticed that the message backtrace facility detected in default set of libraries appears every time CMake is run, and not just when the library is searched for, which is not what the modules usually do. Can this be changed? Good point, I'll prepare a fix for it ASAP. -- 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
Re: [cmake-developers] FindBacktrace.cmake
On 11/20/2013 04:07 PM, Vadim Zhukov wrote: I've pushed the version which uses cmake_push_check_state(RESET). Great, thanks. It is now in master! -Brad -- 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
Re: [cmake-developers] FindBacktrace.cmake
2013/11/19 Vadim Zhukov persg...@gmail.com: 2013/11/19 Brad King brad.k...@kitware.com: On 07/31/2013 10:06 AM, Brad King wrote: The dependency is now in master so please rebase find_backtrace on that so you can use the reset feature. The find_backtrace topic has not yet been merged to 'next' for testing. After the documentation transition I rebased and revised the topic once to use the new documentation system but otherwise did not change it. Has this topic been updated to use the reset feature of CMakePushCheckState? Please check/revise the current topic on the stage and merge to 'next' for testing or remove it if you no longer wish to contribute this module. Sorry for slacking. I've mishandled the topic repo on my side with erroneous git rebase, then was forced to do other things and kept it unupdated for a long time. I'll revise and update the topic today or tomorrow. Thank you for reminding and sorry again. I've pushed the version which uses cmake_push_check_state(RESET). -- 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
Re: [cmake-developers] FindBacktrace.cmake
On 07/31/2013 10:06 AM, Brad King wrote: The dependency is now in master so please rebase find_backtrace on that so you can use the reset feature. The find_backtrace topic has not yet been merged to 'next' for testing. After the documentation transition I rebased and revised the topic once to use the new documentation system but otherwise did not change it. Has this topic been updated to use the reset feature of CMakePushCheckState? Please check/revise the current topic on the stage and merge to 'next' for testing or remove it if you no longer wish to contribute this module. Thanks, -Brad -- 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
Re: [cmake-developers] FindBacktrace.cmake
2013/11/19 Brad King brad.k...@kitware.com: On 07/31/2013 10:06 AM, Brad King wrote: The dependency is now in master so please rebase find_backtrace on that so you can use the reset feature. The find_backtrace topic has not yet been merged to 'next' for testing. After the documentation transition I rebased and revised the topic once to use the new documentation system but otherwise did not change it. Has this topic been updated to use the reset feature of CMakePushCheckState? Please check/revise the current topic on the stage and merge to 'next' for testing or remove it if you no longer wish to contribute this module. Sorry for slacking. I've mishandled the topic repo on my side with erroneous git rebase, then was forced to do other things and kept it unupdated for a long time. I'll revise and update the topic today or tomorrow. Thank you for reminding and sorry again. -- 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
Re: [cmake-developers] FindBacktrace.cmake
On 07/29/2013 12:38 PM, Vadim Zhukov wrote: 2013/7/29 Brad King brad.k...@kitware.com: Let's complete the add-cmake_reset_check_state topic first and then you can rebase the find_backtrace topic on it to use the feature. The former looks good but we'll let it run through the dashboards tonight first. Thank you for looking in. I'll follow this plan then. The dependency is now in master so please rebase find_backtrace on that so you can use the reset feature. Thanks, -Brad -- 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
Re: [cmake-developers] FindBacktrace.cmake
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
Re: [cmake-developers] FindBacktrace.cmake
2013/7/28 Rolf Eike Beer e...@sf-mail.de: On Sun Jul 28 15:46:27 2013 Vadim Zhukov persg...@gmail.com wrote: With all input from you and after setting up local development repo, I created and pushed two branches to stage: If I understand correctly, there should be zero fallout in the night build, and after that, given there will be no blockers, I should merge them into the next branch, right? If you don't merge them to next they will get no coverage as next is what the nightlies build. Oh. Thank you, I've messed up the info from Wiki in my mind. -- 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
Re: [cmake-developers] FindBacktrace.cmake
On 07/29/2013 04:59 AM, Vadim Zhukov wrote:
Thank you, Alexander. I've fixed both points you've mentioned and
merged the add-cmake_reset_check_state branch into the next.
Let's complete the add-cmake_reset_check_state topic first and
then you can rebase the find_backtrace topic on it to use the
feature. The former looks good but we'll let it run through the
dashboards tonight first.
One comment on the latter topic:
+ # Prepend list with library path as it's more common practice
+ set(_Backtrace_STD_ARGS Backtrace_LIBRARIES ${_Backtrace_STD_ARGS})
The FPHSA macro should be given Backtrace_LIBRARY since that is
what the user might modify, no?
Thanks,
-Brad
--
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
Re: [cmake-developers] FindBacktrace.cmake
2013/7/29 Brad King brad.k...@kitware.com:
On 07/29/2013 04:59 AM, Vadim Zhukov wrote:
Thank you, Alexander. I've fixed both points you've mentioned and
merged the add-cmake_reset_check_state branch into the next.
Let's complete the add-cmake_reset_check_state topic first and
then you can rebase the find_backtrace topic on it to use the
feature. The former looks good but we'll let it run through the
dashboards tonight first.
Thank you for looking in. I'll follow this plan then.
One comment on the latter topic:
+ # Prepend list with library path as it's more common practice
+ set(_Backtrace_STD_ARGS Backtrace_LIBRARIES ${_Backtrace_STD_ARGS})
The FPHSA macro should be given Backtrace_LIBRARY since that is
what the user might modify, no?
Yep, you're right. I've just pushed the fix to stage.
--
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
Re: [cmake-developers] FindBacktrace.cmake
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 2. find_backtrace, adding Modules/FindBacktrace.cmake: http://cmake.org/gitweb?p=stage/cmake.git;a=shortlog;h=refs/heads/find_backtrace Are they fine? I made them independent on each other, to make CMakePushCheckState-related conversion as a separate thing later. If I understand correctly, there should be zero fallout in the night build, and after that, given there will be no blockers, I should merge them into the next branch, right? -- 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
Re: [cmake-developers] FindBacktrace.cmake
On Sun Jul 28 15:46:27 2013 Vadim Zhukov persg...@gmail.com wrote: With all input from you and after setting up local development repo, I created and pushed two branches to stage: If I understand correctly, there should be zero fallout in the night build, and after that, given there will be no blockers, I should merge them into the next branch, right? If you don't merge them to next they will get no coverage as next is what the nightlies build. Eike -- 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
Re: [cmake-developers] FindBacktrace.cmake
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 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
Re: [cmake-developers] FindBacktrace.cmake
On Wednesday 03 July 2013, Vadim Zhukov wrote:
2013/7/3 Brad King brad.k...@kitware.com:
On 7/3/2013 11:09 AM, Vadim Zhukov wrote:
I'm an OpenBSD developer, working mostly on porting software. Most of
my work is related to CMake-based land, including modern KDE and some
other Qt4-based stuff.
Great, thanks for coming to us.
Here is a module I constructed a few months ago. It's helpful for
programs that want to use GNU-style backtrace(3) routine, which could
be found in different libraries and be declared in different headers
across OSes outta there.
Thanks for working on this. It looks similar to FindThreads in that
the results could be builtin to the system libraries or in one of
several other vendor-specific libraries. Don't model the module after
FindThreads though as that is one of the oldest modules and does not
fully use modern conventions.
The BACKTRACE_INCLUDE_DIR and BACKTRACE_LIBRARY variables should
not be documented as results that consuming projects should use
directly. They are inputs to the module that end users edit in
the cache. The results go in BACKTRACE_INCLUDE_DIRS and
BACKTRACE_LIBRARIES. Look at how FindProtobuf.cmake separates
the list of variables in its documentation, for example.
Thank you, that was exactly info I was looking for!
The line
set(BACKTRACE_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
looks a bit strange to me. The CMAKE_REQUIRED_LIBRARIES variable
is an input to the check_* APIs and should not be depended upon
or used by a find module. When calling check_symbol_exists you
should set CMAKE_REQUIRED_* to known values not depending on the
calling context. In the case that some non-standard/non-system
library is needed to provide the symbol it should be set by the
user through the BACKTRACE_LIBRARY code path.
Yes, I was not sure if this should be done this way either. Other variants
are:
1. find_library(c) - not as stupid as it looks from the first time,
because C++ compiler could be called with -nostdlib, for example (and
this was the reason for inclusion of CMAKE_REQUIRED_LIBRARIES
initially);
2. Just make sure BACKTRACE_LIBRARIES is empty: if the developer wants
to shoot himself in the foot with -nostdlib, then he's responsible for
the all consequences; and if he wants to avoid -lc, well, he'll
successfully avoid it.
I'd go with either way. For this try I choosed (2).
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.
A few more style-comments:
please use empty else() and endif() clauses. Most of the cmake modules have
been converted to this style recently.
The variables should not use BACKTRACE as prefix, but Backtrace, so they
match the name of the find-module exactly.
Why do you make BACKTRACE_HEADER INTERNAL ? I think INTERNAL variables are
completely hidden, i.e. not accessible, e.g. in cmake-gui.
You use message() without the STATUS keyword. I think it should be used.
Can you switch to the new-style FPHSA() ?
This would be
find_package_handle_standard_args(Backtrace
REQUIRED_VARS ${_BACKTRACE_STD_ARGS} )
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
Re: [cmake-developers] FindBacktrace.cmake
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. 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
Re: [cmake-developers] FindBacktrace.cmake
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 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. -Brad -- 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
Re: [cmake-developers] FindBacktrace.cmake
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? 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 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. Thanks, -Brad -- 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
Re: [cmake-developers] FindBacktrace.cmake
2013/7/8 Brad King brad.k...@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 -- 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
Re: [cmake-developers] FindBacktrace.cmake
On 7/3/2013 11:09 AM, Vadim Zhukov wrote:
I'm an OpenBSD developer, working mostly on porting software. Most of
my work is related to CMake-based land, including modern KDE and some
other Qt4-based stuff.
Great, thanks for coming to us.
Here is a module I constructed a few months ago. It's helpful for
programs that want to use GNU-style backtrace(3) routine, which could
be found in different libraries and be declared in different headers
across OSes outta there.
Thanks for working on this. It looks similar to FindThreads in that
the results could be builtin to the system libraries or in one of
several other vendor-specific libraries. Don't model the module after
FindThreads though as that is one of the oldest modules and does not
fully use modern conventions.
The BACKTRACE_INCLUDE_DIR and BACKTRACE_LIBRARY variables should
not be documented as results that consuming projects should use
directly. They are inputs to the module that end users edit in
the cache. The results go in BACKTRACE_INCLUDE_DIRS and
BACKTRACE_LIBRARIES. Look at how FindProtobuf.cmake separates
the list of variables in its documentation, for example.
The line
set(BACKTRACE_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
looks a bit strange to me. The CMAKE_REQUIRED_LIBRARIES variable
is an input to the check_* APIs and should not be depended upon
or used by a find module. When calling check_symbol_exists you
should set CMAKE_REQUIRED_* to known values not depending on the
calling context. In the case that some non-standard/non-system
library is needed to provide the symbol it should be set by the
user through the BACKTRACE_LIBRARY code path.
Otherwise, the module looks like a great start!
Thanks,
-Brad
--
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
Re: [cmake-developers] FindBacktrace.cmake
2013/7/3 Brad King brad.k...@kitware.com:
On 7/3/2013 11:09 AM, Vadim Zhukov wrote:
I'm an OpenBSD developer, working mostly on porting software. Most of
my work is related to CMake-based land, including modern KDE and some
other Qt4-based stuff.
Great, thanks for coming to us.
Here is a module I constructed a few months ago. It's helpful for
programs that want to use GNU-style backtrace(3) routine, which could
be found in different libraries and be declared in different headers
across OSes outta there.
Thanks for working on this. It looks similar to FindThreads in that
the results could be builtin to the system libraries or in one of
several other vendor-specific libraries. Don't model the module after
FindThreads though as that is one of the oldest modules and does not
fully use modern conventions.
The BACKTRACE_INCLUDE_DIR and BACKTRACE_LIBRARY variables should
not be documented as results that consuming projects should use
directly. They are inputs to the module that end users edit in
the cache. The results go in BACKTRACE_INCLUDE_DIRS and
BACKTRACE_LIBRARIES. Look at how FindProtobuf.cmake separates
the list of variables in its documentation, for example.
Thank you, that was exactly info I was looking for!
The line
set(BACKTRACE_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
looks a bit strange to me. The CMAKE_REQUIRED_LIBRARIES variable
is an input to the check_* APIs and should not be depended upon
or used by a find module. When calling check_symbol_exists you
should set CMAKE_REQUIRED_* to known values not depending on the
calling context. In the case that some non-standard/non-system
library is needed to provide the symbol it should be set by the
user through the BACKTRACE_LIBRARY code path.
Yes, I was not sure if this should be done this way either. Other variants are:
1. find_library(c) - not as stupid as it looks from the first time,
because C++ compiler could be called with -nostdlib, for example (and
this was the reason for inclusion of CMAKE_REQUIRED_LIBRARIES
initially);
2. Just make sure BACKTRACE_LIBRARIES is empty: if the developer wants
to shoot himself in the foot with -nostdlib, then he's responsible for
the all consequences; and if he wants to avoid -lc, well, he'll
successfully avoid it.
I'd go with either way. For this try I choosed (2).
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.
Checked with the Clementine on OpenBSD-CURRENT.
--
WBR,
Vadim Zhukov
FindBacktrace.cmake
Description: Binary data
--
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
