[Basic] Update CMakeLists.txt to handle Repo with git

[minseong.kim]

Summary

The find_first_existing_file and find_first_existing_vc_file macros in lib/Basic/CMakeLists.txt are removed. The macros are also defined in {LLVM}/cmake/modules/AddLLVM.cmake for the same purpose. This change serves the following 2 objectives:

1. To remove the redundant code in clang to use the same macros in llvm,
2. The macros in AddLLVM.cmake can also handle repo for displaying correct version information.

[jordan_rose]

As commented at D34955#798369, this isn't sufficient because it doesn't force a rebuild when new changes are merged in from upstream. It's not harmful to stick with the old version information, but if you can find something better that would be, um, better.

[hintonda]

lib/Basic/CMakeLists.txt

6–7

LLVM already has a version of find_first_existing_vc_file in llvm/include/llvm/Support/CMakelists.txt.

Would it make sense move it to an llvm module and reuse it in clang?

[minseong.kim]

Thanks for the reviewing this patch. Please correct if I am wrong.

The addition of "${path}/.git/HEAD" at line 37 is used only once when repo is initially synced "for the first time". This is because ${git_path}/logs/HEAD file at line 34 for Git or Git submodule does not exist with initial repo sync and therefore no generation of SVNVersion.inc which is referenced by --version option, resulting null for the revision values of clang and llvm. After the second repo sync, the first file "${git_path}/logs/HEAD" will be created and can be used to generate "SVNVersion.inc". This enables correct version information to be displayed even if new changes are merged.

lib/Basic/CMakeLists.txt

6–7

Thanks for the suggestion.
My understanding is that "llvm/include/llvm/Support/CMakeLists.txt" is used to generate VCSRevision.h which is used by llvm-specific modules such as opt, not clang. Furthermore find_first_existing_vc_file function in llvm/include/llvm/Support/CMakeLists.txt does not handle the version info either.

[jordan_rose]

The addition of "${path}/.git/HEAD" at line 37 is used only once when repo is initially synced "for the first time". This is because ${git_path}/logs/HEAD file at line 34 for Git or Git submodule does not exist with initial repo sync[.]

Hm, interesting. Unfortunately, adding more commits won't automatically force a reconfigure. Maybe we should consider adding all of these as dependencies? cc @modocache, who just changed some of this.

[minseong.kim]

Thanks @jordan_rose @modocache @hintonda for your time and efforts.
This patch does regenerate the version control info correctly (SVNVersion.inc) every time I re-make clang. Probably I am missing something here.
Could you please be more specific and share your idea about "adding all of these as dependencies" to force reconfigure revision info whenever new commits added.
I will gladly change this patch accordingly.

[minseong.kim]

kindly ping~

[hintonda]

I've submitted a patch, https://reviews.llvm.org/D36971, that moves find_first_existing_file and
find_first_existing_vc_file to ADDLLVM so they can be reused here. If that patch is accepted and lands,
you can then remove these versions and check to see if that solves your problem.

Note that you must remove these versions in order for cmake to use the moved ones, which would be hidden
by the new ones, but can still be accessed by adding a '_' prefix.

lib/Basic/CMakeLists.txt

6–7

I believe the version of find_first_vc_file in llvm/Support/CMakeLists.txt handles this case correctly. So, instead of fixing/maintaining this version, which is slightly different, perhaps the version in llvm/Support/CMakeList.txt could be moved to ADDLLVM.cmake where it could be used by both.

[minseong.kim]

I will test your patch with repo. Thanks for your time and efforts, @hintonda.

[minseong.kim]

I have updated the description with a hope for it to be more descriptive.

Kindly ping~

[hintonda]

Now that https://reviews.llvm.org/D36971 has been committed, I believe you can just remove the find_first_existing_file and find_first_existing_vc_file macro definitions from this file.

[minseong.kim]

Hi~ @hintonda,

Using using find_first_existing_file in ADDLLVM.cmake solves the cases with repo in conjunction with D35532. However, I am not sure it can handle @modocache's git submodule cases (D34955).

@modocache, could you please check removing find_first_existing_file and find_first_existing_vc_file from lib/Basic/CMakeLists.txt solves git submodule cases ?

If so, I will remove them from clang's cmake to make one unified place for the same functionality.

[hintonda]

In D35533#862798, @minseong.kim wrote:

Hi~ @hintonda,

Using using find_first_existing_file in ADDLLVM.cmake solves the cases with repo in conjunction with D35532. However, I am not sure it can handle @modocache's git submodule cases (D34955).

@modocache, could you please check removing find_first_existing_file and find_first_existing_vc_file from lib/Basic/CMakeLists.txt solves git submodule cases ?

If so, I will remove them from clang's cmake to make one unified place for the same functionality.

Essentially, I think having a single version that works in all cases would be best.
I really haven't looked into it, but would it make sense to incorporate @modocache's module changes into the version in AddLLVM.cmake?

[hintonda]

I just looked into both approaches and believe the one taken in AddLLVM.cmake is correct and handles the submodule case correctly. It uses git rev-parse --git-dir to find the location of the actual .git directory.

Please see https://reviews.llvm.org/D31985 for details.

[minseong.kim]

@hintonda, Absolutely. Incorporating @modocache's module changes into the version in AddLLVM.cmake would solve the current version display issue for repo and do not affect the process of other version control systems (e.g. git, git-svn, svn, and git submodule).

[hintonda]

In D35533#862852, @minseong.kim wrote:

@hintonda, Absolutely. Incorporating @modocache's module changes into the version in AddLLVM.cmake would solve the current version display issue for repo and do not affect the process of other version control systems (e.g. git, git-svn, svn, and git submodule).

I think we crossed, but yes, the version in AddLLVM.cmake handles everything correctly. So, I'd recommend removing these versions.

[minseong.kim]

I have updated the diff.

[minseong.kim]

Re-uploading the patch, removing debug messages accidentally included in the patch.

[hintonda]

LGTM.

lib/Basic/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Core
   MC
   Support
   )
 
-# 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)
-  set(git_path "${path}/.git")
-
-  # Normally '.git' is a directory that contains a 'logs/HEAD' file that
-  # is updated as modifications are made to the repository. In case the
-  # repository is a Git submodule, '.git' is a file that contains text that
-  # indicates where the repository's Git directory exists.
-  if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-    FILE(READ "${git_path}" file_contents)
-    if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
-      # '.git' is indeed a link to the submodule's Git directory.
-      # Use the path to that Git directory.
-      set(git_path "${path}/${CMAKE_MATCH_1}")
-    endif()
-  endif()
-
-  find_first_existing_file(${out_var}
-    "${git_path}/logs/HEAD"  # Git or Git submodule
-    "${path}/.svn/wc.db"     # SVN 1.7
-    "${path}/.svn/entries"   # SVN 1.6
-    "${git_path}/HEAD"       # Repo
-    )
-endmacro()
-
 find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
 find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
 
 MESSAGE("djmin llvm_vc  " "${llvm_vc}")
 MESSAGE("djmin clang_vc " "${clang_vc}")
 
 # The VC revision include that we want to generate.
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/SVNVersion.inc")