Add SVN version to libLLVMLTO

[mehdi_amini]

Summary

For Incremental LTO, we need to make sure that an old
cache entry is not used when incrementally re-linking with a new
libLTO.
Adding a global LLVM_REVISION in llvm-config.h would for to
rebuild/relink the world for every "git pull"/"svn update".
So instead only libLTO is made dependent on the VCS and will
be rebuilt (and the dependent binaries relinked, i.e. as of
today: libLTO.dylib and llvm-lto).

[silvas]

This is sort of thorny.

The revision number will change every time I update my checkout, and llvm-config.h is pretty much at the root of the header dependency graph, so basically everything will be recompiled (this will e.g. make "incremental" buildbots useless).

On the other hand, if LLVM_REVISION_STRING is not updated on each checkout, then it is not reliable and that is sort of pointless.

[mehdi_amini]

Limit to libLTO to prevent rebuilding the world

[silvas]

Could you clarify the relation of this with the SVNVersion.inc code in clang? Did you hoist bring it up and share it, does that even make sense, etc.

[mehdi_amini]

SVNVersion.inc in clang is partially redundant: it includes both the LLVM revision and the clang revision, it will also trigger a rebuild of libclangBasic and a relink of clang binaries that depends on it.
However SVNVersion.inc only includes the SVN rev, while here we handle either svn or git rev, reusing the LLVM_APPEND_VC_REV scripts.

[silvas]

Okay, this makes sense to me then. Seems reasonable to limit this to libLTO.

[beanz]

This all looks reasonable. As a follow-up it might be nice to refactor some of the related clang infrastructure to reduce duplication of functionality, but that shouldn't block landing this change.

A few minor comments inline.

cmake/modules/VersionFromVCS.cmake

6–10

I assume you'll also have a corresponding clang patch to update the caller there, correct? Alternatively you could have the source_dir be an optional parameter and have it fall back to CMAKE_CURRENT_SOURCE_DIR if ARGN == 0.

lib/LTO/CMakeLists.txt

72

Can you name this something that identifies that it is just for libLTO (i.e. LTORevision.h)?

[mehdi_amini]

Address comments

[mehdi_amini]

Thanks, updated. I'll commit later.

cmake/modules/VersionFromVCS.cmake

6–10

Let's go for the optional parameter!

[mehdi_amini]

r266523

[mgorny]

@mehdi_amini, @beanz, I'm sorry for reviving the topic but I can't find the rationale I'm looking for. Am I missing something, but what is the relation between the two new CMake modules (GenerateVersionFromCVS and VersionFromVCS) and the GetSVN module that's there since 2013 (with updates in 2014)? Both seem to revolve around the same code, with little differences. Should other projects be updated to use GenerateVersionFromCVS instead of GetSVN?

[beanz]

@mgorny, as I said in my comment:

As a follow-up it might be nice to refactor some of the related clang infrastructure to reduce duplication of functionality...

We now have 3 separate implementations doing basically the same thing. We really should spend some time cleaning up how these are used in LLVM and the sub-projects. We should be able to get down to a single standard implementation, we're just not anywhere near there.

cmake/modules/GenerateVersionFromCVS.cmake

+# CMake project that writes Subversion revision information to a header.
+#
+# Input variables:
+#   SRC               - Source directory
+#   HEADER_FILE       - The header file to write
+#
+# The output header will contain macros FIRST_REPOSITORY and FIRST_REVISION,
+# and SECOND_REPOSITORY and SECOND_REVISION if requested, where "FIRST" and
+# "SECOND" are substituted with the names specified in the input variables.
+
+
+
+# Chop off cmake/modules/GetSVN.cmake
+get_filename_component(LLVM_DIR "${CMAKE_SCRIPT_MODE_FILE}" PATH)
+get_filename_component(LLVM_DIR "${LLVM_DIR}" PATH)
+get_filename_component(LLVM_DIR "${LLVM_DIR}" PATH)
+
+set(CMAKE_MODULE_PATH
+  ${CMAKE_MODULE_PATH}
+  "${LLVM_DIR}/cmake/modules")
+include(VersionFromVCS)
+
+# Handle strange terminals
+set(ENV{TERM} "dumb")
+
+function(append_info name path)
+  add_version_info_from_vcs(REVISION ${path})
+  string(STRIP "${REVISION}" REVISION)
+  file(APPEND "${HEADER_FILE}.txt"
+    "#define ${name} \"${REVISION}\"\n")
+endfunction()
+
+append_info(${NAME} "${SOURCE_DIR}")
+
+# Copy the file only if it has changed.
+execute_process(COMMAND ${CMAKE_COMMAND} -E copy_if_different
+  "${HEADER_FILE}.txt" "${HEADER_FILE}")
+file(REMOVE "${HEADER_FILE}.txt")
+

cmake/modules/VersionFromVCS.cmake

 # 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 SOURCE_DIR (if provided as an
+# extra argument, otherwise uses CMAKE_CURRENT_SOURCE_DIR).
 
 function(add_version_info_from_vcs VERS)
+  SET(SOURCE_DIR ${ARGV1})
+  if("${SOURCE_DIR}" STREQUAL "")
+      SET(SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+  endif()
   string(REPLACE "svn" "" result "${${VERS}}")
-  if( EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/.svn" )
+  if( EXISTS "${SOURCE_DIR}/.svn" )
     set(result "${result}svn")
     # FindSubversion does not work with symlinks. See PR 8437
-    if( NOT IS_SYMLINK "${CMAKE_CURRENT_SOURCE_DIR}" )
+    if( NOT IS_SYMLINK "${SOURCE_DIR}" )
       find_package(Subversion)
     endif()
     if( Subversion_FOUND )
-      subversion_wc_info( ${CMAKE_CURRENT_SOURCE_DIR} Project )
+      subversion_wc_info( ${SOURCE_DIR} Project )
       if( Project_WC_REVISION )
         set(SVN_REVISION ${Project_WC_REVISION} PARENT_SCOPE)
         set(result "${result}-r${Project_WC_REVISION}")
       endif()
       if( Project_WC_URL )
         set(LLVM_REPOSITORY ${Project_WC_URL} PARENT_SCOPE)
       endif()
     endif()
-  elseif( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/.git )
+  elseif( EXISTS ${SOURCE_DIR}/.git )
     set(result "${result}git")
     # Try to get a ref-id
-    if( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/.git/svn )
+    if( EXISTS ${SOURCE_DIR}/.git/svn )
       find_program(git_executable NAMES git git.exe git.cmd)
       if( git_executable )
         set(is_git_svn_rev_exact false)
         execute_process(COMMAND
           ${git_executable} svn info
-          WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+          WORKING_DIRECTORY ${SOURCE_DIR}
           TIMEOUT 5
           RESULT_VARIABLE git_result
           OUTPUT_VARIABLE git_output)
         if( git_result EQUAL 0 )
           string(REGEX MATCH "URL: ([^ \n]*)" svn_url ${git_output})
           if(svn_url)
             set(LLVM_REPOSITORY ${CMAKE_MATCH_1} PARENT_SCOPE)
           endif()
 
           string(REGEX REPLACE "^(.*\n)?Revision: ([^\n]+).*"
             "\\2" git_svn_rev_number "${git_output}")
           set(SVN_REVISION ${git_svn_rev_number} PARENT_SCOPE)
           set(git_svn_rev "-svn-${git_svn_rev}")
 
           # Determine if the HEAD points directly at a subversion revision.
           execute_process(COMMAND ${git_executable} svn find-rev HEAD
-            WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+            WORKING_DIRECTORY ${SOURCE_DIR}
             TIMEOUT 5
             RESULT_VARIABLE git_result
             OUTPUT_VARIABLE git_output)
           if( git_result EQUAL 0 )
             string(STRIP "${git_output}" git_head_svn_rev_number)
             if( git_head_svn_rev_number EQUAL git_svn_rev_number )
               set(is_git_svn_rev_exact true)
             endif()
           endif()
         else()
           set(git_svn_rev "")
         endif()
         execute_process(COMMAND
           ${git_executable} rev-parse --short HEAD
-          WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+          WORKING_DIRECTORY ${SOURCE_DIR}
           TIMEOUT 5
           RESULT_VARIABLE git_result
           OUTPUT_VARIABLE git_output)
+
         if( git_result EQUAL 0 AND NOT is_git_svn_rev_exact )
           string(STRIP "${git_output}" git_ref_id)
           set(GIT_COMMIT ${git_ref_id} PARENT_SCOPE)
           set(result "${result}${git_svn_rev}-${git_ref_id}")
         else()
           set(result "${result}${git_svn_rev}")
         endif()
 
       endif()
     endif()
   endif()
   set(${VERS} ${result} PARENT_SCOPE)
 endfunction(add_version_info_from_vcs)

lib/LTO/CMakeLists.txt

+# Figure out if we can track VC revisions.
+function(find_first_existing_file out_var)
+  foreach(file ${ARGN})
+    if(EXISTS "${file}")
+      set(${out_var} "${file}" PARENT_SCOPE)
+      return()
+    endif()
+  endforeach()
+endfunction()
+
+macro(find_first_existing_vc_file out_var path)
+  find_first_existing_file(${out_var}
+    "${path}/.git/logs/HEAD" # Git
+    "${path}/.svn/wc.db"     # SVN 1.7
+    "${path}/.svn/entries"   # SVN 1.6
+    )
+endmacro()
+
+find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
+
+# The VC revision include that we want to generate.
+set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/LLVMLTORevision.h")
+
+set(get_svn_script "${LLVM_MAIN_SRC_DIR}/cmake/modules/GenerateVersionFromCVS.cmake")
+
+if(DEFINED llvm_vc)
+  # Create custom target to generate the VC revision include.
+  add_custom_command(OUTPUT "${version_inc}"
+    DEPENDS "${llvm_vc}" "${get_svn_script}"
+    COMMAND
+    ${CMAKE_COMMAND} "-DSOURCE_DIR=${LLVM_MAIN_SRC_DIR}"
+                     "-DNAME=LLVM_REVISION"
+                     "-DHEADER_FILE=${version_inc}"
+                     -P "${get_svn_script}")
+
+  # Mark the generated header as being generated.
+  set_source_files_properties("${version_inc}"
+    PROPERTIES GENERATED TRUE
+               HEADER_FILE_ONLY TRUE)
+
+  # Tell Version.cpp that it needs to build with -DHAVE_SVN_VERSION_INC.
+  set_source_files_properties(Version.cpp
+    PROPERTIES COMPILE_DEFINITIONS "HAVE_SVN_VERSION_INC")
+else()
+  # Not producing a VC revision include.
+  set(version_inc)
+endif()
+
+
 add_llvm_library(LLVMLTO
   LTOModule.cpp
   LTOCodeGenerator.cpp
   UpdateCompilerUsed.cpp
   ThinLTOCodeGenerator.cpp
+  ${version_inc}
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/LTO
 )
 
 add_dependencies(LLVMLTO intrinsics_gen)

lib/LTO/ThinLTOCodeGenerator.cpp

 //===-ThinLTOCodeGenerator.cpp - LLVM Link Time Optimizer -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Thin Link Time Optimization library. This library is
 // intended to be used by linker to optimize code at link time.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/LTO/ThinLTOCodeGenerator.h"
+#include "LLVMLTORevision.h"
 
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Analysis/ModuleSummaryAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/Bitcode/ReaderWriter.h"