[cmake-developers] [PATCH] CPack: add a new setting, CPACK_VERBATIM_VARIABLES

2015-09-20 Thread Роман Донченко
If it's set to a true value, CPack will escape all variables for the CMake
parser when writing them out to configuration files. This parallels the
VERBATIM argument to add_custom_command.

The cpack_encode_variables macro is changed into a function to remove
scope pollution. There should be no other effects.
---
 Help/release/dev/CPack-updates.rst  |  5 +++
 Modules/CPack.cmake | 43 -
 Tests/RunCMake/CPackConfig/RunCMakeTest.cmake   |  2 ++
 Tests/RunCMake/CPackConfig/Special-check.cmake  |  5 +++
 Tests/RunCMake/CPackConfig/Special.cmake|  3 ++
 Tests/RunCMake/CPackConfig/Verbatim-check.cmake | 10 ++
 Tests/RunCMake/CPackConfig/Verbatim.cmake   |  5 +++
 7 files changed, 65 insertions(+), 8 deletions(-)
 create mode 100644 Tests/RunCMake/CPackConfig/Special-check.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Special.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Verbatim-check.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Verbatim.cmake

diff --git a/Help/release/dev/CPack-updates.rst 
b/Help/release/dev/CPack-updates.rst
index 7ac1ed7..ea0780f 100644
--- a/Help/release/dev/CPack-updates.rst
+++ b/Help/release/dev/CPack-updates.rst
@@ -4,3 +4,8 @@ CPack-updates
 * The :module:`CPack` module no longer mangles settings with CMake-special
   characters when they're used as defaults for other settings. The macro
   ``cpack_set_if_not_set``, which was responsible for this, is now deprecated.
+
+* The :module:`CPack` module gained a new setting, 
``CPACK_VERBATIM_VARIABLES``,
+  which can be used to ensure the cpack program receives the settings' values
+  exactly as they were set, even if they contain CMake-special characters.
+  For compatibility, it's off by default.
diff --git a/Modules/CPack.cmake b/Modules/CPack.cmake
index 7d6d54c..ea5283c 100644
--- a/Modules/CPack.cmake
+++ b/Modules/CPack.cmake
@@ -182,6 +182,14 @@
 #  will be a boolean variable which enables stripping of all files (a list
 #  of files evaluates to TRUE in CMake, so this change is compatible).
 #
+# .. variable:: CPACK_VERBATIM_VARIABLES
+#
+#  If set to a true value, the other variables' values will be escaped before
+#  being written to the configuration files, so that the cpack program receives
+#  them exactly as they were specified. If not, characters like quotes and
+#  backslashes can cause parsing errors or alter the value received by the
+#  cpack program. Defaults to FALSE for backwards compatibility.
+#
 # The following CPack variables are specific to source packages, and
 # will not affect binary packages:
 #
@@ -305,21 +313,28 @@ macro(cpack_set_if_not_set name value)
   _cpack_set_default("${name}" "${value}")
 endmacro()
 
-# cpack_encode_variables - Macro to encode variables for the configuration file
+# cpack_encode_variables - Function to encode variables for the configuration 
file
 # find any variable that starts with CPACK and create a variable
 # _CPACK_OTHER_VARIABLES_ that contains SET commands for
 # each cpack variable.  _CPACK_OTHER_VARIABLES_ is then
 # used as an @ replacment in configure_file for the CPackConfig.
-macro(cpack_encode_variables)
-  set(_CPACK_OTHER_VARIABLES_)
+function(cpack_encode_variables)
+  set(commands "")
   get_cmake_property(res VARIABLES)
   foreach(var ${res})
 if(var MATCHES "^CPACK")
-  set(_CPACK_OTHER_VARIABLES_
-"${_CPACK_OTHER_VARIABLES_}\nSET(${var} \"${${var}}\")")
+  if(CPACK_VERBATIM_VARIABLES)
+_cpack_escape_for_cmake(value "${${var}}")
+  else()
+set(value "${${var}}")
   endif()
+
+  set(commands "${commands}\nSET(${var} \"${value}\")")
+endif()
   endforeach()
-endmacro()
+
+  set(_CPACK_OTHER_VARIABLES_ "${commands}" PARENT_SCOPE)
+endfunction()
 
 # Internal use functions
 function(_cpack_set_default name value)
@@ -328,6 +343,11 @@ function(_cpack_set_default name value)
   endif()
 endfunction()
 
+function(_cpack_escape_for_cmake var value)
+  string(REGEX REPLACE "([\\\$\"])" "\\1" escaped "${value}")
+  set("${var}" "${escaped}" PARENT_SCOPE)
+endfunction()
+
 # Set the package name
 _cpack_set_default(CPACK_PACKAGE_NAME "${CMAKE_PROJECT_NAME}")
 _cpack_set_default(CPACK_PACKAGE_VERSION_MAJOR "0")
@@ -608,8 +628,15 @@ _cpack_set_default(CPACK_SOURCE_INSTALLED_DIRECTORIES
 _cpack_set_default(CPACK_SOURCE_TOPLEVEL_TAG "${CPACK_SYSTEM_NAME}-Source")
 _cpack_set_default(CPACK_SOURCE_PACKAGE_FILE_NAME
   "${CPACK_PACKAGE_NAME}-${CPACK_PACKAGE_VERSION}-Source")
-_cpack_set_default(CPACK_SOURCE_IGNORE_FILES
-  "/CVS/;/.svn/;/.bzr/;/.hg/;/.git/;.swp$;.#;/#")
+
+set(__cpack_source_ignore_files_default
+  "/CVS/;/\\.svn/;/\\.bzr/;/\\.hg/;/\\.git/;\\.swp$;\\.#;/#")
+if(NOT CPACK_VERBATIM_VARIABLES)
+  _cpack_escape_for_cmake(__cpack_source_ignore_files_default
+"${__cpack_source_ignore_files_default}")
+endif()
+_cpack_set_default(CPACK_SOURCE_IGNORE_FILES 

[cmake-developers] [PATCH] CPack: don't mangle CMake-special characters when applying default settings

2015-09-03 Thread Роман Донченко
By using a function to do it instead of a macro. Deprecate the old macro,
but keep it for backwards compatibility. Remove existing workarounds for this
problem and add a test.
---
 Help/release/dev/CPack-updates.rst |  6 ++
 Modules/CPack.cmake| 92 ++
 Tests/RunCMake/CPackConfig/Default-check.cmake |  7 ++
 Tests/RunCMake/CPackConfig/Default.cmake   |  3 +
 Tests/RunCMake/CPackConfig/RunCMakeTest.cmake  |  1 +
 Tests/RunCMake/CPackConfig/check.cmake |  4 ++
 6 files changed, 70 insertions(+), 43 deletions(-)
 create mode 100644 Help/release/dev/CPack-updates.rst
 create mode 100644 Tests/RunCMake/CPackConfig/Default-check.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Default.cmake

diff --git a/Help/release/dev/CPack-updates.rst 
b/Help/release/dev/CPack-updates.rst
new file mode 100644
index 000..7ac1ed7
--- /dev/null
+++ b/Help/release/dev/CPack-updates.rst
@@ -0,0 +1,6 @@
+CPack-updates
+-
+
+* The :module:`CPack` module no longer mangles settings with CMake-special
+  characters when they're used as defaults for other settings. The macro
+  ``cpack_set_if_not_set``, which was responsible for this, is now deprecated.
diff --git a/Modules/CPack.cmake b/Modules/CPack.cmake
index e223286..9fe349c 100644
--- a/Modules/CPack.cmake
+++ b/Modules/CPack.cmake
@@ -299,10 +299,10 @@ endif()
 include(CPackComponent)
 
 # Macro for setting values if a user did not overwrite them
+# Mangles CMake-special characters. Only kept for backwards compatibility.
 macro(cpack_set_if_not_set name value)
-  if(NOT DEFINED "${name}")
-set(${name} "${value}")
-  endif()
+  message(DEPRECATION "cpack_set_if_not_set is obsolete; do not use.")
+  _cpack_set_default("${name}" "${value}")
 endmacro()
 
 # cpack_encode_variables - Macro to encode variables for the configuration file
@@ -321,27 +321,35 @@ macro(cpack_encode_variables)
   endforeach()
 endmacro()
 
+# Internal use functions
+
+function(_cpack_set_default name value)
+  if(NOT DEFINED "${name}")
+set("${name}" "${value}" PARENT_SCOPE)
+  endif()
+endfunction()
+
 # Set the package name
-cpack_set_if_not_set(CPACK_PACKAGE_NAME "${CMAKE_PROJECT_NAME}")
-cpack_set_if_not_set(CPACK_PACKAGE_VERSION_MAJOR "0")
-cpack_set_if_not_set(CPACK_PACKAGE_VERSION_MINOR "1")
-cpack_set_if_not_set(CPACK_PACKAGE_VERSION_PATCH "1")
-cpack_set_if_not_set(CPACK_PACKAGE_VERSION
+_cpack_set_default(CPACK_PACKAGE_NAME "${CMAKE_PROJECT_NAME}")
+_cpack_set_default(CPACK_PACKAGE_VERSION_MAJOR "0")
+_cpack_set_default(CPACK_PACKAGE_VERSION_MINOR "1")
+_cpack_set_default(CPACK_PACKAGE_VERSION_PATCH "1")
+_cpack_set_default(CPACK_PACKAGE_VERSION
   
"${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH}")
-cpack_set_if_not_set(CPACK_PACKAGE_VENDOR "Humanity")
-cpack_set_if_not_set(CPACK_PACKAGE_DESCRIPTION_SUMMARY
+_cpack_set_default(CPACK_PACKAGE_VENDOR "Humanity")
+_cpack_set_default(CPACK_PACKAGE_DESCRIPTION_SUMMARY
   "${CMAKE_PROJECT_NAME} built using CMake")
 
-cpack_set_if_not_set(CPACK_PACKAGE_DESCRIPTION_FILE
+_cpack_set_default(CPACK_PACKAGE_DESCRIPTION_FILE
   "${CMAKE_ROOT}/Templates/CPack.GenericDescription.txt")
-cpack_set_if_not_set(CPACK_RESOURCE_FILE_LICENSE
+_cpack_set_default(CPACK_RESOURCE_FILE_LICENSE
   "${CMAKE_ROOT}/Templates/CPack.GenericLicense.txt")
-cpack_set_if_not_set(CPACK_RESOURCE_FILE_README
+_cpack_set_default(CPACK_RESOURCE_FILE_README
   "${CMAKE_ROOT}/Templates/CPack.GenericDescription.txt")
-cpack_set_if_not_set(CPACK_RESOURCE_FILE_WELCOME
+_cpack_set_default(CPACK_RESOURCE_FILE_WELCOME
   "${CMAKE_ROOT}/Templates/CPack.GenericWelcome.txt")
 
-cpack_set_if_not_set(CPACK_MODULE_PATH "${CMAKE_MODULE_PATH}")
+_cpack_set_default(CPACK_MODULE_PATH "${CMAKE_MODULE_PATH}")
 
 if(CPACK_NSIS_ENABLE_UNINSTALL_BEFORE_INSTALL)
   set(CPACK_NSIS_ENABLE_UNINSTALL_BEFORE_INSTALL ON)
@@ -359,7 +367,7 @@ if(__cpack_system_name MATCHES "Windows")
 set(__cpack_system_name win32)
   endif()
 endif()
-cpack_set_if_not_set(CPACK_SYSTEM_NAME "${__cpack_system_name}")
+_cpack_set_default(CPACK_SYSTEM_NAME "${__cpack_system_name}")
 
 # Root dir: default value should be the string literal "$PROGRAMFILES"
 # for backwards compatibility. Projects may set this value to anything.
@@ -369,17 +377,17 @@ if("x${__cpack_system_name}" STREQUAL "xwin64")
 else()
   set(__cpack_root_default "$PROGRAMFILES")
 endif()
-cpack_set_if_not_set(CPACK_NSIS_INSTALL_ROOT "${__cpack_root_default}")
+_cpack_set_default(CPACK_NSIS_INSTALL_ROOT "${__cpack_root_default}")
 
 # -..--.
-cpack_set_if_not_set(CPACK_PACKAGE_FILE_NAME
+_cpack_set_default(CPACK_PACKAGE_FILE_NAME
   "${CPACK_PACKAGE_NAME}-${CPACK_PACKAGE_VERSION}-${CPACK_SYSTEM_NAME}")
-cpack_set_if_not_set(CPACK_PACKAGE_INSTALL_DIRECTORY
+_cpack_set_default(CPACK_PACKAGE_INSTALL_DIRECTORY
   "${CPACK_PACKAGE_NAME} ${CPACK_PACKAGE_VERSION}")
-cpack_set_if_not_set(CPACK_PACKAGE_INSTALL_REGISTRY_KEY

Re: [cmake-developers] [PATCH] Add a test for checking that the CPack module produces correct config files

2015-08-26 Thread Роман Донченко
Domen Vrankar domen.vran...@gmail.com писал в своём письме Wed, 26 Aug  
2015 22:27:43 +0300:



2015-08-19 21:08 GMT+02:00 Роман Донченко d...@corrigendum.ru:
Currently, the only case is for simple variable values (not containing  
any

CMake-special characters and not determined by CPack itself).


In CPackRPM and CPackDeb both cases that this is testing (CMake list
variable and a variable containing spaces) are already tested
implicitly. RunCMake.CPack contains such test. Admittedly both cases
are not present in same variable.



Is this test targeting a particular CPack generator or a CMake-CPack
bug on a particular OS?


My motivation is this. I want to submit a few improvements to the way  
CPack writes config files, starting with that cpack_set_if_not_set change  
I mentioned a while back. This functionality is generator-independent (and  
basically unrelated to actual package generation), so I thought  
RunCMake.CPack was not the appropriate place to test it.


Since there doesn't seem to be a test suite specifically for config file  
writing, I thought I'd create one in a separate patch so that my later  
patches don't have as much boilerplate in them. The Simple test is just  
there to ensure I don't break the simple case while changing the more  
complex ones. Well, and it would be silly to submit a test suite with no  
tests in it.


Roman.
--

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] Add a test for checking that the CPack module produces correct config files

2015-08-19 Thread Роман Донченко
Currently, the only case is for simple variable values (not containing any
CMake-special characters and not determined by CPack itself).
---
 Tests/RunCMake/CMakeLists.txt | 1 +
 Tests/RunCMake/CPackConfig/CMakeLists.txt | 6 ++
 Tests/RunCMake/CPackConfig/RunCMakeTest.cmake | 3 +++
 Tests/RunCMake/CPackConfig/Simple-check.cmake | 3 +++
 Tests/RunCMake/CPackConfig/Simple.cmake   | 1 +
 Tests/RunCMake/CPackConfig/check.cmake| 7 +++
 6 files changed, 21 insertions(+)
 create mode 100644 Tests/RunCMake/CPackConfig/CMakeLists.txt
 create mode 100644 Tests/RunCMake/CPackConfig/RunCMakeTest.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Simple-check.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/Simple.cmake
 create mode 100644 Tests/RunCMake/CPackConfig/check.cmake

diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index c274d8f..2955db2 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -239,6 +239,7 @@ add_RunCMake_test(CommandLine)
 add_RunCMake_test(CommandLineTar)
 
 add_RunCMake_test(install)
+add_RunCMake_test(CPackConfig)
 add_RunCMake_test(CPackInstallProperties)
 add_RunCMake_test(ExternalProject)
 add_RunCMake_test(CTestCommandLine)
diff --git a/Tests/RunCMake/CPackConfig/CMakeLists.txt 
b/Tests/RunCMake/CPackConfig/CMakeLists.txt
new file mode 100644
index 000..9a9e7b4
--- /dev/null
+++ b/Tests/RunCMake/CPackConfig/CMakeLists.txt
@@ -0,0 +1,6 @@
+cmake_minimum_required(VERSION 3.3)
+
+project(${RunCMake_TEST})
+include(${RunCMake_TEST}.cmake)
+
+include(CPack)
diff --git a/Tests/RunCMake/CPackConfig/RunCMakeTest.cmake 
b/Tests/RunCMake/CPackConfig/RunCMakeTest.cmake
new file mode 100644
index 000..6787eb8
--- /dev/null
+++ b/Tests/RunCMake/CPackConfig/RunCMakeTest.cmake
@@ -0,0 +1,3 @@
+include(RunCMake)
+
+run_cmake(Simple)
diff --git a/Tests/RunCMake/CPackConfig/Simple-check.cmake 
b/Tests/RunCMake/CPackConfig/Simple-check.cmake
new file mode 100644
index 000..6e0cf6f
--- /dev/null
+++ b/Tests/RunCMake/CPackConfig/Simple-check.cmake
@@ -0,0 +1,3 @@
+include(${RunCMake_SOURCE_DIR}/check.cmake)
+
+test_variable(CPACK_FOO bar baz;quux)
diff --git a/Tests/RunCMake/CPackConfig/Simple.cmake 
b/Tests/RunCMake/CPackConfig/Simple.cmake
new file mode 100644
index 000..c9f6541
--- /dev/null
+++ b/Tests/RunCMake/CPackConfig/Simple.cmake
@@ -0,0 +1 @@
+set(CPACK_FOO bar baz;quux)
diff --git a/Tests/RunCMake/CPackConfig/check.cmake 
b/Tests/RunCMake/CPackConfig/check.cmake
new file mode 100644
index 000..2fc9f11
--- /dev/null
+++ b/Tests/RunCMake/CPackConfig/check.cmake
@@ -0,0 +1,7 @@
+function(test_variable NAME EXPECTED_VALUE)
+  if(NOT ${${NAME}} STREQUAL ${EXPECTED_VALUE})
+message(FATAL_ERROR ${NAME}: variable mismatch; expected 
[${EXPECTED_VALUE}] actual [${${NAME}}])
+  endif()
+endfunction()
+
+include(${RunCMake_TEST_BINARY_DIR}/CPackConfig.cmake)
-- 
1.9.5.msysgit.0

-- 

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] CPack: change cpack_set_if_not_set into a function

2015-08-17 Thread Роман Донченко
Okay, so after a... brief hiatus, I'm going to tackle this again. There's  
something I don't understand, though...


Brad King brad.k...@kitware.com писал в своём письме Tue, 03 Mar 2015  
23:02:51 +0300:

On 03/03/2015 01:33 PM, Роман Донченко wrote:
Since it's currently a macro, then due to macro processing rules,  
special

characters inside the value are interpreted by the parser, which can
result in syntax errors or a mismatch between the given default and the
value the variable is actually set to.

Remove existing local workarounds for this issue, which are no longer
required.


Thanks.  As nice as it would be to do that, the reason it has not been
done is that this API is exposed publicly.  Any existing calls within
other projects would also have such workarounds and they would be broken
by this change.


What makes this macro a public one? It's not listed in cmake  
--help-module CPack, so I assumed it was an implementation detail.  
Moreover...



Instead one could introduce a new API function for this, e.g.

 cpack_maybe_set(...)


Even supposing that the macro is part of the API and is thus immutable, I  
don't see why its replacement should be. The operation it performs is not  
related to creating packages, so to me it seems that it should not be  
exported outside of the module.


Could you clarify this for me?

Roman.
--

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