pdhaliwal added inline comments.
================ Comment at: llvm/cmake/modules/AddLLVM.cmake:1945 + if (NOT touch_head_result EQUAL 0) + return() + endif() ---------------- scott.linder wrote: > This seems to implement the behavior that when the log is not present and is > not writable the VCS header target is never re-build. Would it instead make > more sense to conservatively assume it should //always// be rebuilt? The > latter case is what is implemented by the stackoverflow snippet you mentioned > previously, right? > > If the assumption is that the failure to write the logs file implies this > source is read-only, then it may make sense to assume it never needs to be > rebuilt. However if the sources are changed in another context (e.g. `sudo -u > user-with-write-privs git checkout rev`) this won't trigger a rebuild until > the user re-runs the CMake config step manually. > > I think I would lean towards the conservative approach of always rebuilding, > but this patch as it stands is still an improvement over just falling over in > this case. To retrigger the build when `.git/logs/HEAD` is not writable, I have added solution from first answer of the linked stackoverflow. Given my limited knowledge of cmake, please let me know if that is correct. Right now, the old behaviour is preserved as it is for read-write context. Also, ninja output on nth build for same branch/tag/commit for read-only source with missing `.git/logs/HEAD`, ``` root@8c167cb7d5b9:/src/build# ninja -j8 [1/93] Generating VCSRevision.h, __FakeVCSRevision.h -- Found Git: /usr/bin/git (found version "2.17.1") root@8c167cb7d5b9:/src/build# ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
