[Lldb-commits] [PATCH] D64806: [CMake] Always build debugserver on Darwin and allow tests to use the system's one

Wed Jul 17 16:00:18 PDT 2019

sgraenitz added a comment.

Thanks for taking a look.

================
Comment at: lldb/cmake/modules/AddLLDB.cmake:292
+  else()
+    string(STRIP ${xcode_dev_dir} xcode_dev_dir)
+    set(subdir "LLDB.framework/Resources/debugserver")
----------------
compnerd wrote:
> Can you add a comment explaining that you want to strip leading whitespace?  Alternatively, if its trailing whitespace, please use `OUTPUT_STRIP_TRAILING_WHITESPACE` in the `execute_process` on L288 please.
It's been there before and I think it may strip a trailing newline. Went with the `execute_process` variant.

================
Comment at: lldb/unittests/CMakeLists.txt:83
   add_subdirectory(debugserver)
 endif()
----------------
compnerd wrote:
> Shouldn't debugserver not be available always?  It doesn't matter if it isn't being used.  Furthermore, elision from the distribution targets will also prevent the unnecessary build so there is no need to worry about the default builds being longer than necessary.
> Shouldn't debugserver not be available always?
This is the directory for debugserver unittests.

================
Comment at: lldb/unittests/tools/lldb-server/CMakeLists.txt:16

-if(DEBUGSERVER_PATH)
-  add_definitions(-DLLDB_SERVER="${DEBUGSERVER_PATH}" -DLLDB_SERVER_IS_DEBUGSERVER=1)
+if(LLDB_CAN_USE_DEBUGSERVER)
+  if(LLDB_USE_SYSTEM_DEBUGSERVER)
----------------
compnerd wrote:
> Why is this being checked in `lldb-server`?
It sets `LLDB_SERVER_IS_DEBUGSERVER`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64806