[Lldb-commits] [PATCH] D55013: [CMake] Streamline code signing for debugserver #2

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz marked 9 inline comments as done.
sgraenitz added inline comments.


================
Comment at: CMakeLists.txt:141
   if(TARGET debugserver)
-    if(NOT CMAKE_HOST_APPLE OR LLDB_CODESIGN_IDENTITY)
-      list(APPEND LLDB_TEST_DEPS debugserver)
-    endif()
+    list(APPEND LLDB_TEST_DEPS debugserver)
   endif()
----------------
If we have a debugserver target, tests should depend on it (either copy or build).


================
Comment at: tools/debugserver/CMakeLists.txt:18
+
+  option(LLDB_USE_ENTITLEMENTS "When code signing, use entitlements if available" ON)
+  set(LLDB_CODESIGN_IDENTITY lldb_codesign CACHE STRING
----------------
JDevlieghere wrote:
> sgraenitz wrote:
> > JDevlieghere wrote:
> > > sgraenitz wrote:
> > > > JDevlieghere wrote:
> > > > > Why do we need to define this option again for the debugserver? Why can't it use the one from LLDBConfig.cmake?
> > > > In a debugserver standalone build we don't `include(LLDBConfig)`. It makes sense to me, because there is not much to configure when only building the debugserver.
> > > Makes sense. Maybe we could put this in the top level CMakeLists.txt? 
> > This is the top level CMakeLists.txt for standalone debugserver :) See my previous comment about testing.
> Thanks. I'm not super excited about duplicating the variables but I understand why it's needed. I think long term it'd be great if we could extract some stuff form the top-level cmake file and just make it dispatch things, but that's beyond the scope of what  you're doing right now.
Less duplication as `LLDB_CODESIGN_IDENTITY` went back to debugserver CMakeLists.txt


================
Comment at: tools/debugserver/source/CMakeLists.txt:109
+  set(LLDB_CODESIGN_IDENTITY_USED lldb_codesign CACHE STRING "" FORCE)
+endif()
+
----------------
New logic for code sign identity. Using a separate cache variable to avoid varying behavior on reconfigurations.


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

https://reviews.llvm.org/D55013