[Lldb-commits] [PATCH] D54476: [CMake] Streamline code signing for debugserver

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

sgraenitz added a comment.

Last revision changed a lot, but I think support for reconfigurations is worth it.
The current state would be acceptable from my side now -- if I didn't miss anything! :-)



================
Comment at: tools/debugserver/source/CMakeLists.txt:101
+option(LLDB_NO_DEBUGSERVER "Delete debugserver after building it, and don't try to codesign it" OFF)
+option(LLDB_USE_SYSTEM_DEBUGSERVER "Neither build nor codesign debugserver. Use the system's debugserver instead (Darwin only)." OFF)
+
----------------
labath wrote:
> sgraenitz wrote:
> > JDevlieghere wrote:
> > > Should we emit and error if these variables are in an inconsistent state, e.g. when both are set? 
> > Yes, could do that.
> Alternatively, we could make this a single tri-state variable. `LLDB_DEBUGSERVER_MODE=OFF|SYSTEM|TREE` or something ?
Yes possible, but I hope it's not necessary. It would be great to keep existing names for user-definable options where possible. Otherwise it breaks bots, etc.


================
Comment at: tools/debugserver/source/CMakeLists.txt:159
+# step at the moment.
+set(default_debugserver_path "${LLVM_TOOLS_BINARY_DIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX}")
+
----------------
This is not a regression. Just added a verbose comment to remember once we look at this.


================
Comment at: tools/debugserver/source/CMakeLists.txt:190
+  message(STATUS "lldb debugserver will not be available.")
 endif()
 
----------------
This is resetting `DEBUGSERVER_PATH` and `SKIP_TEST_DEBUGSERVER` cached variables for the 3 non-error cases (see updated summary). It should be reconfiguration-safe. All manual testing on Darwin passed.


https://reviews.llvm.org/D54476