This is an archive of the discontinued LLVM Phabricator instance.

Introduce install-lldb-framework target
ClosedPublic

Authored by xiaobai on Jul 30 2018, 7:42 PM.

Details

Summary

Previously, I thought that install-liblldb would fail because CMake had
a bug related to installing frameworks. In actuality, I misunderstood the
semantics of add_custom_target: the DEPENDS option refers to specific files,
not targets. Therefore install-liblldb should rely on the actual liblldb
getting generated rather than the target.

This means that the previous patch I committed (to stop relying on CMake's
framework support) is no longer needed and has been reverted. Using CMake's
framework support greatly simplifies the implementation.

install-lldb-framework (and the stripped variant) is as simple as
depending on install-liblldb because CMake knows that liblldb was built as a
framework and will install the whole framework for you. The stripped variant
will depend on the stripped variants of individual tools only to ensure they
actually are stripped as well.

Diff Detail

Repository
rL LLVM

Event Timeline

xiaobai created this revision.Jul 30 2018, 7:42 PM

Using this patch, I was able to build the lldb framework and install it. I configured with:

cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLDB_CODESIGN_IDENTITY="" -DLLDB_BUILD_FRAMEWORK=1 -DLLDB_USE_SYSTEM_SIX=1 -DCMAKE_INSTALL_PREFIX="" -DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" -DSKIP_DEBUGSERVER=1 -DLLVM_DISTRIBUTION_COMPONENTS="lldb;lldb-framework"

and built with:

DESTDIR=/Users/apl/code/llvm/build/tmp ninja install-distribution

This built and installed lldb at DESTDIR. I used the installed lldb to debug a small test binary.

I am glad filing the cmake bug has paid off. :)

I just have one small question about this patch.

cmake/modules/AddLLDB.cmake
81–87 ↗(On Diff #158166)

Shouldn't this be also guarded under the NOT LLVM_INSTALL_TOOLCHAIN_ONLY, like the code above? If someone sets LLVM_INSTALL_TOOLCHAIN_ONLY=True, then the install-liblldb target will not even be generated and these lines will fail (or just do something weird).

I am glad filing the cmake bug has paid off. :)

Same! :)

cmake/modules/AddLLDB.cmake
81–87 ↗(On Diff #158166)

Ahh, yeah I think you're right. I think it should actually be a level deeper in if (NOT CMAKE_CONFIGURATION_TYPES). I'll amend this.

xiaobai updated this revision to Diff 158329.Jul 31 2018, 11:00 AM

Address comments

labath accepted this revision.Aug 1 2018, 1:24 AM

lgtm

This revision is now accepted and ready to land.Aug 1 2018, 1:24 AM

Might be nice to put a blurb in the build page about this in the MacOS section?

Might be nice to put a blurb in the build page about this in the MacOS section?

Yep, I think that wouldn't be a bad idea. I can handle that in a separate commit. Might be nice for one of the buildbots to use this method of building as well. :P

This revision was automatically updated to reflect the committed changes.