Page MenuHomePhabricator

Make `llvm-config` work when static versions of LLVM's components aren't installed.
AbandonedPublic

Authored by DiamondLovesYou on Aug 7 2015, 8:32 AM.

Details

Reviewers
jfb
brad.king
Summary

The following only applies if the following is true:

  • llvm-config isn't inside the build tree (ie it's installed).
  • LLVM is built with shared libs (via --enable-shared or LLVM_BUILD_LLVM_DYLIB) enabled.
  • llvm-config finds the platform specific shared library file in the 'lib' subfolder within the install root (ie it finds 'libLLVM-3.7.0svn.so' or 'libLLVM.so.3.7.0svn' on Linux).

If llvm-config, when visiting all the components, finds a missing
archive, it will override the set of RequiredLibs to only include
that shared library. Note that this means that if one has installed
only select libraries in static form and component X and it's deps are
among them, --libname X will still print static archive names. I
wouldn't recommend doing that (installing shared library + only some
static libraries) for obvious reasons, but I thought it should be
documented nonetheless.

Diff Detail

Repository
rL LLVM

Event Timeline

DiamondLovesYou planned changes to this revision.Aug 7 2015, 8:32 AM
DiamondLovesYou updated this revision to Diff 31514.
DiamondLovesYou retitled this revision from to Make `llvm-config` work when static versions of LLVM's components aren't installed..
DiamondLovesYou updated this object.
DiamondLovesYou added reviewers: jfb, brad.king.
DiamondLovesYou added a subscriber: llvm-commits.
DiamondLovesYou requested a review of this revision.Aug 7 2015, 8:33 AM
brad.king added inline comments.Aug 7 2015, 12:42 PM
tools/llvm-config/llvm-config.cpp
380

When building with CMake the value of BUILD_SHARED_LIBS may have "true" values other than ON. The value could be configured in the header file with

#cmakedefine BUILD_SHARED_LIBS

and then the logic here can just use #ifdef. Or, one could configure another variable that is known to be set as desired:

if(BUILD_SHARED_LIBS)
  set(LLVM_ENABLE_SHARED ON)
else()
  set(LLVM_ENABLE_SHARED OFF)
endif()
tools/llvm-config/llvm-config.cpp
380

But doesn't CMake build separate shared libraries for each component? Meaning libLLVM-3.7.0svn.so wouldn't exist, meaning llvm-config would still use the old behavior? Please correct me if I'm wrong.

delcypher added inline comments.
tools/llvm-config/llvm-config.cpp
380

But doesn't CMake build separate shared libraries for each component

Yes and No. It's complicated if you use BUILD_SHARED_LIBS then think the answer is yes but I don't think that's how it is intended for LLVM to ship and I never build that way.

Chris Bieneman, implemented support for building a single shared library (see LLVM_BUILD_LLVM_DYLIB option). What complicates matters more is that the shared library can have all C++ symbols hidden (see LLVM_DYLIB_EXPORT_ALL) and the components inside the library can be customised using LLVM_DYLIB_COMPONENTS (this currently isn't a CMake cache variable and it probably should be!). The end result of this is just because you have a single shared library it doesn't necessarily mean that all the components are available.

DiamondLovesYou updated this object.
DiamondLovesYou set the repository for this revision to rL LLVM.

Make this patch do the Right Thing on Windows. Also fix a typo in a comment.

beanz added a subscriber: beanz.Aug 24 2015, 10:57 AM

In general I think this patch is moving in the right direction. Eventually we'll need to add better support for working with the CMake-generated shared library. In particular because LLVM_DYLIB_COMPONENTS can be modified to control what LLVM libraries end up in the dylib you can reasonably end up in a position where an llvm-config command might need to return a mix of static and shared libraries.

My plan had been at some point to bake this into the CMake scripts and have that be installed, but we will need that support in llvm-config at some point too.

tools/llvm-config/llvm-config.cpp
380

LLVM_DYLIB_COMPONENTS doesn't need to be a cached value, and I don't think it should be. The only benefit to making it a cached value would be that it would show up in ccmake and the CMake GUI, but it brings downsides because there is no cache invalidation in CMake. If you override it on the command line the overridden value will end up in the cache, so there is no requirement for this to be cached. With it not being in the cache, if the default value changes it will update everyone's build directories, which should work without issue.

tools/llvm-config/llvm-config.cpp
380

Or one could use a shadow variable to check for changes and keep LLVM_DYLIB_COMPONENTS as a cached variable so it can show up in the CMake GUI.

At any rate, this discussion is tangential to Brad's original comment, which is fixed.

DiamondLovesYou marked an inline comment as done.Aug 29 2015, 3:32 PM

Bump. Could I get someone's attention to review this? I'd like to get it accepted.

DiamondLovesYou abandoned this revision.Sep 27 2015, 12:26 AM

I've created a better patch similar to this one: http://reviews.llvm.org/D13198.