Page MenuHomePhabricator

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

Authored by tfiala on Sep 15 2015, 11:04 PM.

Details

Summary

Addresses:

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.

Diff Detail

Event Timeline

tfiala updated this revision to Diff 34874.Sep 15 2015, 11:04 PM
tfiala retitled this revision from to cmake fixups: post-build python step dependency and lldb-server dependency for lldb on Linux.
tfiala updated this object.
tfiala added reviewers: labath, clayborg, emaste.
tfiala added a subscriber: lldb-commits.
tfiala added a comment.EditedSep 15 2015, 11:10 PM

@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 accepted this revision.Sep 16 2015, 1:36 AM
labath edited edge metadata.

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.

This revision is now accepted and ready to land.Sep 16 2015, 1:36 AM
brucem added a subscriber: brucem.Sep 16 2015, 1:41 AM
brucem added inline comments.
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.)

(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.

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.

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 edited edge metadata.Sep 16 2015, 7:59 AM

@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.

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 updated this revision to Diff 34893.Sep 16 2015, 8:03 AM
tfiala edited edge metadata.

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.

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.

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.

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 added inline comments.Sep 16 2015, 8:14 AM
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 added inline comments.Sep 16 2015, 8:17 AM
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 updated this revision to Diff 34894.Sep 16 2015, 8:21 AM

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 marked 5 inline comments as done.Sep 16 2015, 8:22 AM

I think this covers everything.

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

It's all good :-)

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

@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 closed this revision.Sep 16 2015, 8:35 AM

Checked in here:

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

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.

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

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?

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 added a comment.EditedSep 18 2015, 9:41 AM

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.

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.

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.