Re: [cmake-developers] Perforce Patch for CTest

[Pedro Navarro]

>
>
>
> BTW, the UpdateP4 test failed on our nightly build again because
> of timing issues with the busy system trying to start p4d.  I
> changed the way we wait for the server to start:
>
>  CTest.UpdateP4: Wait for p4d to start more robustly
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1cdee0ee
>
> Please try that out on your machine too.
>
>
Works fine for us!

Pedro
--

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

[Brad King]

On 10/24/2013 05:03 PM, Pedro Navarro wrote:
> Turns out that I could find a way to have P4Web do diffs based
> on changelists, so no changes to CTest where needed. I have
> P4Web now working on CDash. What's the process to submit CDash
> patches? I just signed up for its mailing list.

Great, thanks.  You can ask on their list to be sure but I think
a patch file will be fine.

BTW, the UpdateP4 test failed on our nightly build again because
of timing issues with the busy system trying to start p4d.  I
changed the way we wait for the server to start:

 CTest.UpdateP4: Wait for p4d to start more robustly
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1cdee0ee

Please try that out on your machine too.

> About the DetectVCS refactor, I'll take a stab at it!

Super.

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

[Pedro Navarro]

CDash now supports P4Web for viewing files and diffs and the daily changes
are read from a Perforce repository! I'll see what happens tonight, when
the nightly builds are run and will create a thread with the patches in the
CDash mailing list if succesful.

Pedro


On Thu, Oct 24, 2013 at 2:03 PM, Pedro Navarro  wrote:

> Turns out that I could find a way to have P4Web do diffs based on
> changelists, so no changes to CTest where needed. I have P4Web now working
> on CDash. What's the process to submit CDash patches? I just signed up for
> its mailing list.
>
> About the DetectVCS refactor, I'll take a stab at it!
>
> Pedro
>
>
> On Thu, Oct 24, 2013 at 1:01 PM, Brad King  wrote:
>
>> 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

[Pedro Navarro]

Turns out that I could find a way to have P4Web do diffs based on
changelists, so no changes to CTest where needed. I have P4Web now working
on CDash. What's the process to submit CDash patches? I just signed up for
its mailing list.

About the DetectVCS refactor, I'll take a stab at it!

Pedro


On Thu, Oct 24, 2013 at 1:01 PM, Brad King  wrote:

> 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

[Brad King]

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

[Pedro Navarro]

That's right, p4 usually doesn't create a .p4 file so there's no way to
tell by looking at the file system if a directory is under p4 control or
not. Some people set a P4CONFIG variable that points to a .p4 or .p4config
file and that's what DetectVCS looks for to identify the VCS as perforce.
The only way whould be to issue the p4 fstat  or p4 where 
to see if p4 replies with a success, but it didn't feel right to rely on
executing a third party executable from within CTest.

In the test it felt like "cheating" to create a zero length .p4 file -in
almost all p4 installations there won't be such a file- just so I could
trigger ctest's automatic detection so I had to set CTEST_UPDATE_COMMAND to
mark the source directory as being controlled by P4. Do you think there is
a better way? 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.

Pedro



On Thu, Oct 24, 2013 at 5:59 AM, Brad King  wrote:

> On 10/23/2013 08:43 PM, Pedro Navarro wrote:
> > 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.
>
> Okay.  Please attach the next patch here as before.
>
> Also, while fixing the CTest.UpdateP4 test to work without p4
> in the PATH I noticed that the reason the test needs to set
> CTEST_UPDATE_COMMAND/UpdateCommand instead of
> CTEST_P4_COMMAND/P4Command is because the test is not set up
> to trigger cmCTestUpdateHandler::DetectVCS's detection of
> .p4 or .p4client in the source tree.  Therefore CTest cannot
> tell from the source tree which VCS tool manages it.  I'm not
> a regular Perforce user, but is it common to not have any .p4*
> files in a checkout?  Should the test use them?
>
> 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

[Brad King]

On 10/23/2013 08:43 PM, Pedro Navarro wrote:
> 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.

Okay.  Please attach the next patch here as before.

Also, while fixing the CTest.UpdateP4 test to work without p4
in the PATH I noticed that the reason the test needs to set
CTEST_UPDATE_COMMAND/UpdateCommand instead of
CTEST_P4_COMMAND/P4Command is because the test is not set up
to trigger cmCTestUpdateHandler::DetectVCS's detection of
.p4 or .p4client in the source tree.  Therefore CTest cannot
tell from the source tree which VCS tool manages it.  I'm not
a regular Perforce user, but is it common to not have any .p4*
files in a checkout?  Should the test use them?

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

[Brad King]

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

[Brad King]

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::vector >::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

[Rolf Eike Beer]

Am 23.10.2013 22:49, schrieb Brad King:

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


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::vector >::size_type {aka long 
unsigned int}’ may alter its value [-Wconversion]


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

[Pedro Navarro]

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  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

[Brad King]

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

[Pedro Navarro]

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  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 
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 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+//
+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->Re

Re: [cmake-developers] Perforce Patch for CTest

[Rolf Eike Beer]

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

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

[Pedro Navarro]

Attached is the new patch with Eike's recommendations.

Pedro
From d291fde15c8e94ec5c9b93f96714554acf706d02 Mon Sep 17 00:00:00 2001
From: Pedro Navarro 
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 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+//
+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)
+{
+this->SetLog(&P4->Log, prefix);
+this->RegexIdentify.comp

Re: [cmake-developers] Perforce Patch for CTest

[Pedro Navarro]

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  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

[Rolf Eike Beer]

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

[Brad King]

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

[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



> Also I see a possible conflict between LoadRevisions() and
> NoteOldRevision().
> The latter sets OldRevision to "0" in case it can't load any, the former
> tests
> the variable for being empty. Given the right order of events this could
> result in different results.
>

You are absolutely right. That's a logic error. I made a last minute
change to report 0 if there was an error getting the revision number and
didn't update the other function. It's triggered only if there is an issue
connecting to the server, so the sync would fail anyway, and that's why the
unit test didn't catch it.



>
> 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.



>
> Otherwise, awesome work.
>

Thanks! And thanks for the detailed review.

Pedro

>
> 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

[Rolf Eike Beer]

Am Montag, 21. Oktober 2013, 17:56:36 schrieb Pedro Navarro:
> 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.

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).

Also I see a possible conflict between LoadRevisions() and NoteOldRevision(). 
The latter sets OldRevision to "0" in case it can't load any, the former tests 
the variable for being empty. Given the right order of events this could 
result in different results.

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.

Otherwise, awesome work.

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

[Pedro Navarro]

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  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

[Matthew Woehlke]

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

[Bill Hoffman]

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

[Brad King]

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.

-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

[Rolf Eike Beer]

Am Donnerstag, 17. Oktober 2013, 14:18:34 schrieb Bill Hoffman:
> 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.  :)

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.

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

[Brad King]

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

[Pedro Navarro]

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  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 2>&1 &")
>
> 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

[Brad King]

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 2>&1 &")

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

[Bill Hoffman]

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

[Pedro Navarro]

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  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

[Brad King]

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

[Pedro Navarro]

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.

Pedro


On Wed, Oct 16, 2013 at 8:08 AM, Pedro Navarro  wrote:

> 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  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

[Pedro Navarro]

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  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

[Brad King]

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

[Pedro Navarro]

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?

Pedro


On Tue, Oct 15, 2013 at 4:16 PM, Matthew Woehlke <
matthew.woeh...@kitware.com> wrote:

> 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
>
--

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

[Matthew Woehlke]

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

[Pedro Navarro]

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  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

[Brad King]

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

[Pedro Navarro]

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  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  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

[Pedro Navarro]

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  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

[Brad King]

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

[cmake-developers] Perforce Patch for CTest

[Pedro Navarro]

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?

Thanks!
Pedro
--

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

[Pedro Navarro]

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  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

[Brad King]

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

[Pedro Navarro]

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  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  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

[Pedro Navarro]

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  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

[Rolf Eike Beer]

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

[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.


> This will go wrong if the filename contains e.g. "#42". From what your
> examples
> look like this could be changed to " ([a-z]+)$" or if that doesn't file
> maybe "
> ([^ ]+)$".
>

Good point. I'm changing it to ([^ ]+)$ so it will match 


> +  void DoBodyLine()
> +{
> +if(this->Line[0] == '\t')
>
> Is it guaranteed that Line is never empty? Otherwise you should test !this-
> >Line.empty() before accessing it.
>

Yes, it's guaranteed to be not empty because an empty line would trigger a
section change.

>
> +  unsigned int i;
> +  for(i=0; i
> This could cause truncation warnings, use size_t better Options::size_type
> instead. Or use an iterator.
>

Sounds good, will do that.

Thanks for your comments.

Pedro
--

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

[Rolf Eike Beer]

Am Mittwoch, 2. Oktober 2013, 12:00:18 schrieb Pedro Navarro:
> Hi,
> 
> Attached is a patch to add Perforce support to CTest. I searched for
> instances of GIT in the CTest code and added the equivalent P4
> functionality. Hopefully I didn't miss anything!
> 
> The source code is very inspired on the GIT implementation and it's fully
> featured:
> 
>- Commiters are resolved by issuing the p4 user command, so there is
>information about their full name and E-Mail address.
>- Nighly builds are supported by syncing the repo to the specified point
>in time
>- Updates are properly parsed and all the relevant data (file, modify
>status, description, etc) added to Update.xml
>- 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?

> - Several CMake variables are defined:
>   - P4_CLIENT: this will set the client to use when issuing Perforce
>   commands (-c client)
>   - P4_OPTIONS: sets any global Perforce options (like charset, host
>   name) to be passed before any command: p4 [P4_OPTIONS] sync
>   - P4_UPDATE_OPTIONS: Like in GIT, additional flags to be passed when
>   doing an Update (p4 sync)
>   - CTEST_P4_UPDATE_CUSTOM: Like in GIT, a custom command line to be
>   executed when doing an update, instead of the built-in one
> 
> Let me know if there are any issues or missing/wrong functionality. I plan
> on maintaining the code so please do send bugs and feature requests my way.

This looks dangerous:
this->RegexDiff.compile("^\\.\\.\\. (.*)#[0-9]+ (.*)$");

This will go wrong if the filename contains e.g. "#42". From what your examples 
look like this could be changed to " ([a-z]+)$" or if that doesn't file maybe " 
([^ ]+)$".

+  void DoBodyLine()
+{
+if(this->Line[0] == '\t')

Is it guaranteed that Line is never empty? Otherwise you should test !this-
>Line.empty() before accessing it.

+  unsigned int i;
+  for(i=0; i

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

[cmake-developers] Perforce patch for CTest

[Pedro Navarro]

Hi,

Attached is a patch to add Perforce support to CTest. I searched for
instances of GIT in the CTest code and added the equivalent P4
functionality. Hopefully I didn't miss anything!

The source code is very inspired on the GIT implementation and it's fully
featured:

   - Commiters are resolved by issuing the p4 user command, so there is
   information about their full name and E-Mail address.
   - Nighly builds are supported by syncing the repo to the specified point
   in time
   - Updates are properly parsed and all the relevant data (file, modify
   status, description, etc) added to Update.xml
   - 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.
   - Several CMake variables are defined:
  - P4_CLIENT: this will set the client to use when issuing Perforce
  commands (-c client)
  - P4_OPTIONS: sets any global Perforce options (like charset, host
  name) to be passed before any command: p4 [P4_OPTIONS] sync
  - P4_UPDATE_OPTIONS: Like in GIT, additional flags to be passed when
  doing an Update (p4 sync)
  - CTEST_P4_UPDATE_CUSTOM: Like in GIT, a custom command line to be
  executed when doing an update, instead of the built-in one

Let me know if there are any issues or missing/wrong functionality. I plan
on maintaining the code so please do send bugs and feature requests my way.

Next on my list is adding P4WEB support to CDash!

Pedro


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

[Brad King]

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

[cmake-developers] Perforce patch for CTest 2.8

[Pedro Navarro]

Hi all,

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...

What's the proper process to submit this? What documentation needs to be
written (I've added some useful CMake variables like P4_CLIENT or
P4_OPTIONS for example)?

Thanks!
Pedro Navarro
--

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