[benchmark] Change GetGitVersion to only check "dirty" when a tag is found

[andrewng]

Summary

The check for "dirty" executes the following command:

git update-index -q --refresh

This can take a considerable amount of time if the index does
need to be refreshed (particularly with the mono repo). This
situation can arise when building shared source on a host in
VMs.

This change removes the explicit "dirty" check and adds
the "--dirty" flag to the "git describe".

[lebedev.ri]

1. Can you just use something along the lines of git describe --always --dirty to avoid the update-index/diff-index?
2. Can you please either submit this upstream yourself, or explicitly state that it is okay to steal it from you and upstream (to avoid getting out of sync)

[lebedev.ri]

(that being said this entire script is kinda bogus, in this sense of integration into other build, the version tag makes no sense since it will be a llvm-one)

[andrewng]

In D63657#1553961, @lebedev.ri wrote:

1. Can you just use something along the lines of git describe --always --dirty to avoid the update-index/diff-index?

Yes, you're right, I can just add '--dirty' to the 'describe' and remove the 'dirty' check altogether. This would be equivalent and simpler.

2. Can you please either submit this upstream yourself, or explicitly state that it is okay to steal it from you and upstream (to avoid getting out of sync)

I didn't realise that this is kept in sync with upstream. I'm happy for any change to be submitted to the upstream benchmark project.

[andrewng]

Simplified as suggested by reviewer.

[andrewng]

In D63657#1553962, @lebedev.ri wrote:

(that being said this entire script is kinda bogus, in this sense of integration into other build, the version tag makes no sense since it will be a llvm-one)

Yes, it doesn't really apply in this usage scenario. Perhaps the versioning should be changed or disabled in some way?

[andrewng]

Ping.

[lebedev.ri]

Whoops, forgot to click "Submit".

In D63657#1554733, @andrewng wrote:

In D63657#1553961, @lebedev.ri wrote:

1. Can you just use something along the lines of git describe --always --dirty to avoid the update-index/diff-index?

Yes, you're right, I can just add '--dirty' to the 'describe' and remove the 'dirty' check altogether. This would be equivalent and simpler.

Oh nice.

2. Can you please either submit this upstream yourself, or explicitly state that it is okay to steal it from you and upstream (to avoid getting out of sync)

I didn't realise that this is kept in sync with upstream. I'm happy for any change to be submitted to the upstream benchmark project.

(I'm not sure which one that is, either is fine)

In D63657#1554737, @andrewng wrote:

In D63657#1553962, @lebedev.ri wrote:

(that being said this entire script is kinda bogus, in this sense of integration into other build, the version tag makes no sense since it will be a llvm-one)

Yes, it doesn't really apply in this usage scenario. Perhaps the versioning should be changed or disabled in some way?

Yeah, i've thought about it, and i think it will be best to just hardcode v0.0.0 there, avoids the need to submit anything upstream, too

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9db1361..464cfe2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -76,8 +76,11 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 
 
 # Read the git tags to determine the project version
-include(GetGitVersion)
-get_git_version(GIT_VERSION)
+# wARNING: this is meaningless for when the benchmark is being built in-tree,
+# so we disable it, and hardcode a null version.
+# include(GetGitVersion)
+# get_git_version(GIT_VERSION)
+set(GIT_VERSION "v0.0.0")
 
 # Tell the user what versions we are using
 string(REGEX MATCH "[0-9]+\\.[0-9]+\\.[0-9]+" VERSION ${GIT_VERSION})

Please document that in llvm/utils/benchmark/README.LLVM

[andrewng]

Abandoning in favour of D63925.

llvm/utils/benchmark/cmake/GetGitVersion.cmake

 
 if(__get_git_version)
   return()
 endif()
 set(__get_git_version INCLUDED)
 
 function(get_git_version var)
   if(GIT_EXECUTABLE)
-      execute_process(COMMAND ${GIT_EXECUTABLE} describe --match "v[0-9]*.[0-9]*.[0-9]*" --abbrev=8
+      execute_process(COMMAND ${GIT_EXECUTABLE} describe --match "v[0-9]*.[0-9]*.[0-9]*" --abbrev=8 --dirty
           WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
           RESULT_VARIABLE status
           OUTPUT_VARIABLE GIT_VERSION
           ERROR_QUIET)
       if(${status})
           set(GIT_VERSION "v0.0.0")
       else()
           string(STRIP ${GIT_VERSION} GIT_VERSION)
           string(REGEX REPLACE "-[0-9]+-g" "-" GIT_VERSION ${GIT_VERSION})
       endif()
-
-      # Work out if the repository is dirty
-      execute_process(COMMAND ${GIT_EXECUTABLE} update-index -q --refresh
-          WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
-          OUTPUT_QUIET
-          ERROR_QUIET)
-      execute_process(COMMAND ${GIT_EXECUTABLE} diff-index --name-only HEAD --
-          WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
-          OUTPUT_VARIABLE GIT_DIFF_INDEX
-          ERROR_QUIET)
-      string(COMPARE NOTEQUAL "${GIT_DIFF_INDEX}" "" GIT_DIRTY)
-      if (${GIT_DIRTY})
-          set(GIT_VERSION "${GIT_VERSION}-dirty")
-      endif()
   else()
       set(GIT_VERSION "v0.0.0")
   endif()
 
   message("-- git Version: ${GIT_VERSION}")
   set(${var} ${GIT_VERSION} PARENT_SCOPE)
 endfunction()