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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Whoops, forgot to click "Submit".

In D63657#1554733 <https://reviews.llvm.org/D63657#1554733>, @andrewng wrote:

> In D63657#1553961 <https://reviews.llvm.org/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 <https://reviews.llvm.org/D63657#1554737>, @andrewng wrote:

> In D63657#1553962 <https://reviews.llvm.org/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`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63657/new/

https://reviews.llvm.org/D63657