Re: [cmake-developers] Perforce Patch for CTest
On 10/24/2013 03:11 AM, Rolf Eike Beer wrote:
CMake/Source/CTest/cmCTestP4.cxx: In member function ‘virtual void
cmCTestP4::LoadRevisions()’:
CMake/Source/CTest/cmCTestP4.cxx:469:32: warning: conversion to ‘int’
from ‘std::vectorstd::basic_stringchar ::size_type {aka long
unsigned int}’ may alter its value [-Wconversion]
Also std::size_t does not work on VS 6. Both fixed:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=b6fbd681
-Brad
--
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
Re: [cmake-developers] Perforce Patch for CTest
On 10/24/2013 08:17 AM, Brad King wrote: Also std::size_t does not work on VS 6. Both fixed: Also a couple fixes to the test: CTest.UpdateP4: Fix test when p4 client is not in PATH http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=ee51ec64eb CTest.UpdateP4: Run test in directory with space in its name http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9a752135 -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/24/2013 03:57 PM, Pedro Navarro wrote: That's right, p4 usually doesn't create a .p4 file I think it is fine for the test to not place .p4 files since that is representative of real use cases. Ideally each VCS implementation could have a way to tell CTest if a directory it's under that specific VCS control or not, that way we could execute whatever p4 commands we wanted inside cmCTestP4 and return true if the source directory is being managed by p4. I think that would be a great way to refactor DetectVCS. It would avoid putting VCS-tool-specific knowledge (the directory name) in the main class. The existing classes for other tools would just look for its control directory and P4 could run the tool. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
Thanks a lot! Glad we could contribute. What's the process for changes? :) Adding support for P4Web in CDash made me realize that maybe we need to change how revisions are reported in Update.xml (file revisions instead of changelists), I'm looking into that right now. Pedro On Wed, Oct 23, 2013 at 1:49 PM, Brad King brad.k...@kitware.com wrote: On 10/22/2013 06:11 PM, Pedro Navarro wrote: Good point :) Here's the new patch. Great, thanks! I've applied it: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=c63ecee3 with minor tweaks: * Updated commit author date to when you sent the email * Wrote more extensive commit message * Marked P4_EXECUTABLE as an advanced cache option * Simplified the cygwin exclusion logic for adding the test * Quoted the p4d shell command args to support spaces in path I also downloaded the p4 and p4d binaries and put them on the hythloth.kitware dashboard machine so they should run as part of its Linux64-gnu test tonight. I used them to run the test locally and it works for me :) Great work, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/22/2013 02:51 PM, Pedro Navarro wrote:
get_filename_component(TOP ${CMAKE_CURRENT_LIST_FILE} PATH) could be
written
as CMAKE_CURRENT_LIST_DIR. I don't remember exactly when it was introduced,
but you drive that test with the newly built CMake so this must work. And a
newline is missing at the end of that file.
Would that be preferred? I have no issues using it but that's what the other
tests were using, so I wanted them to be as similar as possible.
Yes. The other tests' code predates CMAKE_CURRENT_LIST_DIR.
I would accept the patch for the new test either way though.
Thanks,
-Brad
--
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
Re: [cmake-developers] Perforce Patch for CTest
Am Dienstag, 22. Oktober 2013, 11:51:52 schrieb Pedro Navarro: I would extend the regex in the DiffParser constructor to contain - at the end to make the propability that a filename with # in it would cause issues smaller. Also I find the usage of both Options and options in SetP4Options() an invitation for confusion (and possible errors). I'll change the regex. Although I verified it in debuggex to make sure we could correctly handle different combination of files with # and other characters in it, it makes sense to add the additional check. About the options, do you mean not using P4Options when doing an update if options for the update commands are given? I have to admit that I don't fully understand the difference between Update Options and VCS Specific options (I got that code from the GIT implementation) but Perforce has the notion of global and command specific flags. In this case, the global flags are the P4Options which are used to specify how do you connect to the server (host, port, username) and which client view to use, the other options are the flags you pass for the update command, which are completely different. I agree it can be confusing I meant that you have one variable called Options and one called options. This is just asking for trouble IMHO. Even more as both are of the same type. Eike signature.asc Description: This is a digitally signed message part. -- 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
Re: [cmake-developers] Perforce Patch for CTest
Oh, I see. Sure, I can see how that could become easily a hard to debug problem. On Tue, Oct 22, 2013 at 12:08 PM, Rolf Eike Beer e...@sf-mail.de wrote: Am Dienstag, 22. Oktober 2013, 11:51:52 schrieb Pedro Navarro: I would extend the regex in the DiffParser constructor to contain - at the end to make the propability that a filename with # in it would cause issues smaller. Also I find the usage of both Options and options in SetP4Options() an invitation for confusion (and possible errors). I'll change the regex. Although I verified it in debuggex to make sure we could correctly handle different combination of files with # and other characters in it, it makes sense to add the additional check. About the options, do you mean not using P4Options when doing an update if options for the update commands are given? I have to admit that I don't fully understand the difference between Update Options and VCS Specific options (I got that code from the GIT implementation) but Perforce has the notion of global and command specific flags. In this case, the global flags are the P4Options which are used to specify how do you connect to the server (host, port, username) and which client view to use, the other options are the flags you pass for the update command, which are completely different. I agree it can be confusing I meant that you have one variable called Options and one called options. This is just asking for trouble IMHO. Even more as both are of the same type. Eike -- 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
Re: [cmake-developers] Perforce Patch for CTest
Attached is the new patch with Eike's recommendations.
Pedro
From d291fde15c8e94ec5c9b93f96714554acf706d02 Mon Sep 17 00:00:00 2001
From: Pedro Navarro pnava...@netflix.com
Date: Tue, 1 Oct 2013 18:49:47 -0700
Subject: [PATCH] Perforce support for CTest
---
Modules/CTest.cmake |5 +
Modules/DartConfiguration.tcl.in |7 +
Source/CMakeLists.txt |2 +
Source/CTest/cmCTestP4.cxx| 569 +
Source/CTest/cmCTestP4.h | 71
Source/CTest/cmCTestUpdateCommand.cxx |8 +
Source/CTest/cmCTestUpdateHandler.cxx | 26 +-
Source/CTest/cmCTestUpdateHandler.h |1 +
Tests/CMakeLists.txt | 23 ++
Tests/CTestUpdateCommon.cmake |2 +-
Tests/CTestUpdateP4.cmake.in | 263 +++
11 files changed, 975 insertions(+), 2 deletions(-)
create mode 100644 Source/CTest/cmCTestP4.cxx
create mode 100644 Source/CTest/cmCTestP4.h
create mode 100644 Tests/CTestUpdateP4.cmake.in
diff --git a/Modules/CTest.cmake b/Modules/CTest.cmake
index 643cd29..ada8655 100644
--- a/Modules/CTest.cmake
+++ b/Modules/CTest.cmake
@@ -149,6 +149,7 @@ if(BUILD_TESTING)
find_program(BZRCOMMAND bzr)
find_program(HGCOMMAND hg)
find_program(GITCOMMAND git)
+ find_program(P4COMMAND p4)
if(NOT UPDATE_TYPE)
if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/CVS)
@@ -180,6 +181,9 @@ if(BUILD_TESTING)
elseif(${_update_type} STREQUAL git)
set(UPDATE_COMMAND ${GITCOMMAND})
set(UPDATE_OPTIONS ${GIT_UPDATE_OPTIONS})
+ elseif(${_update_type} STREQUAL p4)
+set(UPDATE_COMMAND ${P4COMMAND})
+set(UPDATE_OPTIONS ${P4_UPDATE_OPTIONS})
endif()
set(DART_TESTING_TIMEOUT 1500 CACHE STRING
@@ -275,6 +279,7 @@ if(BUILD_TESTING)
CVS_UPDATE_OPTIONS
DART_TESTING_TIMEOUT
GITCOMMAND
+P4COMMAND
HGCOMMAND
MAKECOMMAND
MEMORYCHECK_COMMAND
diff --git a/Modules/DartConfiguration.tcl.in b/Modules/DartConfiguration.tcl.in
index 9e49ac7..68fadf6 100644
--- a/Modules/DartConfiguration.tcl.in
+++ b/Modules/DartConfiguration.tcl.in
@@ -52,6 +52,13 @@ GITCommand: @GITCOMMAND@
GITUpdateOptions: @GIT_UPDATE_OPTIONS@
GITUpdateCustom: @CTEST_GIT_UPDATE_CUSTOM@
+# Perforce options
+P4Command: @P4COMMAND@
+P4Client: @CTEST_P4_CLIENT@
+P4Options: @CTEST_P4_OPTIONS@
+P4UpdateOptions: @CTEST_P4_UPDATE_OPTIONS@
+P4UpdateCustom: @CTEST_P4_UPDATE_CUSTOM@
+
# Generic update command
UpdateCommand: @UPDATE_COMMAND@
UpdateOptions: @UPDATE_OPTIONS@
diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 01e4f88..fd97a20 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -465,6 +465,8 @@ set(CTEST_SRCS cmCTest.cxx
CTest/cmCTestGIT.h
CTest/cmCTestHG.cxx
CTest/cmCTestHG.h
+ CTest/cmCTestP4.cxx
+ CTest/cmCTestP4.h
)
# Build CTestLib
diff --git a/Source/CTest/cmCTestP4.cxx b/Source/CTest/cmCTestP4.cxx
new file mode 100644
index 000..ac33997
--- /dev/null
+++ b/Source/CTest/cmCTestP4.cxx
@@ -0,0 +1,569 @@
+/*
+ CMake - Cross Platform Makefile Generator
+ Copyright 2000-2013 Kitware, Inc.
+
+ Distributed under the OSI-approved BSD License (the License);
+ see accompanying file Copyright.txt for details.
+
+ This software is distributed WITHOUT ANY WARRANTY; without even the
+ implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ See the License for more information.
+*/
+#include cmCTestP4.h
+
+#include cmCTest.h
+#include cmSystemTools.h
+#include cmXMLSafe.h
+
+#include cmsys/RegularExpression.hxx
+#include cmsys/ios/sstream
+#include cmsys/Process.h
+
+#include sys/types.h
+#include time.h
+#include ctype.h
+
+//
+cmCTestP4::cmCTestP4(cmCTest* ct, std::ostream log):
+ cmCTestGlobalVC(ct, log)
+{
+ this-PriorRev = this-Unknown;
+}
+
+//
+cmCTestP4::~cmCTestP4()
+{
+}
+
+//
+class cmCTestP4::IdentifyParser: public cmCTestVC::LineParser
+{
+public:
+ IdentifyParser(cmCTestP4* p4, const char* prefix,
+ std::string rev): Rev(rev)
+{
+this-SetLog(p4-Log, prefix);
+this-RegexIdentify.compile(^Change ([0-9]+) on);
+}
+private:
+ std::string Rev;
+ cmsys::RegularExpression RegexIdentify;
+
+ bool ProcessLine()
+{
+if(this-RegexIdentify.find(this-Line))
+ {
+ this-Rev = this-RegexIdentify.match(1);
+ return false;
+ }
+return true;
+}
+};
+
+//
+class cmCTestP4::ChangesParser: public cmCTestVC::LineParser
+{
+public:
+ ChangesParser(cmCTestP4* p4, const char* prefix) : P4(p4)
+{
+
Re: [cmake-developers] Perforce Patch for CTest
Good point :) Here's the new patch. GetWorkingRevision returns 0 instead
of empty now, eliminating the need to do the check in NoteOldRevision and
NoteNewRevision
Pedro
On Tue, Oct 22, 2013 at 2:49 PM, Rolf Eike Beer e...@sf-mail.de wrote:
Am Dienstag, 22. Oktober 2013, 14:36:36 schrieb Pedro Navarro:
Attached is the new patch with Eike's recommendations.
One more nitpick ;)
+ this-OldRevision = this-GetWorkingRevision();
+ if(this-OldRevision.empty())
+{
+this-OldRevision = 0;
+}
That code comes twice, for the only 2 callers of GetWorkingRevision(). I
would
say just modify that method to return 0 in that case.
Eike
From d2cb109754699cbc82ba4334f448f41028642adc Mon Sep 17 00:00:00 2001
From: Pedro Navarro pnava...@netflix.com
Date: Tue, 1 Oct 2013 18:49:47 -0700
Subject: [PATCH] Perforce support for CTest
---
Modules/CTest.cmake |5 +
Modules/DartConfiguration.tcl.in |7 +
Source/CMakeLists.txt |2 +
Source/CTest/cmCTestP4.cxx| 569 +
Source/CTest/cmCTestP4.h | 71
Source/CTest/cmCTestUpdateCommand.cxx |8 +
Source/CTest/cmCTestUpdateHandler.cxx | 26 +-
Source/CTest/cmCTestUpdateHandler.h |1 +
Tests/CMakeLists.txt | 23 ++
Tests/CTestUpdateCommon.cmake |2 +-
Tests/CTestUpdateP4.cmake.in | 263 +++
11 files changed, 975 insertions(+), 2 deletions(-)
create mode 100644 Source/CTest/cmCTestP4.cxx
create mode 100644 Source/CTest/cmCTestP4.h
create mode 100644 Tests/CTestUpdateP4.cmake.in
diff --git a/Modules/CTest.cmake b/Modules/CTest.cmake
index 643cd29..ada8655 100644
--- a/Modules/CTest.cmake
+++ b/Modules/CTest.cmake
@@ -149,6 +149,7 @@ if(BUILD_TESTING)
find_program(BZRCOMMAND bzr)
find_program(HGCOMMAND hg)
find_program(GITCOMMAND git)
+ find_program(P4COMMAND p4)
if(NOT UPDATE_TYPE)
if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/CVS)
@@ -180,6 +181,9 @@ if(BUILD_TESTING)
elseif(${_update_type} STREQUAL git)
set(UPDATE_COMMAND ${GITCOMMAND})
set(UPDATE_OPTIONS ${GIT_UPDATE_OPTIONS})
+ elseif(${_update_type} STREQUAL p4)
+set(UPDATE_COMMAND ${P4COMMAND})
+set(UPDATE_OPTIONS ${P4_UPDATE_OPTIONS})
endif()
set(DART_TESTING_TIMEOUT 1500 CACHE STRING
@@ -275,6 +279,7 @@ if(BUILD_TESTING)
CVS_UPDATE_OPTIONS
DART_TESTING_TIMEOUT
GITCOMMAND
+P4COMMAND
HGCOMMAND
MAKECOMMAND
MEMORYCHECK_COMMAND
diff --git a/Modules/DartConfiguration.tcl.in b/Modules/DartConfiguration.tcl.in
index 9e49ac7..68fadf6 100644
--- a/Modules/DartConfiguration.tcl.in
+++ b/Modules/DartConfiguration.tcl.in
@@ -52,6 +52,13 @@ GITCommand: @GITCOMMAND@
GITUpdateOptions: @GIT_UPDATE_OPTIONS@
GITUpdateCustom: @CTEST_GIT_UPDATE_CUSTOM@
+# Perforce options
+P4Command: @P4COMMAND@
+P4Client: @CTEST_P4_CLIENT@
+P4Options: @CTEST_P4_OPTIONS@
+P4UpdateOptions: @CTEST_P4_UPDATE_OPTIONS@
+P4UpdateCustom: @CTEST_P4_UPDATE_CUSTOM@
+
# Generic update command
UpdateCommand: @UPDATE_COMMAND@
UpdateOptions: @UPDATE_OPTIONS@
diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 01e4f88..fd97a20 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -465,6 +465,8 @@ set(CTEST_SRCS cmCTest.cxx
CTest/cmCTestGIT.h
CTest/cmCTestHG.cxx
CTest/cmCTestHG.h
+ CTest/cmCTestP4.cxx
+ CTest/cmCTestP4.h
)
# Build CTestLib
diff --git a/Source/CTest/cmCTestP4.cxx b/Source/CTest/cmCTestP4.cxx
new file mode 100644
index 000..32e8486
--- /dev/null
+++ b/Source/CTest/cmCTestP4.cxx
@@ -0,0 +1,569 @@
+/*
+ CMake - Cross Platform Makefile Generator
+ Copyright 2000-2013 Kitware, Inc.
+
+ Distributed under the OSI-approved BSD License (the License);
+ see accompanying file Copyright.txt for details.
+
+ This software is distributed WITHOUT ANY WARRANTY; without even the
+ implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ See the License for more information.
+*/
+#include cmCTestP4.h
+
+#include cmCTest.h
+#include cmSystemTools.h
+#include cmXMLSafe.h
+
+#include cmsys/RegularExpression.hxx
+#include cmsys/ios/sstream
+#include cmsys/Process.h
+
+#include sys/types.h
+#include time.h
+#include ctype.h
+
+//
+cmCTestP4::cmCTestP4(cmCTest* ct, std::ostream log):
+ cmCTestGlobalVC(ct, log)
+{
+ this-PriorRev = this-Unknown;
+}
+
+//
+cmCTestP4::~cmCTestP4()
+{
+}
+
+//
+class cmCTestP4::IdentifyParser: public cmCTestVC::LineParser
+{
+public:
+ IdentifyParser(cmCTestP4* p4, const char* prefix,
+ std::string rev):
Re: [cmake-developers] Perforce Patch for CTest
Attached is the latest version of the Perforce support patch for CTest. I've added a test (CTest.UpdateP4) that launches a Perforce server listening on a custom port and performs the same operations as other VCS tools. Some release notes: - Unix is expected. Windows could work (it's a matter of changing how the Perforce service is started) but I have no Windows machines handy to add support for it. - The Perforce p4 and p4d utilities must be installed. find_program() will be used to locate them - p4d will be started and a new database will be created at the beginning of each test run, so we will always test against a fresh and new repository. - Based on the tests, I modified how the P4 CTest client reports its modified paths, so now relative paths to the root are returned (without the depot name). - As I had a server (p4d) I was able to play succesfully with message localization so now I set the messages language to English before each command is executed, preventing problems with the results parsing if somebody ever configures Perforce in another language. - Fixed little cosmetic things here and there as a result of having it running in house for a couple of weeks. Pedro On Wed, Oct 16, 2013 at 1:10 PM, Brad King brad.k...@kitware.com wrote: On 10/16/2013 03:11 PM, Pedro Navarro wrote: I was thinking that as the test requires the p4 tool to be installed, we might as well require also p4d (the server, which is now free for up to 20 users). In that case the test can bring up a local server and work against it which, in the end, will create less issues as the p4 database will be deleted when the test ends. If we work against a real production server we might not be able to use p4 obliterate and we will be leaving those temporary checkins the test does polluting the history, and that might not be desired. We definitely need the test to run against a fresh repository each time. The other VCS tool tests all create local repos from scratch when they run. Whatever you need to require to achieve this with P4 is acceptable. Thanks, -Brad 0001-Perforce-support-for-CTest.patch Description: Binary data -- 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
Re: [cmake-developers] Perforce Patch for CTest
Ok, I have the patch working and I'm going to send it soon. One question, is it possible to launch a detached process from within CMake? If I use EXECUTE_PROCESS to start p4d, CMake waits for it to finish so -for now- I'm launching the Perforce daemon outside CMake. Pedro On Wed, Oct 16, 2013 at 1:10 PM, Brad King brad.k...@kitware.com wrote: On 10/16/2013 03:11 PM, Pedro Navarro wrote: I was thinking that as the test requires the p4 tool to be installed, we might as well require also p4d (the server, which is now free for up to 20 users). In that case the test can bring up a local server and work against it which, in the end, will create less issues as the p4 database will be deleted when the test ends. If we work against a real production server we might not be able to use p4 obliterate and we will be leaving those temporary checkins the test does polluting the history, and that might not be desired. We definitely need the test to run against a fresh repository each time. The other VCS tool tests all create local repos from scratch when they run. Whatever you need to require to achieve this with P4 is acceptable. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/17/2013 1:52 PM, Pedro Navarro wrote: Ok, I have the patch working and I'm going to send it soon. One question, is it possible to launch a detached process from within CMake? If I use EXECUTE_PROCESS to start p4d, CMake waits for it to finish so -for now- I'm launching the Perforce daemon outside CMake. Pedro It is not possible to do this. One work around is to put the start of the process in a shell script or bat file that is used to run the ctest script which is basically what you are doing. :) -Bill -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/17/2013 02:18 PM, Bill Hoffman wrote: On 10/17/2013 1:52 PM, Pedro Navarro wrote: Ok, I have the patch working and I'm going to send it soon. One question, is it possible to launch a detached process from within CMake? If I use EXECUTE_PROCESS to start p4d, CMake waits for it to finish so -for now- I'm launching the Perforce daemon outside CMake. It is not possible to do this. One work around is to put the start of the process in a shell script or bat file that is used to run the ctest script which is basically what you are doing. :) If this is restricted to a UNIX-like environment one can use a shell script that internally executes a background job, e.g. execute_process(COMMAND sh -c nohup p4d /dev/null 21 ) Actually the internal KWSys Process library used to implement the execute_process command does know how to create and disown daemon processes. If necessary one could teach execute_process options to trigger this behavior. In either case there is no clear way to terminate the daemon when the test is done. -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
I experimented with nohup but from inside a shell script that was being executed from execute_process, but that didn't work. I'll give that a try, thanks! There happens to be a way to cleanly shutdown perforce, executing p4 admin stop, so this could mean that we can start p4d if it's present and on Unix and exit and remote the database when the test finishes, which would be the preferred approach. Pedro On Thu, Oct 17, 2013 at 11:44 AM, Brad King brad.k...@kitware.com wrote: On 10/17/2013 02:18 PM, Bill Hoffman wrote: On 10/17/2013 1:52 PM, Pedro Navarro wrote: Ok, I have the patch working and I'm going to send it soon. One question, is it possible to launch a detached process from within CMake? If I use EXECUTE_PROCESS to start p4d, CMake waits for it to finish so -for now- I'm launching the Perforce daemon outside CMake. It is not possible to do this. One work around is to put the start of the process in a shell script or bat file that is used to run the ctest script which is basically what you are doing. :) If this is restricted to a UNIX-like environment one can use a shell script that internally executes a background job, e.g. execute_process(COMMAND sh -c nohup p4d /dev/null 21 ) Actually the internal KWSys Process library used to implement the execute_process command does know how to create and disown daemon processes. If necessary one could teach execute_process options to trigger this behavior. In either case there is no clear way to terminate the daemon when the test is done. -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/17/2013 03:23 PM, Pedro Navarro wrote: I experimented with nohup but from inside a shell script that was being executed from execute_process, but that didn't work. You also need to disconnect the stdout/stderr pipes by redirecting them to /dev/null. Then execute_process can fully let go because the pipes it uses to communicate with the child will be closed when the shell exits. -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/17/2013 4:59 PM, Brad King wrote: As I explained in a sibling response the KWSys Process library has support for creating detached processes. It should only be a matter of exposing that through execute_process command options. +1, this does come up a lot. It would be good to have this exposed in the API. -Bill -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 2013-10-17 16:59, Brad King wrote: On 10/17/2013 04:56 PM, Rolf Eike Beer wrote: We should think if this should be something that is needed. Running some sort of background process is a common pattern for all sorts of tests. Often really detaching is not needed, It is usually sufficient to have one process running in the background while the tests runs. As I explained in a sibling response the KWSys Process library has support for creating detached processes. It should only be a matter of exposing that through execute_process command options. Probably not a bad idea. Does KWSys get the resulting PID? If yes, it might be good to have a way to pass that back into CMake in order to also add a kill_process command at the same time in order to have a general way to stop processes launched in the background. (And/or a wait_process, etc.) Something to keep in mind... -- Matthew -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/15/2013 07:39 PM, Pedro Navarro wrote: That's exactly my point. Is it ok to rely on environment variables (P4PORT, P4HOST), ie, expect the build machine to be set up in a way before running the tests or are those values passed in some kind of configuration file, added to the CMake Cache or passed as -D? For now I think it is okay to read those. The surrounding dashboard client script can set up the environment accordingly. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
Sounds good. Those environment variables are read automatically by Perforce's command line tools, so there's nothing to do on the test's end then. Pedro On Wed, Oct 16, 2013 at 5:23 AM, Brad King brad.k...@kitware.com wrote: On 10/15/2013 07:39 PM, Pedro Navarro wrote: That's exactly my point. Is it ok to rely on environment variables (P4PORT, P4HOST), ie, expect the build machine to be set up in a way before running the tests or are those values passed in some kind of configuration file, added to the CMake Cache or passed as -D? For now I think it is okay to read those. The surrounding dashboard client script can set up the environment accordingly. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
Hi! I have the test succesfully running using the provided framework for CTestUpdate, so I have a new CTest.UpdateP4 test. Right now it runs against a local p4d server instance that you can download for free from Perforce. I'm imagining that, depending on the testing machine, we might need to be able to specify where the perforce server is located, if it's not the local machine (using either a command line switch or an environment variable). Is there any infrastructure for that or will we assume that the machine running the tests will have a local p4d running? Should we assume that p4d is running or should I start it as part of the test? Thanks, Pedro Navarro On Mon, Oct 14, 2013 at 12:00 PM, Brad King brad.k...@kitware.com wrote: On 10/14/2013 2:41 PM, Pedro Navarro wrote: I'm developing the test, working against Perforce's free 20 user server. What's the procedure build and run the test from a CMake source build? In the build tree run bin/ctest -R CTest.UpdateP4 assuming you've modified Tests/CMakeLists.txt appropriately to add the test. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 2013-10-15 18:44, Pedro Navarro wrote: I'm imagining that, depending on the testing machine, we might need to be able to specify where the perforce server is located, if it's not the local machine (using either a command line switch or an environment variable). Is there any infrastructure for that? (IIRC) $P4PORT? IIRC perforce already has a convention for using environment variables to locate the server; you shouldn't need to invent something new. -- Matthew -- 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
Re: [cmake-developers] Perforce Patch for CTest
I'm developing the test, working against Perforce's free 20 user server. What's the procedure build and run the test from a CMake source build? Thanks! PEdro On Thu, Oct 10, 2013 at 1:40 PM, Pedro Navarro pnava...@netflix.com wrote: I saw that and I thought that was meant for Eike. Ok I'll work on the test cases and I'll see what I can do about the nightly dashboard submissions. Pedro On Thu, Oct 10, 2013 at 1:14 PM, Brad King brad.k...@kitware.com wrote: On 10/10/2013 04:01 PM, Pedro Navarro wrote: Hi guys, just checking: am I expected to do anything else with the P4 patch or is it now in your hands and will eventually be integrated? As requested here: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/8085/focus=8098 the patch needs to include a new Tests/CTestUpdateP4.cmake.in and related changes to add the test case. Also since none of the upstream nightly test machines has Perforce installed we'll need you to run a nightly CMake dashboard submission on a machine that does. Without this testing we cannot maintain the P4 support. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/14/2013 2:41 PM, Pedro Navarro wrote: I'm developing the test, working against Perforce's free 20 user server. What's the procedure build and run the test from a CMake source build? In the build tree run bin/ctest -R CTest.UpdateP4 assuming you've modified Tests/CMakeLists.txt appropriately to add the test. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
On 10/10/2013 04:01 PM, Pedro Navarro wrote: Hi guys, just checking: am I expected to do anything else with the P4 patch or is it now in your hands and will eventually be integrated? As requested here: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/8085/focus=8098 the patch needs to include a new Tests/CTestUpdateP4.cmake.in and related changes to add the test case. Also since none of the upstream nightly test machines has Perforce installed we'll need you to run a nightly CMake dashboard submission on a machine that does. Without this testing we cannot maintain the P4 support. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce Patch for CTest
I saw that and I thought that was meant for Eike. Ok I'll work on the test cases and I'll see what I can do about the nightly dashboard submissions. Pedro On Thu, Oct 10, 2013 at 1:14 PM, Brad King brad.k...@kitware.com wrote: On 10/10/2013 04:01 PM, Pedro Navarro wrote: Hi guys, just checking: am I expected to do anything else with the P4 patch or is it now in your hands and will eventually be integrated? As requested here: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/8085/focus=8098 the patch needs to include a new Tests/CTestUpdateP4.cmake.in and related changes to add the test case. Also since none of the upstream nightly test machines has Perforce installed we'll need you to run a nightly CMake dashboard submission on a machine that does. Without this testing we cannot maintain the P4 support. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce patch for CTest
On 10/02/2013 06:00 PM, Pedro Navarro wrote: Attached is a new patch with Rolf's suggestions (improved regex and usage of size_type to go through the Options vector). Thanks. (BTW, he goes by Eike.) I just amended my commit and created a new patch file, instead of doing two smaller ones. That is preferred, thanks. Eike also asked: On 10/02/2013 04:19 PM, Rolf Eike Beer wrote: And of course this needs testcases, and a host that has Perforce installed and drives those tests. ;) This will mean adding a test like those in Tests/CTestUpdate*.cmake.in, and ideally running a nightly dashboard submission on a machine with the proper tools installed to enable the new test. See here to run a nightly submission: http://www.cmake.org/Wiki/CMake/Git/Dashboard Of course the actual running of this test won't happen until the change is integrated. Thanks, -Brad -- 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
Re: [cmake-developers] Perforce patch for CTest
We deployed CTest today on our production machine and found an off by one error when describing changelists. I've updated the patch and attached it to this mail. Other than that, we have ctest running here with no problems! Pedro On Thu, Oct 3, 2013 at 10:19 AM, Brad King brad.k...@kitware.com wrote: On 10/02/2013 06:00 PM, Pedro Navarro wrote: Attached is a new patch with Rolf's suggestions (improved regex and usage of size_type to go through the Options vector). Thanks. (BTW, he goes by Eike.) I just amended my commit and created a new patch file, instead of doing two smaller ones. That is preferred, thanks. Eike also asked: On 10/02/2013 04:19 PM, Rolf Eike Beer wrote: And of course this needs testcases, and a host that has Perforce installed and drives those tests. ;) This will mean adding a test like those in Tests/CTestUpdate*.cmake.in, and ideally running a nightly dashboard submission on a machine with the proper tools installed to enable the new test. See here to run a nightly submission: http://www.cmake.org/Wiki/CMake/Git/Dashboard Of course the actual running of this test won't happen until the change is integrated. Thanks, -Brad -- 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 0001-Added-support-for-Perforce.patch Description: Binary data -- 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
Re: [cmake-developers] Perforce patch for CTest
Am Mittwoch, 2. Oktober 2013, 13:10:20 schrieb Pedro Navarro: - It requires an English version of Perforce (use the P4_OPTIONS variable, documented below to pass the -L switch to change the message language if you installation has a different one). It shouldn't be too hard to change the regular expressions used for parsing to not key on a specific word but on character ranges. I'll look into that if this becomes an issue. Maybe set P4_OPTIONS automatically to -Lenglish (or whatever) in case it is not set? I tried that originally but had no effect. Also, on the perforce documentation it says that Specifies the language to use for error messages from the Perforce service. In order for this flag to work, your administrator must have loaded support for non-English messages in the database, so I didn't feel comfortable adding a flag globably without knowing it might even trigger an error (like invalid language or something like that). Perforce offers the P4LANGUAGE environment variable which can be used to configure the client for updates, and we also have the P4_OPTIONS variable, so I think for now it's a good solution/workaround until we have more information. Can you set P4LANGUANGE unconditionally then? Like it is done in FindSubversion.cmake to get proper output. And of course this needs testcases, and a host that has Perforce installed and drives those tests. ;) Eike signature.asc Description: This is a digitally signed message part. -- 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
Re: [cmake-developers] Perforce patch for CTest
The issue with the P4LANGUAGE environment variable, as well as the -L switch is that I don't know the language string I need to set it to. Another thing is that Perforce says that it will change the text of the error messages, so it's not clear if the ouput I'm parsing will be affected or not. I guess we would need to set up a temporary P4 server and test all of this but, for now, we offer enough flexibility to let users set the language if they have a non-English Perforce. Pedro On Wed, Oct 2, 2013 at 1:19 PM, Rolf Eike Beer e...@sf-mail.de wrote: Am Mittwoch, 2. Oktober 2013, 13:10:20 schrieb Pedro Navarro: - It requires an English version of Perforce (use the P4_OPTIONS variable, documented below to pass the -L switch to change the message language if you installation has a different one). It shouldn't be too hard to change the regular expressions used for parsing to not key on a specific word but on character ranges. I'll look into that if this becomes an issue. Maybe set P4_OPTIONS automatically to -Lenglish (or whatever) in case it is not set? I tried that originally but had no effect. Also, on the perforce documentation it says that Specifies the language to use for error messages from the Perforce service. In order for this flag to work, your administrator must have loaded support for non-English messages in the database, so I didn't feel comfortable adding a flag globably without knowing it might even trigger an error (like invalid language or something like that). Perforce offers the P4LANGUAGE environment variable which can be used to configure the client for updates, and we also have the P4_OPTIONS variable, so I think for now it's a good solution/workaround until we have more information. Can you set P4LANGUANGE unconditionally then? Like it is done in FindSubversion.cmake to get proper output. And of course this needs testcases, and a host that has Perforce installed and drives those tests. ;) Eike -- 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 -- 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
Re: [cmake-developers] Perforce patch for CTest
Attached is a new patch with Rolf's suggestions (improved regex and usage of size_type to go through the Options vector). I just amended my commit and created a new patch file, instead of doing two smaller ones. Pedro On Wed, Oct 2, 2013 at 2:28 PM, Pedro Navarro pnava...@netflix.com wrote: The issue with the P4LANGUAGE environment variable, as well as the -L switch is that I don't know the language string I need to set it to. Another thing is that Perforce says that it will change the text of the error messages, so it's not clear if the ouput I'm parsing will be affected or not. I guess we would need to set up a temporary P4 server and test all of this but, for now, we offer enough flexibility to let users set the language if they have a non-English Perforce. Pedro On Wed, Oct 2, 2013 at 1:19 PM, Rolf Eike Beer e...@sf-mail.de wrote: Am Mittwoch, 2. Oktober 2013, 13:10:20 schrieb Pedro Navarro: - It requires an English version of Perforce (use the P4_OPTIONS variable, documented below to pass the -L switch to change the message language if you installation has a different one). It shouldn't be too hard to change the regular expressions used for parsing to not key on a specific word but on character ranges. I'll look into that if this becomes an issue. Maybe set P4_OPTIONS automatically to -Lenglish (or whatever) in case it is not set? I tried that originally but had no effect. Also, on the perforce documentation it says that Specifies the language to use for error messages from the Perforce service. In order for this flag to work, your administrator must have loaded support for non-English messages in the database, so I didn't feel comfortable adding a flag globably without knowing it might even trigger an error (like invalid language or something like that). Perforce offers the P4LANGUAGE environment variable which can be used to configure the client for updates, and we also have the P4_OPTIONS variable, so I think for now it's a good solution/workaround until we have more information. Can you set P4LANGUANGE unconditionally then? Like it is done in FindSubversion.cmake to get proper output. And of course this needs testcases, and a host that has Perforce installed and drives those tests. ;) Eike -- 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 0001-Added-support-for-Perforce.patch Description: Binary data -- 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
Re: [cmake-developers] Perforce patch for CTest 2.8
On 09/30/2013 08:36 PM, Pedro Navarro wrote: I've written a patch to add Perforce support for CTest 2.8. Although it still needs a little bit more work (I need to look into what I'm supposed to do with Nightly builds ) it properly notifies CTest of all details about check ins, revision numbers, authors and their E-Mail address (by matching them to the P4 database), local changes, etc... Great, thanks for working on this. What's the proper process to submit this? Please use git format-patch -M origin/master.. to produce a patch series and post it here. What documentation needs to be written (I've added some useful CMake variables like P4_CLIENT or P4_OPTIONS for example)? Unfortunately we currently have no place for builtin CTest variable documentation: http://www.cmake.org/Bug/view.php?id=14246 We do have plans to add one but don't wait for that to contribute. Just put the documentation in comments next to where the variables are used so we can port it when we sweep through to collect CTest variable documentation later. Thanks, -Brad -- 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
