scott.linder added inline comments.

================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1945
+          if (NOT touch_head_result EQUAL 0)
+            return()
+          endif()
----------------
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.


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

Reply via email to