[cfe-commits] [PATCH][CMake] Fix SVN_REVISION for Basic/Version.cpp using git-svn.

[Michael Spencer]

On Wed, Feb 15, 2012 at 12:47 AM, nobled <nobled at dreamwidth.org> wrote:
> On Mon, Feb 13, 2012 at 11:11 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
>> This patch correctly defines SVN_REVISION in Basic/Version.cpp for git
>> for both in tree and out of tree clang when using CMake.
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 2087c6a..477403c 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -38,6 +38,12 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
>   include(HandleLLVMOptions)
>
>   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
> +  include(VersionFromVCS)
> +  option(CLANG_APPEND_VC_REV
> +    "Append the version control system revision id to CLANG version" OFF)
> +  if( CLANG_APPEND_VC_REV )
> +    add_version_info_from_vcs(PACKAGE_VERSION)
> +  endif()
> Won't this create a confusing pileup of revision suffixes when both
> LLVM_APPEND_VC_REV and CLANG_APPEND_VC_REV are on? Something like:
> 3.1git-svn-r150477-e800a7dgit-svn-r150560-37ce010
> And you can't tell which is clang's and which is LLVM's.
>
> Maybe instead, initialize PACKAGE_VERSION with these new variables
> before appending:
> set(PACKAGE_VERSION "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}svn")

Ah, I see what you mean. It currently generates
"3.1git--r150621-db6dbd5git-svn-r150419-e696d36". Which I agree is
confusing.

We really should decide what PACKAGE_VERSION actually represents. The
CMake docs define it in relation to find_package, which we don't
support, so we can really choose whatever we want to.

I believe it should represent specifically Clang's version info (drop
the LLVM PACKAGE_VERSION), and should be formtted as:

<major>.<minor><vc-system>-<branch>-<svn-rev>-<git-hash>

3.1{,git,svn,git-svn}-master-r150000-2390abe89f

Ideally I would like to be able to get a version line such as.

Clang 3.1git-svn-cranch_branch-<hash> (based on svn-r150344) with LLVM
3.1git-svn-master-r150344-<hash>

To make it clear that we are not at r150344, but that it is the most
recent svn revision we branched from.

- Michael Spencer

>   set(LLVM_MAIN_INCLUDE_DIR "${LLVM_MAIN_SRC_DIR}/include")
>   set(LLVM_BINARY_DIR ${CMAKE_BINARY_DIR})
> diff --git a/lib/Basic/CMakeLists.txt b/lib/Basic/CMakeLists.txt
> index ef2e93c..6856122 100644
> --- a/lib/Basic/CMakeLists.txt
> +++ b/lib/Basic/CMakeLists.txt
> @@ -22,14 +22,8 @@ add_clang_library(clangBasic
>  # Determine Subversion revision.
>  # FIXME: This only gets updated when CMake is run, so this revision number
>  # may be out-of-date!
> -if( NOT IS_SYMLINK "${CLANG_SOURCE_DIR}" )  # See PR 8437
> -  find_package(Subversion)
> -endif()
> -if (Subversion_FOUND AND EXISTS "${CLANG_SOURCE_DIR}/.svn")
> -  Subversion_WC_INFO(${CLANG_SOURCE_DIR} CLANG)
> -  set_source_files_properties(Version.cpp
> -    PROPERTIES COMPILE_DEFINITIONS "SVN_REVISION=\"${CLANG_WC_REVISION}\"")
> -endif()
> +set_source_files_properties(Version.cpp
> +  PROPERTIES COMPILE_DEFINITIONS "SVN_REVISION=\"${SVN_REVISION}\"")
>
>  add_dependencies(clangBasic
>                  ClangARMNeon
>
>
>
>>
>> - Michael Spencer