[cmake-developers] ExternalProject and git clone

[David Cole]

Comments:


(1) Obviously, remove the "FIXME" and actually add the documentation 
before merging this into CMake 'next'...


(2) This won't work:
+string(REGEX MATCH \"^\${abs_binary_dir}\" source_in_binary_dir 
\"\${abs_source_dir}\")

If there are REGEX characters in the directory name.

We have a similar test in 
CMake/Tests/CMakeTests/CheckSourceTreeTest.cmake.in, but it uses 
STREQUAL, string lengths and SUBSTRING to test if it's an in-source 
build. Something similar to that would work reliably even when REGEX 
characters are in the directory name.


(3) Here:
+  if(NOT IS_DIRECTORY \"${source_dir}\")

Do you mean to test for the ".git" directory?


(4) The second time the "Repository URL is different" code is hit, 
there will already be an "origin.old" remote. Does that mean the call 
will fail with an error? Could you construct a unique name each time 
through instead to avoid the error?


Looks reasonable (just at first glance) beyond those comments. But I 
did not run any tests using it...

I'll let Brad and Rob make the call about whether a patch with the 
above comments incorporated should be merged to 'next' as is, or 
whether it should be optionally enabled by a flag as well. I already 
said I think we should preserve existing behavior, and only explicitly 
do this new behavior if it's requested by a new parameter. I'm 
conservative that way, and would prefer not to have existing folks who 
are happy with the behavior become upset when they suddenly get a new 
error just because they updated to a new version of CMake.

Anybody else have an opinion on this? Should it be discussed on the 
users mailing list as well?


HTH,
David