compnerd added inline comments.
================
Comment at: lldb/cmake/modules/AddLLDB.cmake:292
+ else()
+ string(STRIP ${xcode_dev_dir} xcode_dev_dir)
+ set(subdir "LLDB.framework/Resources/debugserver")
----------------
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.
================
Comment at: lldb/cmake/modules/AddLLDB.cmake:293
+ string(STRIP ${xcode_dev_dir} xcode_dev_dir)
+ set(subdir "LLDB.framework/Resources/debugserver")
+ set(path_shared "${xcode_dev_dir}/../SharedFrameworks/${subdir}")
----------------
I think that `subdir` is a misleading name. This is the path to debugserver,
`subdir` should be `LLDB.framework/Resources`. I don't really have an opinion
on renaming the variable or composing the variable, that choice is yours :-)
================
Comment at: lldb/tools/debugserver/source/CMakeLists.txt:258
endif()
-endif()
+#endif()
----------------
sgraenitz wrote:
> Removing the `if() ... endif()` would bloat the diff even more. Would do it
> in a follow-up NFC commit.
Sure, although ignoring whitespace also is helpful :-)
================
Comment at: lldb/unittests/CMakeLists.txt:83
add_subdirectory(debugserver)
endif()
----------------
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.
================
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)
----------------
Why is this being checked in `lldb-server`?
================
Comment at: lldb/unittests/tools/lldb-server/CMakeLists.txt:20
+ else()
+ set(debugserver_path $<TARGET_FILE:lldb-server>)
+ endif()
----------------
Should this not be `debugserver`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64806/new/
https://reviews.llvm.org/D64806
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
