Merge pull request #486 from TriBITSPub/485-address-review-comments

Address review feedback from PR #485

test/core/TestingFunctionMacro_UnitTests.cmake

@@ -494,7 +494,7 @@ function(unittest_tribits_git_repo_sha1)
   message("*** Testing tribits_git_repo_sha1()")
   message("***\n")
 
-  find_program(GIT_EXECUTABLE ${GIT_NAME})
+  find_package(Git REQUIRED)
 
   set(tribitsProjDir "${${PROJECT_NAME}_TRIBITS_DIR}/..")
 

test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt

@@ -98,16 +98,16 @@ set(TribitsExampleMetaProject_COMMON_CONFIG_ARGS
 ##########################################################################################
 
 
-  set(cdash_build_url_expected_regex
+set(cdash_build_url_expected_regex
     "https://testing[.]sandia[.]gov/cdash/index[.]php[?]project=TribitsExampleMetaProject&filtercount=3&showfilters=1&filtercombine=and&field1=site&compare1=61&value1=CustomSite&field2=buildname&compare2=61&value2=CTestDriver_TribitsExMetaProj_clone_default_branch_remote&field3=buildstamp&compare3=61&value3=[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9]-Continuous" )
   # NOTE: Above, we have to repeat [0-9] 8 times and 4 times for the regex for
   # the number of digits in the buildstarttime.  CMake regex does not support
   # \d{8}-\d{4} :-(
 
-  set(cdash_revisions_builds_url_expected_regex
+set(cdash_revisions_builds_url_expected_regex
      "https://testing[.]sandia[.]gov/cdash/index.php[?]project=TribitsExampleMetaProject&filtercount=1&showfilters=1&field1=revision&compare1=61&value1=863461e3035d24c632e175c087761e83db28bdc3")
 
-  set(cdash_revisions_nonpassing_tests_expected_regex
+set(cdash_revisions_nonpassing_tests_expected_regex
     "https://testing[.]sandia[.]gov/cdash/queryTests.php[?]project=TribitsExampleMetaProject&filtercount=2&showfilters=1&filtercombine=and&field1=revision&compare1=61&value1=863461e3035d24c632e175c087761e83db28bdc3&field2=status&compare2=62&value2=passed" )
 
 

tribits/core/package_arch/TribitsAddAdvancedTest.cmake

@@ -46,7 +46,13 @@ include(AppendStringVar)
 include(PrintVar)
 
 
-#
+# Avoid quoted strings lookup variables
+cmake_policy(SET CMP0054 NEW)
+# NOTE: For some reason, setting this policy at the top level with TriBITS
+# in TribitsCMakePolices.cmake does not affect this function.  Therefore, I
+# have to set it again here.
+
+
 # @FUNCTION: tribits_add_advanced_test()
 #
 # Function that creates an advanced test defined by stringing together one or
@@ -900,11 +906,6 @@ function(tribits_add_advanced_test TEST_NAME_IN)
     set(TEST_NAME ${TEST_NAME_IN})
   endif()
 
-  # Avoid quoted strings lookup variables
-  cmake_policy(SET CMP0054 NEW)
-  # NOTE: For some reason, setting this policy at the top level with TriBITS
-  # in TribitsCMakePolices.cmake does not affect this function.  Therefore, I
-  # have to set it again here.
 
   #
   # A) Parse the overall arguments and figure out how many tests

tribits/core/package_arch/TribitsAddTest.cmake

@@ -39,6 +39,8 @@
 
 include(TribitsAddTestHelpers)
 
+cmake_policy(SET CMP0007 NEW) # Don't ignore empty list elements
+
 
 # @FUNCTION: tribits_add_test()
 #
@@ -815,8 +817,6 @@ function(tribits_add_test EXE_NAME)
     message("TRIBITS_ADD_TEST: ${EXE_NAME} ${ARGN}")
   endif()
 
-  cmake_policy(SET CMP0007 NEW) # Don't ignore empty list elements
-
   global_set(TRIBITS_ADD_TEST_ADD_TEST_INPUT)
   global_set(TRIBITS_SET_TEST_PROPERTIES_INPUT)
   global_set(MESSAGE_WRAPPER_INPUT)

tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake

@@ -41,6 +41,8 @@ include(TribitsGeneralMacros)
 
 include(MessageWrapper)
 
+cmake_policy(SET CMP0057 NEW) # Support if ( ... IN_LIST ... )
+
 
 # @FUNCTION: tribits_external_package_write_config_file()
 #
@@ -458,7 +460,6 @@ function(tribits_external_package_process_libraries_list_library_entry
     tplName  libentry  libEntryType
     libTargetsInOut  lastLibProcessedInOut  configFileStrInOut
   )
-  cmake_policy(SET CMP0057 NEW) # Support if ( ... IN_LIST ... )
   # Set local vars for inout vars
   set(libTargets ${${libTargetsInOut}})
   set(lastLibProcessed ${${lastLibProcessedInOut}})
@@ -644,7 +645,6 @@ endfunction()
 function(tribits_external_package_append_upstream_target_link_libraries_get_name_and_vis
     upstreamTplDepEntry  upstreamTplDepNameOut  upstreamTplDepVisOut
   )
-  cmake_policy(SET CMP0057 NEW)
   # Split on ':' to get <PkgName>[:<Vis>]
   string(REPLACE ":" ";" upstreamTplAndVisList  "${upstreamTplDepEntry}")
   list(LENGTH upstreamTplAndVisList upstreamTplAndVisListLen)

tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake

@@ -83,24 +83,17 @@ function(tribits_git_repo_sha1  gitRepoDir  gitRepoSha1Out)
   set(gitRepoSha1 "")
   if (failureMsg STREQUAL "")
     execute_process(
-      COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%H"
+      COMMAND ${GIT_EXECUTABLE} log -1 "--pretty=format:%H"
       WORKING_DIRECTORY  ${gitRepoDir}
       RESULT_VARIABLE  gitCmndRtn  OUTPUT_VARIABLE  gitCmndOutput
       )
-    # NOTE: Above we have to add quotes '"' or CMake will not accept the
-    # command.  However, git will put those quotes in the output so we have to
-    # strip them out below!
 
     if (NOT gitCmndRtn STREQUAL 0)
       set(failureMsg "ERROR: ${GIT_EXECUTABLE} command returned ${gitCmndRtn}!=0 for repo ${gitRepoDir}!")
     else()
-      # Strip the quotes off :-(
-      string(LENGTH "${gitCmndOutput}" gitCmndOutputLen)
-      math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2")
-      string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep} gitRepoSha1)
+      set(gitRepoSha1 "${gitCmndOutput}")
     endif()
   endif()
-  # ToDo: Factor above out into its own function!
 
   if (NOT  failureMsg  STREQUAL "" AND  PARSE_FAILURE_MESSAGE_OUT  STREQUAL "")
     message(FATAL_ERROR "${failureMsg}")
@@ -123,31 +116,24 @@ function(tribits_generate_single_repo_version_string  gitRepoDir
   # A) Get the basic version info.
 
   execute_process(
-    COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%h [%ad] <%ae>"
+    COMMAND ${GIT_EXECUTABLE} log -1 "--pretty=format:%h [%ad] <%ae>"
     WORKING_DIRECTORY ${gitRepoDir}
     RESULT_VARIABLE gitCmndRtn
     OUTPUT_VARIABLE gitCmndOutput
     )
-  # NOTE: Above we have to add quotes '"' or CMake will not accept the
-  # command.  However, git will put those quotes in the output so we have to
-  # strip them out later :-(
 
   if (NOT gitCmndRtn STREQUAL 0)
     message(FATAL_ERROR "ERROR, ${GIT_EXECUTABLE} command returned ${gitCmndRtn}!=0"
       " for repo ${gitRepoDir}!")
     set(gitVersionLine "Error, could not get version info!")
   else()
-    # Strip the quotes off :-(
-    string(LENGTH "${gitCmndOutput}" gitCmndOutputLen)
-    math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2")
-    string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep}
-      gitVersionLine)
+    set(gitVersionLine "${gitCmndOutput}")
   endif()
 
   # B) Get the first 80 chars of the summary message for more info
 
   execute_process(
-    COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%s"
+    COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:%s
     WORKING_DIRECTORY ${gitRepoDir}
     RESULT_VARIABLE gitCmndRtn
     OUTPUT_VARIABLE gitCmndOutput
@@ -158,25 +144,16 @@ function(tribits_generate_single_repo_version_string  gitRepoDir
       " for extra repo ${gitRepoDir}!")
     set(gitSummaryStr "Error, could not get version summary!")
   else()
-    # Strip off quotes and quote the 80 char string
     set(maxSummaryLen 80)
-    math(EXPR maxSummaryLen_PLUS_2 "${maxSummaryLen}+2")
-    string(LENGTH "${gitCmndOutput}" gitCmndOutputLen)
-    math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2")
-    string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep}
-      gitCmndOutputStripped)
-    if (gitCmndOutputLen GREATER ${maxSummaryLen_PLUS_2})
-      string(SUBSTRING "${gitCmndOutputStripped}" 0 ${maxSummaryLen}
-         gitSummaryStr)
-    else()
-      set(gitSummaryStr "${gitCmndOutputStripped}")
-    endif()
+    string(SUBSTRING "${gitCmndOutput}" 0 ${maxSummaryLen} gitSummaryStr)
   endif()
 
   set(${repoVersionStringOut}
     "${gitVersionLine}\n${gitSummaryStr}" PARENT_SCOPE)
 
 endfunction()
+# NOTE: Above, it is fine if ${maxSummaryLen} > len(${gitCmndOutput}) as
+# string(SUBSTRING ...) will just shorten this to the lenght of the string.
 
 
 function(tribits_assert_git_executable)

tribits/core/utils/TribitsParseArgumentsHelpers.cmake

@@ -84,7 +84,7 @@ endfunction()
 
 # @MACRO: tribits_assert_parse_arg_one_or_more_values()
 #
-# Assert that a set of parse argument have at least one value
+# Assert that a set of parse arguments have at least one value
 #
 # Usage::
 #

tribits/core/utils/UnitTestHelpers.cmake

@@ -40,6 +40,8 @@
 include(CMakeParseArguments)
 include(GlobalSet)
 
+cmake_policy(SET CMP0007 NEW)
+
 
 # @MACRO: unittest_initialize_vars()
 #
@@ -159,11 +161,6 @@ endfunction()
 #
 function(unittest_string_block_compare  stringVar  stringExpected)
 
-  # Don't ignore empty elements in list() operations (they are very important
-  # here)
-  cmake_policy(PUSH)
-  cmake_policy(SET CMP0007 NEW)
-
   message("\nCheck: ${stringVar} equals expected string:")
 
   math( EXPR NUMRUN ${UNITTEST_OVERALL_NUMRUN}+1 )
@@ -233,8 +230,6 @@ function(unittest_string_block_compare  stringVar  stringExpected)
     global_set(UNITTEST_OVERALL_NUMPASSED ${NUMPASSED})
   endif()
 
-  cmake_policy(POP)
-
 endfunction()
 
 

tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake

@@ -458,8 +458,8 @@ function(tribits_get_cdash_site_from_drop_site_and_location)
   tribits_assert_parse_arg_one_value(PREFIX  CTEST_DROP_LOCATION)
   tribits_assert_parse_arg_one_value(PREFIX  CDASH_SITE_URL_OUT)
   # Get the full CDash site from parts
-  string(FIND "${PREFIX_CTEST_DROP_LOCATION}" "?" endOfSubmitPhpIdx)
-  string(SUBSTRING "${PREFIX_CTEST_DROP_LOCATION}" 0 ${endOfSubmitPhpIdx} submitPhpPart)
+  string(FIND "${PREFIX_CTEST_DROP_LOCATION}" "?" beginningOfQueryString)
+  string(SUBSTRING "${PREFIX_CTEST_DROP_LOCATION}" 0 ${beginningOfQueryString} submitPhpPart)
   string(REPLACE "/submit.php" "" endCDashUrl "${submitPhpPart}")
   set(cdashSiteUrl "${PREFIX_CTEST_DROP_SITE}${endCDashUrl}")
   set(${PREFIX_CDASH_SITE_URL_OUT} "https://${cdashSiteUrl}" PARENT_SCOPE)

tribits/ctest_driver/TribitsReadTagFile.cmake

@@ -1,3 +1,6 @@
+cmake_policy(SET CMP0007 NEW)
+
+
 # @FUNCTION: tribits_read_ctest_tag_file()
 #
 # Read in the <build>/Testing/TAG file contents
@@ -10,7 +13,6 @@
 function(tribits_read_ctest_tag_file  tagFileIn
     buildStartTimeOut  cdashGroupOut  cdashModelOut
   )
-  cmake_policy(SET CMP0007 NEW)
   file(READ "${tagFileIn}" tagFileStr)
   string(REPLACE "\n" ";" tagFileStrList "${tagFileStr}")
   list(GET tagFileStrList 0 buildStartTime)