Re: [cmake-developers] [PATCH] FindCUDA: Wrap keyword in if() string comparison
On 09/22/2014 10:07 AM, Brad King wrote: Or, one could add the explicit PUSH/POP in modules, e.g. cmake_policy(PUSH) cmake_policy(SET CMP0054 NEW) ... module code ... cmake_policy(POP) Until such time as someone wishes to port a module explicitly with the above approach we can use the explicit dereference approach Adam first posted. I've applied his patch here: FindCUDA: Avoid if() auto-dereference in string comparisons http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=84e3fde9 -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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
Again, shouldn't we consider having CMP0011 always NEW for internal modules / or do implicit pushpop for include when CMP0011 is NEW *OR* when included module is part of CMake own modules? --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
On 09/24/2014 10:49 AM, Adam Strzelecki wrote: do implicit pushpop for include when CMP0011 is NEW *OR* when included module is part of CMake own modules I think this would be okay, as long as NO_POLICY_SCOPE is still honored when given explicitly to include() and find_package(). The only reason for the pre-CMP0011 behavior and NO_POLICY_SCOPE is in case the purpose of a module is to set policies. None of the builtin modules do that. 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
On 09/21/2014 02:48 PM, Adam Strzelecki wrote: FYI unfortunately this solution does not work for CMakeFiles.txt with cmake_minimum_required(VERSION 2.6.1) or earlier because of CMP0011 that does implicit PUSH/POP does not work there. So setting cmake_policy(SET CMP0054 NEW) is internal modules will cause this policy to be not popped and carried out of the module. I do not have any idea how to solve this in some elegant fashion This is one reason I just fixed the warnings with the super ugly approach on some of the other modules. It is a cost we pay upstream to support older clients, but the policy will still allow cleaner code in applications. Or, one could add the explicit PUSH/POP in modules, e.g. cmake_policy(PUSH) cmake_policy(SET CMP0054 NEW) ... module code ... cmake_policy(POP) -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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
FYI unfortunately this solution does not work for CMakeFiles.txt with cmake_minimum_required(VERSION 2.6.1) or earlier because of CMP0011 that does implicit PUSH/POP does not work there. So setting cmake_policy(SET CMP0054 NEW) is internal modules will cause this policy to be not popped and carried out of the module. This is pretty nasty surprise, since I thought this implicit PUSH/POP happened since the beginning. So currently I do not have any idea how to solve this in some elegant fashion therefore for now I removed stage/CMP0054-FindCUDA branch. Maybe you can suggest something? --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
Addendum: Maybe CMP0011 should be forced implicitly for all built-in modules? --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
I've pushed stage/CMP0054-FindCUDA: commit 8998557e7c9a7542e78e07b8b06fd728604f0bdf Author: Adam Strzelecki o...@java.pl Date: Tue Sep 16 23:31:44 2014 +0200 FindCUDA: Audit for modern CMP0054 if() syntax Enables CMP0054 for whole module to silence quoted variable expansion warning. Just added where the actual code starts: cmake_policy(SET CMP0054 NEW) # modern if() syntax And made one simple fix with unneeded variable and unneeded quoting. I think we may do the same for the other modules. WDYT? --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
On 09/19/2014 07:30 AM, James Bigler wrote:
Could someone please explain to me why we need to change all the if
statements to look super ugly?
It sounds like you might have missed the follow up within this thread.
Expanding WIN32 to dereference the value is just a recipe for
confusion. Honestly I would prefer if there was a policy to never
deference variables in if statements and replace the if's with:
if (${arg} STREQAL WIN32)
To recap I did introduce a policy similar to this:
http://www.cmake.org/cmake/help/git-master/policy/CMP0054.html
This is paradoxically also the reason for the proposed super ugly changes.
The policy not only changes the behavior when set to NEW it also warns
about ambiguities like WIN32 in an if statement while using the OLD
behavior.
Since the policy may or may not be active (depending on the user's
project) they might be using FindCUDA.cmake with CMP0054 set to NEW or
OLD. To get identical (and warning free) behavior irregardless of the
current policy setting Adam added the proposed ugliness.
Afterwards I proposed setting all policies to NEW for internal CMake
modules but Brad argues that since there is no proper coverage for find
modules within the test suite this might have unintended consequences.
As Brad further stated you could set CMP0054 to NEW explicitly within
FindCUDA.cmake (so that the ugliness could be avoided) if you can verify
that this does not break the module for existing projects.
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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
Since the policy may or may not be active (depending on the user's project) they might be using FindCUDA.cmake with CMP0054 set to NEW or OLD. To get identical (and warning free) behavior irregardless of the current policy setting Adam added the proposed ugliness. Putting my 2ยข, we can either/or: (1) put everywhere these super ugly changes, which will make code (even more) obscure (2) put cmake_policy(SET CMP0053 NEW) at the every internal .cmake file that has been reviewed for this policy and use normal (not ugly syntax) Since include/find_package does implicit policy PUSH POP for file scope this is IMHO much cleaner solution. Of course we would need to rollback couple of existing fixes committed by Brad. WDYT? --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
I like the cmake_policy(SET CMP0053 NEW) solution, too. We should make a concerted effort to encourage module maintainers to do this before the 3.1 release for as many modules as possible. 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
Could someone please explain to me why we need to change all the if
statements to look super ugly?
Expanding WIN32 to dereference the value is just a recipe for confusion.
Honestly I would prefer if there was a policy to never deference variables
in if statements and replace the if's with:
if (${arg} STREQAL WIN32)
This way there isn't any trickery involved with whether WIN32 is string
or a variable.
By the way creating a variable that has the same string as arguments to
command (see add_executable) is an unfortunate dirtying of the language
(WIN32 is a variable here, but over here it's an argument e.g. string).
James
On Wed, Sep 17, 2014 at 5:54 AM, Adam Strzelecki o...@java.pl wrote:
This silents possible CMP0054 related warnings.
---
Modules/FindCUDA.cmake | 14 +++---
Modules/FindCUDA/run_nvcc.cmake | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Modules/FindCUDA.cmake b/Modules/FindCUDA.cmake
index 9348aa5..2e2b21c 100644
--- a/Modules/FindCUDA.cmake
+++ b/Modules/FindCUDA.cmake
@@ -894,15 +894,15 @@ macro(CUDA_GET_SOURCES_AND_OPTIONS _sources
_cmake_options _options)
set( ${_options} )
set( _found_options FALSE )
foreach(arg ${ARGN})
-if(arg STREQUAL OPTIONS)
+if(x${arg} STREQUAL xOPTIONS)
set( _found_options TRUE )
elseif(
-arg STREQUAL WIN32 OR
-arg STREQUAL MACOSX_BUNDLE OR
-arg STREQUAL EXCLUDE_FROM_ALL OR
-arg STREQUAL STATIC OR
-arg STREQUAL SHARED OR
-arg STREQUAL MODULE
+x${arg} STREQUAL xWIN32 OR
+x${arg} STREQUAL xMACOSX_BUNDLE OR
+x${arg} STREQUAL xEXCLUDE_FROM_ALL OR
+x${arg} STREQUAL xSTATIC OR
+x${arg} STREQUAL xSHARED OR
+x${arg} STREQUAL xMODULE
)
list(APPEND ${_cmake_options} ${arg})
else()
diff --git a/Modules/FindCUDA/run_nvcc.cmake
b/Modules/FindCUDA/run_nvcc.cmake
index 58e0d31..abdd307 100644
--- a/Modules/FindCUDA/run_nvcc.cmake
+++ b/Modules/FindCUDA/run_nvcc.cmake
@@ -126,7 +126,7 @@ endif()
# and other return variables are present after executing the process.
macro(cuda_execute_process status command)
set(_command ${command})
- if(NOT _command STREQUAL COMMAND)
+ if(NOT x${_command} STREQUAL xCOMMAND)
message(FATAL_ERROR Malformed call to cuda_execute_process. Missing
COMMAND as second argument. (command = ${command}))
endif()
if(verbose)
--
1.9.3 (Apple Git-50)
--
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
--
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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
This silents possible CMP0054 related warnings.
---
Modules/FindCUDA.cmake | 14 +++---
Modules/FindCUDA/run_nvcc.cmake | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Modules/FindCUDA.cmake b/Modules/FindCUDA.cmake
index 9348aa5..2e2b21c 100644
--- a/Modules/FindCUDA.cmake
+++ b/Modules/FindCUDA.cmake
@@ -894,15 +894,15 @@ macro(CUDA_GET_SOURCES_AND_OPTIONS _sources
_cmake_options _options)
set( ${_options} )
set( _found_options FALSE )
foreach(arg ${ARGN})
-if(arg STREQUAL OPTIONS)
+if(x${arg} STREQUAL xOPTIONS)
set( _found_options TRUE )
elseif(
-arg STREQUAL WIN32 OR
-arg STREQUAL MACOSX_BUNDLE OR
-arg STREQUAL EXCLUDE_FROM_ALL OR
-arg STREQUAL STATIC OR
-arg STREQUAL SHARED OR
-arg STREQUAL MODULE
+x${arg} STREQUAL xWIN32 OR
+x${arg} STREQUAL xMACOSX_BUNDLE OR
+x${arg} STREQUAL xEXCLUDE_FROM_ALL OR
+x${arg} STREQUAL xSTATIC OR
+x${arg} STREQUAL xSHARED OR
+x${arg} STREQUAL xMODULE
)
list(APPEND ${_cmake_options} ${arg})
else()
diff --git a/Modules/FindCUDA/run_nvcc.cmake b/Modules/FindCUDA/run_nvcc.cmake
index 58e0d31..abdd307 100644
--- a/Modules/FindCUDA/run_nvcc.cmake
+++ b/Modules/FindCUDA/run_nvcc.cmake
@@ -126,7 +126,7 @@ endif()
# and other return variables are present after executing the process.
macro(cuda_execute_process status command)
set(_command ${command})
- if(NOT _command STREQUAL COMMAND)
+ if(NOT x${_command} STREQUAL xCOMMAND)
message(FATAL_ERROR Malformed call to cuda_execute_process. Missing
COMMAND as second argument. (command = ${command}))
endif()
if(verbose)
--
1.9.3 (Apple Git-50)
--
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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
Wait, what? This is actually the opposite of what that policy is for, no? Adam, I don't blame you, just to get that said first. The question is: does this policy warning trigger far too often? Yes, you are absolutely right. But the problem is that internal modules run in whatever policy is currently set. Few of them conditionally change policy for some short scopes. I was more less following how Brad has been fixing this issues on other modules, i.e.: e177e7affb10fc25b71d4c9d9796c9df7fcdb800 Honestly I would expect that all internal modules run in latest policy by default, or at least declare their policy in file header, but this is not a case. --Adam -- 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] [PATCH] FindCUDA: Wrap keyword in if() string comparison
On 17.09.2014 14:48, Nils Gladitz wrote: Maybe modules loaded from the cmake installation itself should implicitly have their policy version set to the current cmake version? I pushed root-modules-set-policies-new to stage which sets all policies to NEW for modules from cmake's own Modules directory if NO_POLICY_SCOPE is not used. I haven't merged it to next yet in case someone things this is the wrong thing to do in general. I figured since the new behavior only applies to cmake's own modules it should not require a policy. 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
