[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<Options.size(); i++)

This could cause truncation warnings, use size_t better Options::size_type 
instead. Or use an iterator.

Greetings,

Eike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20131002/5245afff/attachment.sig>