Hi, Attached is an updated version of the P4 patch.
Perforce always returns absolute file paths with the depot name (//depot/path/to/file.txt) and that was breaking the test infrastructure, which expects relative path names. Previously I was removing the depot name (//depot/) to make the test happy but I realized it was a bad idea now that we are looking at changelists from CDash using P4Web, as the depot needs to be added manually (which will break multi-depot repositories). The new version does not strip the depot name from the files, which are passed untouched to Update.xml (that should have been the right behavior all along). CTestUpdateCommon removes a given prefix (if defined in the REPOSITORY_FILE_PREFIX variable) after reading files from Update.xml to satisfy the checks. Pedro
From 1e5899d9c40a3f9f13ef7f4b52a089e6904fb9b0 Mon Sep 17 00:00:00 2001 From: Pedro Navarro <[email protected]> Date: Thu, 31 Oct 2013 13:24:57 -0700 Subject: [PATCH] Do not remove the depot name Instead of removing the depot name, which causes problems when looking at the file change list in CDash, make the Update test remove a given prefix from the files retrieved from Update.xml. --- Source/CTest/cmCTestP4.cxx | 12 +----------- Tests/CTestUpdateCommon.cmake | 9 +++++++++ Tests/CTestUpdateP4.cmake.in | 1 + 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Source/CTest/cmCTestP4.cxx b/Source/CTest/cmCTestP4.cxx index a504157..0058721 100644 --- a/Source/CTest/cmCTestP4.cxx +++ b/Source/CTest/cmCTestP4.cxx @@ -144,17 +144,7 @@ private: if(!this->Line.empty() && this->Line[0] == '=' && this->RegexDiff.find(this->Line)) { - std::string Path = this->RegexDiff.match(1); - // See if we need to remove the //depot prefix - if(Path.length() > 2 && Path[0] == '/' && Path[1] == '/') - { - size_t found = Path.find('/', 2); - if(found != std::string::npos) - { - Path = Path.substr(found + 1); - } - } - CurrentPath = Path; + CurrentPath = this->RegexDiff.match(1); AlreadyNotified = false; } else diff --git a/Tests/CTestUpdateCommon.cmake b/Tests/CTestUpdateCommon.cmake index ae8fda2..db4e08d 100644 --- a/Tests/CTestUpdateCommon.cmake +++ b/Tests/CTestUpdateCommon.cmake @@ -37,10 +37,19 @@ function(check_updates build) REGEX "<(${types}|FullName)>" LIMIT_INPUT ${max_update_xml_size} ) + string(REGEX REPLACE "[ \t]*<(${types})>[ \t]*;[ \t]*<FullName>([^<]*)</FullName>" "\\1{\\2}" UPDATE_XML_ENTRIES "${UPDATE_XML_ENTRIES}") + # If specified, remove the given prefix from the files in Update.xml. + # Some VCS systems, like Perforce, return absolute locations + if(DEFINED REPOSITORY_FILE_PREFIX) + string(REPLACE + "${REPOSITORY_FILE_PREFIX}" "" + UPDATE_XML_ENTRIES "${UPDATE_XML_ENTRIES}") + endif() + # Compare expected and actual entries set(EXTRA "${UPDATE_XML_ENTRIES}") list(REMOVE_ITEM EXTRA ${ARGN} ${UPDATE_EXTRA} ${UPDATE_MAYBE}) diff --git a/Tests/CTestUpdateP4.cmake.in b/Tests/CTestUpdateP4.cmake.in index f23bd11..f0420c4 100644 --- a/Tests/CTestUpdateP4.cmake.in +++ b/Tests/CTestUpdateP4.cmake.in @@ -8,6 +8,7 @@ set(P4_TOP "${TOP}") set(TOP "${TOP}/@CTestUpdateP4_DIR@") # Include code common to all update tests. +set(REPOSITORY_FILE_PREFIX "//ctest/") include("@CMAKE_CURRENT_SOURCE_DIR@/CTestUpdateCommon.cmake") #----------------------------------------------------------------------------- -- 1.7.10.4
-- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
