Page MenuHomePhabricator

[CMake] make SONAME of libLLVM configurable
AbandonedPublic

Authored by axw on Oct 24 2015, 12:23 AM.

Details

Summary

Make the SONAME of the monolithic libLLVM
configurable. This is part of enabling the creation
of libraries with consistent names between old
autotools-based builds, and new CMake builds.
This diff does not change the current naming
scheme, but it would be trivial to do so later.

This is in response to
https://llvm.org/bugs/show_bug.cgi?id=25059, and
also to enable Debian's LLVM packaging to move to
CMake.

llvm_add_library now has SOVERSION and VERSION
options, which, if non-empty and not equal to "cmake-default",
will be set as properties on shared library targets.
The "cmake-default" special case is due to empty string
arguments not resulting in variables defined. This allows us
to have:
(a) defaults if SOVERSION/VERSION are unspecified (i.e. existing MAJOR.MINOR for SOVERSION, etc.)
(b) a means of using the CMake-default SOVERSION/VERSION (i.e. by passing "cmake-default")

There are two new user-configurable options:

LLVM_DYLIB_SOVERSION (defaults to $MAJOR.$MINOR)
LLVM_DYLIB_VERSION (defaults to $PACKAGE_VERSION)

A follow-up will introduce configuration for the name of
the libLLVM shared library.

Diff Detail

Event Timeline

axw updated this revision to Diff 38295.Oct 24 2015, 12:23 AM
axw retitled this revision from to [CMake] make name of libLLVM configurable.
axw updated this object.
axw added a subscriber: llvm-commits.
axw updated this object.Oct 24 2015, 12:28 AM
axw updated this revision to Diff 38298.Oct 24 2015, 12:30 AM

Fix whitespace

I think we should make the shared object name available via an llvm-config option.

sylvestre.ledru requested changes to this revision.Oct 27 2015, 7:42 AM
sylvestre.ledru edited edge metadata.

Not sure I like the "." idea... This is not obvious. What is wrong with unset ?

By the way, I think we also update the documentation.

This revision now requires changes to proceed.Oct 27 2015, 7:42 AM
axw added a comment.Oct 27 2015, 8:00 AM

Not sure I like the "." idea... This is not obvious. What is wrong with unset ?

I agree it's not very obvious, but it remains that we need some way of preventing SOVERSION from being set.

Can you please explain what you mean by "unset"? Are you suggesting "unset" as an alternative string to compare against, or are you referring to the CMake unset() command? If you mean the command, I'm not sure it would help.

By the way, I think we also update the documentation.

Sure, I'll take a pass over the docs to see what needs updating.

axw added a comment.Oct 27 2015, 8:01 AM

I think we should make the shared object name available via an llvm-config option.

Sounds good, will do.

beanz edited edge metadata.Oct 27 2015, 9:47 AM

Can you separate out LLVM_DYLIB_NAME into a separate patch? Since that will also require support for llvm-config it might be nice to not have it attached to this patch.

Thanks,
-Chris

axw updated this revision to Diff 38790.Oct 30 2015, 3:02 AM
axw edited edge metadata.
  • Drop LLVM_DYLIB_NAME changes
  • Use "cmake-default" as the magic string to not set any SOVERSION/VERSION property
axw retitled this revision from [CMake] make name of libLLVM configurable to [CMake] make SONAME of libLLVM configurable.Oct 30 2015, 3:04 AM
axw updated this object.
axw edited edge metadata.
beanz added a comment.Nov 2 2015, 9:59 AM

I have no objection to this patch, but what is the motivation?

As far as I can tell this doesn't actually address the problems from PR25059, and it doesn't support making the name match autoconf.

Are there specific name transformations that you need that this supports?

beanz added a comment.Nov 2 2015, 11:26 AM

According to Tom my other patch addresses his problem (see D13841). Can you please take a look at that patch and see if it addresses what you need as well?

axw added a comment.Nov 2 2015, 4:33 PM

I have no objection to this patch, but what is the motivation?

As far as I can tell this doesn't actually address the problems from PR25059, and it doesn't support making the name match autoconf.

Not directly. My intention was to leave switching the defaults to a follow-up.

Are there specific name transformations that you need that this supports?

Yes, Debian uses a slightly different SONAME in its builds (e.g. for LLVM 3.8.0, SONAME is libLLVM-3.8.so.1), and would also like to maintain compatibility. I was hoping to cut down on the number of patches that Debian makes on top of LLVM, but perhaps it's not a good argument for adding this complexity to the LLVM project.

beanz added a comment.Nov 2 2015, 4:37 PM

@axw, your patches are way less complex than mine, but I'm not sure it is possible to match the autoconf system without overriding the output name and more or less replacing the symlink creation code.

Perhaps the right approach is to land my changes and rebase yours on top to add the customization you need. Does that sound reasonable to you?

axw abandoned this revision.Nov 7 2015, 11:08 PM

I don't think this is necessary now. Debian will just need to patch to set the SOVERSION, but that's a Debian-specific thing that doesn't belong in the LLVM tree.