cmake fixups: post-build python step dependency and lldb-server dependency for lldb on Linux

[tfiala]

Summary

Addresses:

• https://llvm.org/bugs/show_bug.cgi?id=24831: cmake + ninja: 'ninja lldb' misses lib/python2.7 build dependency on Linux
• https://llvm.org/bugs/show_bug.cgi?id=24833: cmake + ninja: 'ninja lldb' misses lldb-server dependency on Linux

Without this fix, 'cmake && ninja lldb' would:

• miss building lldb-server on Linux
• miss copying the python module and fail to create the _lldb.so symlink

With this fix, those issues are resolved on Linux. (The python post-build step issue is resolved for all platforms that don't disable python and use the lldb target).

This also prevents the need for executing:
'ninja'
which builds a number of additional executables (llvm/clang) which aren't needed to test/run lldb. So it should trim down build times for those just wanting lldb and its immediate dependencies.

[tfiala]

@emaste, I'm adding you because I have no idea what the state of the *BSD world is with regards to lldb-server. It looks like you might still use the process monitor bits so lldb-server might not be interesting over there. If it is, we can extend the "always add the lldb-server dependency to lldb" for the BSDs as well.

[labath]

Looks good, thanks for fixing this.

tools/driver/CMakeLists.txt

13

We should probably add a dependency on debugserver on darwin for consistency, since some people use cmake to build lldb there also.

[brucem]

tools/driver/CMakeLists.txt

13

Instead of checking CMAKE_SYSTEM_NAME in multiple places for deciding whether or not we care about lldb-server, could we just set LLDB_BUILD_LLDB_SERVER or something once in the cmake/module/LLDBConfig.cmake file and then check that here and in tools/CMakeLists.txt?

(These have already diverged as you are only doing this for Linux, but lldb-server is built on FreeBSD as well.)

[tfiala]

(These have already diverged as you are only doing this for Linux, but lldb-server is built on FreeBSD as well.)

Hi Bruce,

My primary goal was to make sure 'ninja lldb' builds the parts that are necessary for lldb to run and be tested. Linux cannot debug a local executable without lldb-server, so it is essentially a core part of it.

I can see doing it on OS X since the real build process for OS X will not use cmake, and thus it is not increasing build time for typical users there. And we can use it in the tests if it is built.

I am looking for Ed's input on the FreeBSD side.

Instead of checking CMAKE_SYSTEM_NAME in multiple places for deciding whether or not we care about lldb-server, could we just set LLDB_BUILD_LLDB_SERVER or something once in the cmake/module/LLDBConfig.cmake file and then check that here and in tools/CMakeLists.txt?

I suppose. It could allow more sophisticated checks (e.g. if Ed wanted to come in and flip it on for FreeBSD) without clouding the dependency addition. If we're going to turn it on in multiple cases, I can see that being a win.

Okay I'll make an adjustment to turn it on for Darwin and setting it based on a flag.

[tfiala]

Okay I'll make an adjustment to turn it on for Darwin and setting it based on a flag.

I'm going to do it slightly differently --- I'll set a flag in the modules config that indicates if lldb *wants* lldb-server. The logic on whether to build it or not should, as before, be driven by the targets asked for. So we're not really setting whether we build it or not, we're setting whether we want it if we're already building lldb.

[zturner]

Just a drive by comment since I don't have a vested interest in lldb-server
or this CL at the moment. But adding lldb-server as a dependency of lldb
seems strange to me. I mean I get it, it just seems strange. They're not
really dependencies in the normal sense. If you just use "ninja" instead
of "ninja lldb" you should get both targets, while still allowing "ninja
lldb" to only build lldb, as it currently does.

Up to you guys though

[emaste]

In D12899#246881, @tfiala wrote:

@emaste, I'm adding you because I have no idea what the state of the *BSD world is with regards to lldb-server. It looks like you might still use the process monitor bits so lldb-server might not be interesting over there.

We are still using the process monitor bits right now, but the eventual goal is to migrate to lldb-server or something equivalent. lldb-server does build on FreeBSD and I think it's worth having it in the default targets list so that we can catch build breakage early, even if the resulting binary isn't currently usable for us.

[emaste]

In D12899#247074, @zturner wrote:

They're not really dependencies in the normal sense.

They're not build dependencies, but they are dependencies in the sense that lldb-server is required for lldb to be usable in the normal use case. In my opinion I'd hope that "ninja lldb" is sufficient to build a usable lldb and that requires lldb-server.

[tfiala]

Adjustments to set indirect variable for whether lldb can use lldb-server. If so, then we'll add the dependency for lldb-server on lldb at the time we declare the lldb target.

[tfiala]

In D12899#247098, @tfiala wrote:

Adjustments to set indirect variable for whether lldb can use lldb-server. If so, then we'll add the dependency for lldb-server on lldb at the time we declare the lldb target.

Also adds lldb-server dependency on Darwin lldb.

[labath]

tools/driver/CMakeLists.txt

12

In case of darwin you need *debugserver* to be able to debug locally. lldb-server is only used for remote debugging here afaik.

[tfiala]

In D12899#247074, @zturner wrote:

Just a drive by comment since I don't have a vested interest in lldb-server
or this CL at the moment. But adding lldb-server as a dependency of lldb
seems strange to me. I mean I get it, it just seems strange. They're not
really dependencies in the normal sense. If you just use "ninja" instead
of "ninja lldb" you should get both targets, while still allowing "ninja
lldb" to only build lldb, as it currently does.

Up to you guys though

There are a few problems with this that we've hit in spades. People build "lldb" and think they can test it. They can't, not on Linux. Not a single test will work. That's because an essential component of lldb to function on that system. It would be like building 'git' but not getting any of the implementation detail scripts in libexec --- you can't do much with the executable without it.

Practically speaking, yesterday I was working with somebody who built 'ninja lldb' as we state in the official website docs, and it (the lldb produced) couldn't be used for testing or debugging because it was missing essential pieces. And I always avoid it by just doing 'ninja', but then I waste time building other pieces of llvm/clang that I don't use.

The line I initially was drawing was "if it is required to use lldb, it should be a dependency of the lldb target". I feel like asking for it on Darwin crosses that line, since we do not need it there at all. But I rationalized that one because we will use it on the tests if we find it.

[tfiala]

tools/driver/CMakeLists.txt

12

Actually it is also used for the lldb-server/debugserver consistency checks. So the tests will use it if it is built.

I'll add another layer for the debugserver target. Although I don't use cmake for darwin builds, so I'm not sure who we're serving by that.

[labath]

tools/driver/CMakeLists.txt

12

Yeah... it is starting to get a bit over-engineered. :)

I know all lldb-mi folks use cmake on darwin, and I think I remember speaking to some other people as well.

[tfiala]

Adds an orthogonal can-use-debugserver flag. Flips it on for Darwin.

I'm okay with the Darwin cmake build building both of those for the lldb target since I want the consistency checks between the two running on Darwin.

[tfiala]

I think this covers everything.

[tfiala]

Yeah... it is starting to get a bit over-engineered. :)

It's all good :-)

[tfiala]

I'll check this in if I don't hear boo in the next few minutes...

[tfiala]

In D12899#247092, @emaste wrote:

In D12899#246881, @tfiala wrote:

@emaste, I'm adding you because I have no idea what the state of the *BSD world is with regards to lldb-server. It looks like you might still use the process monitor bits so lldb-server might not be interesting over there.

We are still using the process monitor bits right now, but the eventual goal is to migrate to lldb-server or something equivalent. lldb-server does build on FreeBSD and I think it's worth having it in the default targets list so that we can catch build breakage early, even if the resulting binary isn't currently usable for us.

Okay. Ed - I think I'm going to put this one in as is, but it will be trivial for you to add the extra CMAKE_SYSTEM_NAME Matches check for FreeBSD in the LLDBConfig.cmake line when you can verify that it does the intended thing. An easy way to check is do a build the normal way with 'ninja lldb', see that you don't have lldb-server, make your change, and repeat, and you should get an lldb-server at that point. (And also the lib/python2.7 bits that we were forgetting to require on lldb in the past).

[tfiala]

Checked in here:

Sending        CMakeLists.txt
Sending        cmake/modules/LLDBConfig.cmake
Sending        tools/driver/CMakeLists.txt
Transmitting file data ...
Committed revision 247810.

[ki.stfu]

Hi guys,

As for me it wasn't a bug and lldb should not require lldb-server and python framework. "ninja lldb" means "make a lldb executable please". I don't want to build lldb-server or python framework when typing that. If you are really want to get all those files, you can use "ninja". Or, if there is some (unknown for me) reason why you don't want to use it, perhaps it would better to introduce a new meta target named like lldb-bundle which depends on all lldb targets.

[ki.stfu]

i. e. +1 with @zturner

[zturner]

One possible solution is to make an lldb-all target.

[ki.stfu]

In D12899#248648, @zturner wrote:

One possible solution is to make an lldb-all target.

As I said, it would much better rather than changing lldb dependencies. But I'm still not sure, do we really need something like lldb-all? Why we can't use "ninja" (i.e. without arguments) for building a whole LLDB?

[tberghammer]

In D12899#248654, @ki.stfu wrote:

In D12899#248648, @zturner wrote:

One possible solution is to make an lldb-all target.

As I said, it would much better rather than changing lldb dependencies. But I'm still not sure, do we really need something like lldb-all? Why we can't use "ninja" (i.e. without arguments) for building a whole LLDB?

"ninja" without any argument builds everything including llvm, clang and a lot of related tools what increase the link time by a lot (assuming something changed in the llvm/clang repository since the last build).

[tfiala]

From a high level, when we ask a build system to build the lldb executable, we want that to mean "build me an lldb and anything it will use as part of its normal operation."

If we did not build the lldb executable's expected components (as was the case when I started this), we would have:

• no python support copied over. Surprise! If we don't do this, we actually fall back to using the system-installed python lldb libraries, out of sync with the lldb binary just built, and significant issues will ensue as lldb with python-enabled *will* pick up an LLDB python module from somewhere. (This was fixed when I made the python library post-build step dependent on lldb, which by a purist "'only build me exactly what I asked for, and assume I know every bit of what is needed at all times" approach would exclude performing. And we hit this the other day).

• no lldb-server. Oops - lldb doesn't actually work to debug anything on platforms that need it. Do you know what to put on the command line to get it to work? Do we want users to have to know all that?

I do see the point of view of being pure and saying "give me only what I want." But being pure can mean "hey, you only asked for the lldb driver, so don't build the lldb core content that goes in a .so." That starts getting absurd --- we clearly are saying at some point "when you say lldb, you mean give me the internal details needed for this to really work."

I was for not including lldb-server on platforms where it isn't needed right now (which was pretty much anywhere except Linux AFAICT), because then you incur extra build time for something that you might not need --- lldb works fine without lldb-server on some platforms (Darwin, FreeBSD, maybe others). But where it is needed for proper lldb operation, not building it is asking our users to know more about the internal organization of the execution than I think we should be trying to take on.

[ki.stfu]

In D12899#248657, @tberghammer wrote:

In D12899#248654, @ki.stfu wrote:

In D12899#248648, @zturner wrote:

One possible solution is to make an lldb-all target.

As I said, it would much better rather than changing lldb dependencies. But I'm still not sure, do we really need something like lldb-all? Why we can't use "ninja" (i.e. without arguments) for building a whole LLDB?

"ninja" without any argument builds everything including llvm, clang and a lot of related tools what increase the link time by a lot (assuming something changed in the llvm/clang repository since the last build).

cmake/ninja build has no a big difference between "ninja" and "ninja lldb":

$ ninja -j1
[1/3214] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/RandomNumberGenerator.cpp.o^C
ninja: build stopped: interrupted by user.
$ ninja -j1 lldb
[1/2848] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/RandomNumberGenerator.cpp.o^C
ninja: build stopped: interrupted by user.

So it doesn't matter what to do: ninja or ninja lldb.

[tfiala]

In D12899#248741, @ki.stfu wrote:

In D12899#248657, @tberghammer wrote:

In D12899#248654, @ki.stfu wrote:

In D12899#248648, @zturner wrote:

One possible solution is to make an lldb-all target.

As I said, it would much better rather than changing lldb dependencies. But I'm still not sure, do we really need something like lldb-all? Why we can't use "ninja" (i.e. without arguments) for building a whole LLDB?

"ninja" without any argument builds everything including llvm, clang and a lot of related tools what increase the link time by a lot (assuming something changed in the llvm/clang repository since the last build).

cmake/ninja build has no a big difference between "ninja" and "ninja lldb":

That's a 12.8% increase in build-related jobs in 'ninja' vs. 'ninja lldb'. And more linking, which tends to be relatively slow. On 4-core MacBook Pros and VMs, that's a non-trivial difference.

$ ninja -j1
[1/3214] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/RandomNumberGenerator.cpp.o^C
ninja: build stopped: interrupted by user.
$ ninja -j1 lldb
[1/2848] Building CXX object lib/Support/CMakeFiles/LLVMSupport.dir/RandomNumberGenerator.cpp.o^C
ninja: build stopped: interrupted by user.

So it doesn't matter what to do: ninja or ninja lldb.

CMakeLists.txt

 add_subdirectory(lit)
 
 if (NOT LLDB_DISABLE_PYTHON)
-  # Add a Post-Build Event to copy over Python files and create the symlink to liblldb.so for the Python API(hardlink on Windows)
+    # Add a Post-Build Event to copy over Python files and create the symlink to liblldb.so for the Python API(hardlink on Windows)
     add_custom_target( finish_swig ALL
         COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py "--srcRoot=${LLDB_SOURCE_DIR}" "--targetDir=${CMAKE_CURRENT_BINARY_DIR}/scripts" "--cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}/scripts" "--prefix=${CMAKE_BINARY_DIR}" "--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}" -m
         DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
         COMMENT "Python script sym-linking LLDB Python API")
+    # We depend on liblldb being built before we can do this step.
     add_dependencies(finish_swig liblldb argdumper)
+
+    # Ensure we do the python post-build step when building lldb.
+    add_dependencies(lldb finish_swig)
 endif ()

tools/driver/CMakeLists.txt

   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
+# On Linux, ensure we get lldb-server when we build lldb.
+if ( CMAKE_SYSTEM_NAME MATCHES "Linux" )
+  add_dependencies(lldb lldb-server)
+endif()
+
 target_link_libraries(lldb liblldb)
 # TODO: why isn't this done by add_lldb_executable?
 #target_link_libraries(lldb ${LLDB_USED_LIBS})