[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

Pushpinder Singh via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 28 03:13:03 PDT 2020


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 at 8c167cb7d5b9:/src/build# ninja -j8
[1/93] Generating VCSRevision.h, __FakeVCSRevision.h
-- Found Git: /usr/bin/git (found version "2.17.1") 
root at 8c167cb7d5b9:/src/build#
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79400





More information about the llvm-commits mailing list