[PATCH] CMake add_version_info_from_vcs SVN_REPOSITORY

Tilmann Scheller t.scheller at samsung.com
Wed May 14 05:00:42 PDT 2014

Hi Adam,

thanks for your patch, it looks good to me, just one comment below:

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Adam Strzelecki
Sent: Tuesday, May 13, 2014 10:22 PM
To: llvm-commits at cs.uiuc.edu
Cc: Adam Strzelecki

--- a/cmake/modules/VersionFromVCS.cmake
+++ b/cmake/modules/VersionFromVCS.cmake
@@ -1,9 +1,13 @@
 # Adds version control information to the variable VERS. For  # determining
the Version Control System used (if any) it inspects the -# existence of
certain subdirectories under CMAKE_CURRENT_SOURCE_DIR.
+# existence of certain subdirectories under CMAKE_CURRENT_SOURCE_DIR # 
+or optional 2nd argument.
 function(add_version_info_from_vcs VERS)
   string(REPLACE "svn" "" result "${${VERS}}")
+  if( ARGV1 )
+  endif()
     set(result "${result}svn")
     # FindSubversion does not work with symlinks. See PR 8437 @@ -14,15
+18,16 @@ 

I think it would be cleaner to make the second argument a non-optional
argument and always pass in the source directory (e.g. change the use of
add_version_info_from_vcs() within LLVM to pass in
CMAKE_CURRENT_SOURCE_DIR), that way we can get rid of the additional if
statement above.

Thanks for working on this!



More information about the llvm-commits mailing list