[lldb-dap] Split lldb-dap into library and tool (NFC)

[JDevlieghere]

Split lldb-dap into a library (lldbDAP) and a tool (lldb-dap). The motivation is being able to link parts of lldb-dap separately, for example to support unit testing and fizzing.

[JDevlieghere]

In part motivated by #139393

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Split lldb-dap into a library (lldbDAP) and a tool (lldb-dap). The motivation is being able to link parts of lldb-dap separately, for example to support unit testing and fizzing.

---

Full diff: https://github.com/llvm/llvm-project/pull/139402.diff

4 Files Affected:

• (modified) lldb/tools/lldb-dap/CMakeLists.txt (+8-14)
• (added) lldb/tools/lldb-dap/tool/CMakeLists.txt (+28)
• (renamed) lldb/tools/lldb-dap/tool/lldb-dap-Info.plist.in ()
• (renamed) lldb/tools/lldb-dap/tool/lldb-dap.cpp ()

diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index a9dc19006293b..25bacd91fe581 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -1,20 +1,14 @@
-if(APPLE)
-  configure_file(
-    ${CMAKE_CURRENT_SOURCE_DIR}/lldb-dap-Info.plist.in
-    ${CMAKE_CURRENT_BINARY_DIR}/lldb-dap-Info.plist
-    )
-  # Inline info plist in binary (use target_link_options for this as soon as CMake 3.13 is available)
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-dap-Info.plist")
-endif()
-
 # We need to include the llvm components we depend on manually, as liblldb does
 # not re-export those.
 set(LLVM_LINK_COMPONENTS Support)
 set(LLVM_TARGET_DEFINITIONS Options.td)
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(LLDBDAPOptionsTableGen)
-add_lldb_tool(lldb-dap
-  lldb-dap.cpp
+
+include_directories(${CMAKE_CURRENT_SOURCE_DIR})
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+
+add_lldb_library(lldbDAP
   Breakpoint.cpp
   BreakpointBase.cpp
   DAP.cpp
@@ -85,10 +79,8 @@ add_lldb_tool(lldb-dap
     Support
   )
 
-target_include_directories(lldb-dap PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
-
 if(LLDB_DAP_WELCOME_MESSAGE)
-  target_compile_definitions(lldb-dap
+  target_compile_definitions(lldbDAP
     PRIVATE
     -DLLDB_DAP_WELCOME_MESSAGE=\"${LLDB_DAP_WELCOME_MESSAGE}\")
 endif()
@@ -105,3 +97,5 @@ if(LLDB_BUILD_FRAMEWORK)
       "@loader_path/../../Library/PrivateFrameworks"
   )
 endif()
+
+add_subdirectory(tool)
diff --git a/lldb/tools/lldb-dap/tool/CMakeLists.txt b/lldb/tools/lldb-dap/tool/CMakeLists.txt
new file mode 100644
index 0000000000000..e418737bc05b1
--- /dev/null
+++ b/lldb/tools/lldb-dap/tool/CMakeLists.txt
@@ -0,0 +1,28 @@
+if(APPLE)
+  configure_file(
+    ${CMAKE_CURRENT_SOURCE_DIR}/lldb-dap-Info.plist.in
+    ${CMAKE_CURRENT_BINARY_DIR}/lldb-dap-Info.plist
+    )
+  # Inline info plist in binary (use target_link_options for this as soon as CMake 3.13 is available)
+  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-dap-Info.plist")
+endif()
+
+add_lldb_tool(lldb-dap
+  lldb-dap.cpp
+
+  LINK_LIBS
+    lldbDAP
+  )
+
+if(LLDB_BUILD_FRAMEWORK)
+  # In the build-tree, we know the exact path to the framework directory.
+  # The installed framework can be in different locations.
+  lldb_setup_rpaths(lldb-dap
+    BUILD_RPATH
+      "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
+    INSTALL_RPATH
+      "@loader_path/../../../SharedFrameworks"
+      "@loader_path/../../System/Library/PrivateFrameworks"
+      "@loader_path/../../Library/PrivateFrameworks"
+  )
+endif()
diff --git a/lldb/tools/lldb-dap/lldb-dap-Info.plist.in b/lldb/tools/lldb-dap/tool/lldb-dap-Info.plist.in
similarity index 100%
rename from lldb/tools/lldb-dap/lldb-dap-Info.plist.in
rename to lldb/tools/lldb-dap/tool/lldb-dap-Info.plist.in
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
similarity index 100%
rename from lldb/tools/lldb-dap/lldb-dap.cpp
rename to lldb/tools/lldb-dap/tool/lldb-dap.cpp

[ashgti]

Looks good to me, I started something similar to write unit tests for the c++ code a while ago but never made it to a PR. Looks good to start having more direct tests of the c++ parts.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-win running on as-builder-10 while building lldb at step 16 "test-check-lldb-unit".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/5187

Here is the relevant piece of the build log for the reference

Step 16 (test-check-lldb-unit) failure: Test just built components: check-lldb-unit completed (failure)
******************** TEST 'lldb-unit :: DAP/./DAPTests.exe/failed_to_discover_tests_from_gtest' FAILED ********************

********************

[ashgti]

Hmm after a sync, this started causing my cmake to fail to configure with:

$ cmake --fresh -B ~/Projects/lldb-build -G Ninja \                                
        -C ~/Projects/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake \
        -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \
        -DCMAKE_OSX_DEPLOYMENT_TARGET=10.12 \
        -DLLVM_ENABLE_PROJECTS="clang;lld;lldb;clang-tools-extra" \
        -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;compiler-rt;libunwind" \
        -DLLDB_USE_SYSTEM_DEBUGSERVER:BOOL=true \
        -DLIBCXX_HARDENING_MODE=none \
        -DLLVM_LIT_ARGS=-v \
        -DLIBCXX_ENABLE_SHARED:BOOL=true \
        -DCMAKE_BUILD_TYPE=Debug \
        -DLLVM_ENABLE_ASSERTIONS:BOOL=true \
        ~/Projects/llvm-project/llvm
loading initial cache file /Users/harjohn/Projects/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake

...

CMake Error at /Users/harjohn/Projects/llvm-project/lldb/cmake/modules/AddLLDB.cmake:352 (set_target_properties):
  set_target_properties Can not find target to add properties to: lldb-dap
Call Stack (most recent call first):
  /Users/harjohn/Projects/llvm-project/lldb/tools/lldb-dap/CMakeLists.txt:91 (lldb_setup_rpaths)

Adding -DLLDB_BUILD_FRAMEWORK:BOOL=false fixed this, but I think the rpath may not be correct.

[JDevlieghere]

I'll take a look. I see it failing here too: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-standalone/1688/

[JDevlieghere]

Fixed by fb9b43a

[nico]

Should this move too? The .inc file is only used in lldb-dap.cpp.