[PATCH] D57063: [CMake] Unify scripts for generating VCS headers

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/cmake/modules/VersionFromVCS.cmake:58
+      if (EXISTS "${git_dir}/svn/refs")
+        execute_process(COMMAND ${GIT_EXECUTABLE} svn info
+          WORKING_DIRECTORY ${path}
----------------
mgorny wrote:
> phosek wrote:
> > mgorny wrote:
> > > Can't you reuse `get_source_info_git_svn` here?
> > I considered it but that macro does some duplicate work like finding Git and finding the working directory which seems unnecessary.
> Isn't the result cached? And if it isn't, can you cache it? Avoiding duplicate code makes future maintenance easier, and I think this is exactly the purpose of this diff.
I'm not sure what you mean by duplicate code? I have removed `get_source_info_git_svn` (the one in the first patch was just a leftover) and instead merged its logic into this macro to avoid duplication. I can split this code into a macro again if you prefer?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57063