[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
-------------- 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/20131022/318a28e1/attachment.sig>