<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 14, 2014 at 3:08 PM, Adam Strzelecki <span dir="ltr"><<a href="mailto:ono@java.pl" target="_blank">ono@java.pl</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
CMake add_version_info_from_vcs function now requires 2nd SOURCE_DIR argument<br>
specifying where to lookup repository information.<br></blockquote><div><br></div><div>I'm curious, why not just call this from the right position? I'm not really opposed to using a SOURCE_DIR, but it seems not unreasonable that each subproject would call this from its top-most CMakeLists.txt only, and set up variables to be used from there down.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
It also tries now to figure out repository URL and revision on Git mirror<br>
parsing git-svn-id: footer from last commit (if present).<br></blockquote><div><br></div><div>This seems like strict goodness. Should it be separated into its own patch?</div><div><br></div><div>Also, you seem to be making a large number of cleanups to the function while there. This is nice, but should be mentioned.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
This will be used by Clang to show full build information when<br>
LLVM_APPEND_VC_REV is enabled and LLVM/Clang are built from Git.<br>
---<br>
CMakeLists.txt | 2 +-<br>
cmake/modules/VersionFromVCS.cmake | 73 +++++++++++++++++++++++++-------------<br>
2 files changed, 50 insertions(+), 25 deletions(-)<br>
<br>
diff --git a/CMakeLists.txt b/CMakeLists.txt<br>
index 9ec3e33..bc91b00 100644<br>
--- a/CMakeLists.txt<br>
+++ b/CMakeLists.txt<br>
@@ -46,7 +46,7 @@ option(LLVM_APPEND_VC_REV<br>
"Append the version control system revision id to LLVM version" OFF)<br>
<br>
if( LLVM_APPEND_VC_REV )<br>
- add_version_info_from_vcs(PACKAGE_VERSION)<br>
+ add_version_info_from_vcs(PACKAGE_VERSION ${CMAKE_CURRENT_SOURCE_DIR})<br>
endif()<br>
<br>
set(PACKAGE_NAME LLVM)<br>
diff --git a/cmake/modules/VersionFromVCS.cmake b/cmake/modules/VersionFromVCS.cmake<br>
index 26314d4..867331f 100644<br>
--- a/cmake/modules/VersionFromVCS.cmake<br>
+++ b/cmake/modules/VersionFromVCS.cmake<br>
@@ -1,31 +1,34 @@<br>
-# Adds version control information to the variable VERS. For<br>
-# determining the Version Control System used (if any) it inspects the<br>
-# existence of certain subdirectories under CMAKE_CURRENT_SOURCE_DIR.<br>
+# Adds version control information to the variable VERS. For determining the<br>
+# Version Control System used (if any) it inspects the existence of certain<br>
+# subdirectories under SOURCE_DIR.<br>
+# Additionaly sets SVN_REVISION and SVN_REPOSITORY parent scope variables to<br>
+# source code revision and repository URL.<br></blockquote><div><br></div><div>It's good to clarify that this function is mucking with the parent scope, but I really wish it just *didn't* muck with the parent scope.</div>
<div><br></div><div>If you're interested in this area, the correct interface (IMO) is accept a variable *prefix*, and then to set prefixed variables so I could write:</div><div><br></div><div>add_version_info_from_vcs(FOO)</div>
<div>message("Foo version: ${FOO_VERSION}")</div><div>message("Foo svn revision: ${FOO_SVN_REVISION}")</div><div>message("Foo svn repository: ${FOO_SVN_REPOSITORY}")</div><div>message("Foo git commit: ${FOO_GIT_COMMIT}")</div>
<div><br></div><div>(Or something like this... It's just pseudo-CMake but you get the idea I suspect)</div><div><br></div><div>This will also require some changes throughout LLVM and Clang's existing setup, but will make your subsequent patch make a lot more sense because then we'll have LLVM_SVN_REVISION from the beginning, and not have any name conflicts.</div>
<div><br></div><div>Also, we've been really lax about exactly how to represent the N revision numbers that apply to a single clang binary. We should be more careful about this, but it just hasn't come up thus far.... We should at least clearly separate the revision of LLVM and the revision of Clang...</div>
<div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
-function(add_version_info_from_vcs VERS)<br>
+function(add_version_info_from_vcs VERS SOURCE_DIR)<br>
string(REPLACE "svn" "" result "${${VERS}}")<br>
- if( EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/.svn" )<br>
+ if( EXISTS "${SOURCE_DIR}/.svn" )<br>
set(result "${result}svn")<br>
# FindSubversion does not work with symlinks. See PR 8437<br>
- if( NOT IS_SYMLINK "${CMAKE_CURRENT_SOURCE_DIR}" )<br>
+ if( NOT IS_SYMLINK "${SOURCE_DIR}" )<br>
find_package(Subversion)<br>
endif()<br>
if( Subversion_FOUND )<br>
- subversion_wc_info( ${CMAKE_CURRENT_SOURCE_DIR} Project )<br>
+ subversion_wc_info( ${SOURCE_DIR} Project )<br>
if( Project_WC_REVISION )<br>
set(SVN_REVISION ${Project_WC_REVISION} PARENT_SCOPE)<br>
+ set(SVN_REPOSITORY ${Project_WC_URL} PARENT_SCOPE)<br>
set(result "${result}-r${Project_WC_REVISION}")<br>
endif()<br>
endif()<br>
- elseif( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/.git )<br>
+ elseif( EXISTS ${SOURCE_DIR}/.git )<br>
set(result "${result}git")<br>
- # Try to get a ref-id<br>
- if( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/.git/svn )<br>
- find_program(git_executable NAMES git git.exe git.cmd)<br>
- if( git_executable )<br>
+ find_program(git_executable NAMES git git.exe git.cmd)<br>
+ if( git_executable )<br>
+ # Try to get a ref-id<br>
+ if( EXISTS ${SOURCE_DIR}/.git/svn )<br>
set(is_git_svn_rev_exact false)<br>
execute_process(COMMAND ${git_executable} svn log --limit=1 --oneline<br>
- WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}<br>
+ WORKING_DIRECTORY ${SOURCE_DIR}<br>
TIMEOUT 5<br>
RESULT_VARIABLE git_result<br>
OUTPUT_VARIABLE git_output)<br>
@@ -36,10 +39,19 @@ function(add_version_info_from_vcs VERS)<br>
string(SUBSTRING "${git_svn_rev}" 1 ${rev_length} git_svn_rev_number)<br>
set(SVN_REVISION ${git_svn_rev_number} PARENT_SCOPE)<br>
set(git_svn_rev "-svn-${git_svn_rev}")<br>
-<br>
+ # Get repository URL<br>
+ execute_process(COMMAND ${git_executable} svn info --url<br>
+ WORKING_DIRECTORY ${SOURCE_DIR}<br>
+ TIMEOUT 5<br>
+ RESULT_VARIABLE git_result<br>
+ OUTPUT_VARIABLE git_output)<br>
+ if( git_result EQUAL 0 )<br>
+ string(STRIP "${git_output}" git_svn_info_url)<br>
+ set(SVN_REPOSITORY ${git_svn_info_url} PARENT_SCOPE)<br>
+ endif()<br>
# Determine if the HEAD points directly at a subversion revision.<br>
execute_process(COMMAND ${git_executable} svn find-rev HEAD<br>
- WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}<br>
+ WORKING_DIRECTORY ${SOURCE_DIR}<br>
TIMEOUT 5<br>
RESULT_VARIABLE git_result<br>
OUTPUT_VARIABLE git_output)<br>
@@ -52,20 +64,33 @@ function(add_version_info_from_vcs VERS)<br>
else()<br>
set(git_svn_rev "")<br>
endif()<br>
- execute_process(COMMAND<br>
- ${git_executable} rev-parse --short HEAD<br>
- WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}<br>
+ else() # Figure out revision and URL from last commit footer<br>
+ execute_process(COMMAND ${git_executable} log -1 --pretty=format:%b<br>
+ WORKING_DIRECTORY ${SOURCE_DIR}<br>
TIMEOUT 5<br>
RESULT_VARIABLE git_result<br>
OUTPUT_VARIABLE git_output)<br>
- if( git_result EQUAL 0 AND NOT is_git_svn_rev_exact )<br>
- string(STRIP "${git_output}" git_ref_id)<br>
- set(GIT_COMMIT ${git_ref_id} PARENT_SCOPE)<br>
- set(result "${result}${git_svn_rev}-${git_ref_id}")<br>
- else()<br>
- set(result "${result}${git_svn_rev}")<br>
+ if( git_result EQUAL 0 AND<br>
+ git_output MATCHES "^(.*\n)?git-svn-id: ([^@]*)@([0-9]+)" )<br>
+ set(SVN_REVISION ${CMAKE_MATCH_3} PARENT_SCOPE)<br>
+ set(SVN_REPOSITORY ${CMAKE_MATCH_2} PARENT_SCOPE)<br>
+ set(git_svn_rev "-svn-r${CMAKE_MATCH_3}")<br>
+ set(is_git_svn_rev_exact true)<br>
endif()<br>
endif()<br>
+ execute_process(COMMAND<br>
+ ${git_executable} rev-parse --short HEAD<br>
+ WORKING_DIRECTORY ${SOURCE_DIR}<br>
+ TIMEOUT 5<br>
+ RESULT_VARIABLE git_result<br>
+ OUTPUT_VARIABLE git_output)<br>
+ if( git_result EQUAL 0 AND NOT is_git_svn_rev_exact )<br>
+ string(STRIP "${git_output}" git_ref_id)<br>
+ set(GIT_COMMIT ${git_ref_id} PARENT_SCOPE)<br>
+ set(result "${result}${git_svn_rev}-${git_ref_id}")<br>
+ else()<br>
+ set(result "${result}${git_svn_rev}")<br>
+ endif()<br>
endif()<br>
endif()<br>
set(${VERS} ${result} PARENT_SCOPE)<br>
<span class=""><font color="#888888">--<br>
1.8.5.2 (Apple Git-48)<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</font></span></blockquote></div><br></div></div>