Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 03:40 PM, Nils Gladitz wrote: I think I've got it rewritten properly Yes, it looks good! Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/10/2014 03:42 PM, Nils Gladitz wrote: extract everything into its own class cmConditionEvaluator and have it keep everything that was being passed around as members instead. Good work. I think the net change is now in good shape. To make it easier to review now and bisect in the future, please rewrite the topic to start with refactoring cmIfCommand to split out the cmConditionEvaluator, and then add the rest of the changes. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 11.09.2014 15:04, Brad King wrote: Good work. I think the net change is now in good shape. To make it easier to review now and bisect in the future, please rewrite the topic to start with refactoring cmIfCommand to split out the cmConditionEvaluator, and then add the rest of the changes. Thanks. Is there a trick to recreate the history in that order or would I have to start from scratch? Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 09:20 AM, Nils Gladitz wrote:
Is there a trick to recreate the history in that order or
would I have to start from scratch?
First rewrite the branch to squash your updates back into
the first commit, leaving all my CMP0054 warning commits
later in the topic. Then start a new branch from the
squashed commit containing only your part of the changes.
Note the sha1 of this commit, say a01b2c3. Then edit it
to *remove* the changes besides the refactoring and amend
the commit. The result should be just one commit with the
refactoring parts.
Then you can use the git commit-tree plumbing to create
a commit with the tree object of your original change but
set its parent as the edited commit. This will manufacture
a commit that makes the changes on top of the refactoring:
commit=$(git commit-tree a01b2c3^{tree} -p HEAD)
git merge $commit
git commit --amend -C a01b2c3
Finally, rebase the rest of the topic back onto that.
The tip of the resulting topic should look identical to
what is on the stage now.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 11.09.2014 15:28, Brad King wrote: First rewrite the branch to squash your updates back into the first commit, leaving all my CMP0054 warning commits later in the topic. Then start a new branch from the squashed commit containing only your part of the changes. Note the sha1 of this commit, say a01b2c3. Then edit it to *remove* the changes besides the refactoring Thanks for the walk through. I am a bit stuck on removing the CMP0054 changes from cmConditionEvaluator.cxx part given that file is new and didn't exist without the changes. I'll redo the refactoring part manually from scratch if I can't figure out something better. Might take me a bit; I haven't had much experience with those git aspects yet. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 11:35 AM, Nils Gladitz wrote: Thanks for the walk through. I am a bit stuck on removing the CMP0054 changes from cmConditionEvaluator.cxx part given that file is new and didn't exist without the changes. The file should exist, just remove the CMP0054 parts from it and amend the commit. I'll redo the refactoring part manually from scratch if I can't figure out something better. Might take me a bit; I haven't had much experience with those git aspects yet. If it takes too much time, don't bother doing this part unless you are really interested in using it as a learning opportunity. Let me know either way, please. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 11.09.2014 17:39, Brad King wrote: On 09/11/2014 11:35 AM, Nils Gladitz wrote: Thanks for the walk through. I am a bit stuck on removing the CMP0054 changes from cmConditionEvaluator.cxx part given that file is new and didn't exist without the changes. The file should exist, just remove the CMP0054 parts from it and amend the commit. I might be missing something obvious. cmConditionEvaluator.cxx doesn't exist without the CMP0054 changes because I created it after I did most of the CMP0054 changes. Since most of the changes are replacements rather than plain additions *removing* those changes would mean having to restore those code parts which however used to be in cmIf.cxx rather than cmConditionEvaluator.cxx. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 11:52 AM, Nils Gladitz wrote:
cmConditionEvaluator.cxx doesn't exist without the CMP0054 changes
because I created it after I did most of the CMP0054 changes.
Since most of the changes are replacements rather than plain additions
*removing* those changes would mean having to restore those code parts
which however used to be in cmIf.cxx rather than cmConditionEvaluator.cxx.
I think you missed the part about squashing your changes together
into the first commit of the topic. I just rewrote the topic
to do that. Now you can start working from commit 5922fc2c:
git checkout -b tmp 5922fc2c
git rm Help/policy/CMP0054.rst Help/release/dev/if-sanity.rst
git checkout HEAD~1 -- Help
git rm -rf Tests/RunCMake/CMP0054
git checkout HEAD~1 -- Tests/RunCMake/CMakeLists.txt
git commit --amend
Then keep editing the files and amending the commit to leave
behind only the refactoring pieces. Amend the commit message
accordingly too. Then do:
commit=$(git commit-tree 5922fc2c^{tree} -p HEAD)
git merge $commit
git commit --amend -C 5922fc2c
to restore the rest of the commit's changes. Then rebase
the rest of the topic to get the warning cleanups.
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 11.09.2014 18:53, Brad King wrote: On 09/11/2014 11:52 AM, Nils Gladitz wrote: cmConditionEvaluator.cxx doesn't exist without the CMP0054 changes because I created it after I did most of the CMP0054 changes. Since most of the changes are replacements rather than plain additions *removing* those changes would mean having to restore those code parts which however used to be in cmIf.cxx rather than cmConditionEvaluator.cxx. I think you missed the part about squashing your changes together into the first commit of the topic. I just rewrote the topic to do that. Now you can start working from commit 5922fc2c: No I did that as well. git checkout -b tmp 5922fc2c git rm Help/policy/CMP0054.rst Help/release/dev/if-sanity.rst git checkout HEAD~1 -- Help git rm -rf Tests/RunCMake/CMP0054 git checkout HEAD~1 -- Tests/RunCMake/CMakeLists.txt git commit --amend I think I did all that too. Then keep editing the files and amending the commit to leave behind only the refactoring pieces. That is the part I was stuck at. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 12:57 PM, Nils Gladitz wrote: Then keep editing the files and amending the commit to leave behind only the refactoring pieces. That is the part I was stuck at. At this point cmConditionEvaluator.cxx exists in the source tree. What is the problem? -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 11.09.2014 18:59, Brad King wrote: At this point cmConditionEvaluator.cxx exists in the source tree. What is the problem? I can not create a CMP0054 free version of cmConditionEvaluator.cxx by simply removing content from the file (or the patch). I would have to add back lines to cmConditionEvaluator.cxx which where removed while they were still in cmIfCommand.cxx. The problem is that I am not sure how to do that properly. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/11/2014 01:07 PM, Nils Gladitz wrote: I would have to add back lines to cmConditionEvaluator.cxx which where removed while they were still in cmIfCommand.cxx. Look at the diff in commit 5922fc2c and you will see all those lines as removed from cmIfCommand. You can put them all in cmConditionEvaluator. Some of the work is manual. Effectively you are re-doing the refactoring from scratch without the CMP0054 pieces. The use of Git up to this point is just to help get close. The reason I'm asking for this is that the refactoring done to create cmConditionEvaluator is more intrusive than the original CMP0054 change. It will be much easier to convince myself that the whole thing is correct if I can see the refactoring done first and independently. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
I think I've got it rewritten properly but I didn't know what half the git commands I ran did most of the time so I am not entirely secure with the result. Would have probably not figured this out without your help. Thanks again! Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/08/2014 11:35 AM, Nils Gladitz wrote: I updated the topic, squished and merged. Thanks. I've updated the topic to include all the actual fixes for CMP0054 warnings within CMake's own code that I've found so far. I have a few more changes to request that you add to the topic: * The wording of the warning says what the change in behavior is, but does not inform the user that the old behavior is being used for compatibility. Currently without reading the policy documentation one might think from the messages that the logic is being executed differently than before. * In cmIfCommand we should avoid doing the lookups when the policy is NEW. Instead of calling mf-GetPolicyStatus on every step of the way, call it once at the beginning of the if() command execution and save the result in a member. Then in cmIfCommand::GetDefinitionIfUnquoted, check for quoting and the policy status before even trying to do the lookup. That should make processing of quoted arguments much faster because we won't have to check for a definition with every string value. * The CMP0054-keywords-NEW test does not cover ( and ). (Good catch on implementing those, BTW.) * The tests need to cover if() inside while() and foreach() as well as macro() and function(). Also add nested if() for completeness. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/10/2014 05:33 PM, Brad King wrote: Thanks. I've updated the topic to include all the actual fixes for CMP0054 warnings within CMake's own code that I've found so far. Thanks! I have a few more changes to request that you add to the topic: * The wording of the warning says what the change in behavior is, but does not inform the user that the old behavior is being used for compatibility. Currently without reading the policy documentation one might think from the messages that the logic is being executed differently than before. Done. * In cmIfCommand we should avoid doing the lookups when the policy is NEW. Instead of calling mf-GetPolicyStatus on every step of the way, call it once at the beginning of the if() command execution and save the result in a member. Then in cmIfCommand::GetDefinitionIfUnquoted, check for quoting and the policy status before even trying to do the lookup. That should make processing of quoted arguments much faster because we won't have to check for a definition with every string value. The functions for condition evaluation were global or static (I assume so they could be shared with while()) and I would have had to pass through the policy state as extra parameters. This was also done with CMP0012 but I figured I might just as well extract everything into its own class cmConditionEvaluator and have it keep everything that was being passed around as members instead. * The CMP0054-keywords-NEW test does not cover ( and ). (Good catch on implementing those, BTW.) * The tests need to cover if() inside while() and foreach() as well as macro() and function(). Also add nested if() for completeness. I added tests but specifically for the loops I am not entirely sure what to cover given that the context of recording and replaying is basically the same (unlike with functions/macros)? I wouldn't mind some extra scrutiny to make sure they cover what you had in mind. Thanks. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 09:19 AM, Nils Gladitz wrote:
On 09/05/2014 02:50 PM, Brad King wrote:
On 09/04/2014 11:58 AM, Nils Gladitz wrote:
- The dashboard submissions that bootstrap got many CMP0054
warnings. Most of them are the same warning repeated due
to presence in a macro or loop. Please update the warning
to not warn on the same line more than once. A set of
already-emitted warning lines can be kept somewhere.
I'll look into it.
Good work on the revisions.
In the warning message:
Quoted variables like 'SOME_VAR' are no longer dereferenced.
I think the 'single quoting' may be confusing since it looks
like an example of the quoting that is no longer dereferenced.
Instead use double-quotes:
Quoted variables like SOME_VAR are no longer dereferenced.
I tested this on a few projects and found that the warning is
repeated in cases like:
macro(foo VAR)
if(${VAR} MATCHES ^${VAR}$)
endmacro()
because each call has a different quoted variable name after
macro expansion. Since if() does not even get the arguments
until after the macro execution has substituted for ${VAR},
it is hard to eliminate this unless we take out the variable
name from the dedup set key. I think perhaps we should do
that even though it could suppress cases with multiple separate
problems in a single if() command. The goal of dedup is to
reduce the number of warnings that show up. Once a developer
sits down to resolve the warnings, s/he will repeat running
CMake until all the warnings are gone anyway.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/08/2014 04:44 PM, Brad King wrote: Good work on the revisions. Thanks. I updated the topic, squished and merged. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 11:58 AM, Nils Gladitz wrote:
Thanks, that actually sounds doable.
I'll give it a try.
Good work on the topic so far. I have a few more thoughts:
- The dashboard submissions that bootstrap got many CMP0054
warnings. Most of them are the same warning repeated due
to presence in a macro or loop. Please update the warning
to not warn on the same line more than once. A set of
already-emitted warning lines can be kept somewhere.
- I think we can generalize the policy a bit further. Consider:
set(x EXISTS)
if(${x} STREQUAL EXISTS)
Currently that will get an error because the evaluated
variable spells EXISTS which is treated as a keyword.
The policy could also make if() honor such keywords only
when they are unquoted.
- The new cmExpandedCommandArgument class interface could
be simplified by making it derive from std::string and
then just add the boolean as an extra member.
- In the documentation we need to be precise about when
the expansion is still allowed. The cmake-language(7)
manual defines unquoted, quoted, and [[bracket]]
arguments as distinct argument types. We should use the
same terminology here. You can even link back to the
manual with cross-reference syntax:
:ref:`Bracket Argument`
:ref:`Quoted Argument`
:ref:`Unquoted Argument`
Also please extend the test cases for this policy to cover:
- All if() command conditions that do variable-or-string.
- if() inside a macro() and a function(), covering each
combination of policy settings when the macro/function
is recorded and when it is invoked. The policy value
should be that when the macro/function was recorded.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 02:50 PM, Brad King wrote:
On 09/04/2014 11:58 AM, Nils Gladitz wrote:
- The dashboard submissions that bootstrap got many CMP0054
warnings. Most of them are the same warning repeated due
to presence in a macro or loop. Please update the warning
to not warn on the same line more than once. A set of
already-emitted warning lines can be kept somewhere.
I'll look into it.
Would it be ok to set the policy for cmcurl to OLD?
I assume that makes it easier to pull changes from upstream than when I
fix the code to comply with the new policy; also it isn't always clear
what the intention was with the given conditionals and I am more likely
to break something.
- I think we can generalize the policy a bit further. Consider:
set(x EXISTS)
if(${x} STREQUAL EXISTS)
Currently that will get an error because the evaluated
variable spells EXISTS which is treated as a keyword.
The policy could also make if() honor such keywords only
when they are unquoted.
I'll look into it.
- The new cmExpandedCommandArgument class interface could
be simplified by making it derive from std::string and
then just add the boolean as an extra member.
I think composition is cleaner and more future proof in this case
though; I'd like to keep it unless you insist it should be changed.
- In the documentation we need to be precise about when
the expansion is still allowed. The cmake-language(7)
manual defines unquoted, quoted, and [[bracket]]
arguments as distinct argument types. We should use the
same terminology here. You can even link back to the
manual with cross-reference syntax:
:ref:`Bracket Argument`
:ref:`Quoted Argument`
:ref:`Unquoted Argument`
I kept the distinction in cmExpandedCommandArgument at first but then
generalized it so that bracketed, quoted and C++ side 0, 1 constant
arguments would be subsumed. This could be expanded again if there are
future use cases where they need to be distinguished.
I'll document the current behavior for if().
Also please extend the test cases for this policy to cover:
- All if() command conditions that do variable-or-string.
Will do.
I was also thinking about setting up an alternative testing
infrastructure parallel to RunCMake which runs cmake in script mode
rather than performing configuration/generation.
It would be nice to have something less granular which would allow
multiple related test cases, expected outputs and exit statuses in a
single file (which I think could be nicely done with the new bracket
syntax).
Not for this topic obviously but would this make sense in general?
e.g. something similar to
expect_failure(
[[
if(
]],
[[Parse error. Function missing ending ). End of file reached.
]])
- if() inside a macro() and a function(), covering each
combination of policy settings when the macro/function
is recorded and when it is invoked. The policy value
should be that when the macro/function was recorded.
Is that behavior command specific?
Isn't it covered by policy testing in general?
Nils
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 09:19 AM, Nils Gladitz wrote: Would it be ok to set the policy for cmcurl to OLD? No, but the code could be fixed to not trigger the warning. See similar changes here: Check*: Allow result variables to contain regex special characters http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=4f2fcce4 Alternatively, just let the warnings go for now so we can see how they look. Then we can fix the code in a follow-up change. I kept the distinction in cmExpandedCommandArgument at first but then generalized it so that bracketed, quoted and C++ side 0, 1 constant arguments would be subsumed. This could be expanded again if there are future use cases where they need to be distinguished. That's fine. I'm just talking about the documentation. Right now it says quoted arguments do not expand in the NEW behavior. It should say something more like only unquoted arguments expand since both quoted and [[bracket]] arguments are left alone now. I was also thinking about setting up an alternative testing infrastructure parallel to RunCMake which runs cmake in script mode rather than performing configuration/generation. You can use run_cmake_command to run arbitrary cmake command-line signatures and still use the rest of the infrastructure. It would be nice to have something less granular which would allow multiple related test cases, expected outputs and exit statuses in a single file (which I think could be nicely done with the new bracket syntax). Not for this topic obviously but would this make sense in general? Yes. The existing RunCMake infrastructure could be extended with more commands that take the expected pieces directly as arguments. Then the current infrastructure could be ported to use it after reading info from the files. - if() inside a macro() and a function(), covering each combination of policy settings when the macro/function is recorded and when it is invoked. The policy value should be that when the macro/function was recorded. Is that behavior command specific? Isn't it covered by policy testing in general? The policy scope is general behavior, but we need to know that the forwarding of arguments into commands when replayed from macro/function works correctly to implement this policy. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
I was also thinking about setting up an alternative testing infrastructure parallel to RunCMake which runs cmake in script mode rather than performing configuration/generation. This already exists in a form in the Tests/CMakeTests/*TestScript* tests. An example: http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Tests/CMakeTests/StringTestScript.cmake;h=a562e71d38ff7db040e567f0ef9b899c2101bc9f;hb=ff1fddb0bf40b8a7170d54ccdc9420c2d7190472 Any *TestScript* file also requires a driving *Test.cmake.in file and an entry in Tests/CMakeTests/CMakeLists.txt, but the infrastructure is there already. Actually, there are two infrastructures in that folder, but this one is similar to the one you're proposing. HTH, David C. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 03:31 PM, Brad King wrote: You can use run_cmake_command to run arbitrary cmake command-line signatures and still use the rest of the infrastructure. Ok. I just thought it would be nice to distinguish tests that don't configure a project. (Headline for RunCMake is This directory contains tests that run CMake to configure a project but do not actually build anything.) E.g. have a well defined set of tests which are generator independent as well. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 03:33 PM, David Cole wrote: I was also thinking about setting up an alternative testing infrastructure parallel to RunCMake which runs cmake in script mode rather than performing configuration/generation. This already exists in a form in the Tests/CMakeTests/*TestScript* tests. An example: http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Tests/CMakeTests/StringTestScript.cmake;h=a562e71d38ff7db040e567f0ef9b899c2101bc9f;hb=ff1fddb0bf40b8a7170d54ccdc9420c2d7190472 Any *TestScript* file also requires a driving *Test.cmake.in file and an entry in Tests/CMakeTests/CMakeLists.txt, but the infrastructure is there already. Actually, there are two infrastructures in that folder, but this one is similar to the one you're proposing. I used the second infrastructure for the TIMESTAMP tests apparently. Those feel similar to the tests in RunCMake e.g. lots of files that I would prefer to combine into something more compact. The other infrastructure I don't think I have touched but optically it looks very noisy. Having something that takes away repetitive tasks, more compact and easier on the eye would be nice. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 09:50 AM, Nils Gladitz wrote: On 09/05/2014 03:31 PM, Brad King wrote: You can use run_cmake_command to run arbitrary cmake command-line signatures and still use the rest of the infrastructure. Ok. I just thought it would be nice to distinguish tests that don't configure a project. (Headline for RunCMake is This directory contains tests that run CMake to configure a project but do not actually build anything.) E.g. have a well defined set of tests which are generator independent as well. I think the description of RunCMake can just be updated. All of its tests are about running cmake command lines. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/05/2014 03:31 PM, Brad King wrote: I think the description of RunCMake can just be updated. All of its tests are about running cmake command lines. So you see no advantage to having the distinction? E.g. if you consider testing a set of generators (or variants of the same generator) possibly non-native and distinct from the generator being used to build cmake itself it might make sense to run tests which perform configuration once for each of those but script mode tests would only have to be run once. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
...I would prefer to combine into something more compact.The other infrastructure I don't think I have touched but optically it looks very noisy.Having something that takes away repetitive tasks, more compact and easier on the eye would be nice. I agree. That would be nice. Feel free to reinvent and re-factor. The ones I put in there were added in a flurry of increasing coverage a few years ago. I was just pointing it out in case you wanted to take advantage of an existing run-this-script pattern we already use. Cheers, D -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
[cmake-developers] Alternate if() without implicit variable expansion
This keeps coming up and there probably have been many discussions on how this might be fixed as well. I am wondering if we could provide an alias for the if command (e.g. safe_if) that would inherit cmIfCommand but would reimplement GetVariableOrString() without the variable lookup. This would leave if() behavior intact for compatibility but would provide an alternative with less surprising semantics. I would keep else() elseif() endif() without additional aliases and leave the behavior to the initial if variant. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
Another approach might be to add STRING_EQUAL and STRING_MATCHES (or similarly unambiguous names) operators that do not do the variable lookup. The documentation would start with: if(string STRING_EQUAL string) if(string STRING_MATCHES regex) ...i.e., not mentioning variable anywhere for these operators. D -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 10:25 AM, Nils Gladitz wrote:
This keeps coming up and there probably have been many discussions on
how this might be fixed as well.
As I have explained every other time it has come up there is exactly
one way to fix it: a policy that makes expansion happen only for
unquoted arguments. Someone just has to do it.
I am wondering if we could provide an alias for the if command (e.g.
safe_if) that would inherit cmIfCommand but would reimplement
GetVariableOrString() without the variable lookup.
This would leave if() behavior intact for compatibility but would
provide an alternative with less surprising semantics.
I think a new command would (if_noexpand) would make the correct
code less readable than the incorrect code. Also the implicit-bool
conversion of variables is more readable IMO; consider
if(${FOO_FOUND})
versus
if(FOO_FOUND)
On 09/04/2014 10:32 AM, David Cole via cmake-developers wrote:
Another approach might be to add STRING_EQUAL and STRING_MATCHES (or
similarly unambiguous names) operators that do not do the variable
lookup.
This is a reasonable proposal but the difference to STREQUAL
and MATCHES is quite subtle. Also adding more operators this
late in the game is more likely to collide with variable names
people already use.
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 04:40 PM, Brad King wrote:
On 09/04/2014 10:25 AM, Nils Gladitz wrote:
I think a new command would (if_noexpand) would make the correct
code less readable than the incorrect code. Also the implicit-bool
conversion of variables is more readable IMO; consider
if(${FOO_FOUND})
versus
if(FOO_FOUND)
I am rather used to if() as well but safe_if()/if_noexpand() might
still be more readable than the workarounds that people are using now to
get something close to the expected behavior with regular if() [1].
You would also still be able to use regular if() in cases where you
prefer the aesthetics of implicit-bool.
Nils
[1] http://www.cmake.org/pipermail/cmake/2014-September/058476.html
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 10:51 AM, Nils Gladitz wrote: I am rather used to if() as well but safe_if()/if_noexpand() might still be more readable than the workarounds that people are using now to get something close to the expected behavior with regular if() [1]. If if() is never fixed people will still run into that and be confused. Adding another command will leave people with the decision about which one to use in what cases and be even more confused. As I have explained every other time it has come up there is exactly one way to fix it: a policy that makes expansion happen only for unquoted arguments. Someone just has to do it. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 05:13 PM, Brad King wrote: As I have explained every other time it has come up there is exactly one way to fix it: a policy that makes expansion happen only for unquoted arguments. Someone just has to do it. The fact that this behaviour has persisted this long and that no one has volunteered to fix it is a bit intimidating. How large scale would the required changes be? I assume the fact that arguments were quoted would have to be preserved and the implementation of all existing commands touched so that they can actually make use of that information (even if only if() would currently make use of it). Are there any major pitfalls to look out for? Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 11:41 AM, Nils Gladitz wrote: I assume the fact that arguments were quoted would have to be preserved and the implementation of all existing commands touched so that they can actually make use of that information (even if only if() would currently make use of it). Most commands just implement InitialPass, which gets the expanded arguments. Some implement InvokeInitialPass which gets the args before expansion. The cmIfCommand does this already, and its call to cmMakefile::ExpandArguments could be updated to request the additional information. Other commands should not need to be modified. The cmMakefile::ExpandArguments method's output will have to provide the information. I'm not sure whether all call sites need it or not, but it could simply be implemented as a second argument of type vectorcmListFileArgument::Delimiter* = 0 to be populated with the same number of entries as the original argument. Or, a second signature of vectorCommandArg could be created to pair each output argument string with its delimiter type. Then cmIfCommand could use that internally. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Alternate if() without implicit variable expansion
On 09/04/2014 05:53 PM, Brad King wrote: Most commands just implement InitialPass, which gets the expanded arguments. Some implement InvokeInitialPass which gets the args before expansion. The cmIfCommand does this already, and its call to cmMakefile::ExpandArguments could be updated to request the additional information. Other commands should not need to be modified. The cmMakefile::ExpandArguments method's output will have to provide the information. I'm not sure whether all call sites need it or not, but it could simply be implemented as a second argument of type vectorcmListFileArgument::Delimiter* = 0 to be populated with the same number of entries as the original argument. Or, a second signature of vectorCommandArg could be created to pair each output argument string with its delimiter type. Then cmIfCommand could use that internally. Thanks, that actually sounds doable. I'll give it a try. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
