[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 318003.
tstellar added a comment.

Fix shared object symlinks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

Files:
  clang/CMakeLists.txt
  clang/tools/clang-shlib/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -150,6 +150,7 @@
   else()
 set_target_properties(libclang
   PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION}
   VERSION ${LIBCLANG_LIBRARY_VERSION}
   DEFINE_SYMBOL _CINDEX_LIB_)
 # FIXME: _CINDEX_LIB_ affects dllexport/dllimport on Win32.
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,8 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+
+set_target_properties(clang-cpp
+  PROPERTIES
+  VERSION ${LIBCLANG_LIBRARY_VERSION}
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -509,7 +509,7 @@
 "${CLANG_VERSION_MAJOR}" CACHE STRING
 "Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
-"${CLANG_VERSION_MAJOR}" CACHE STRING
+"${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
 "Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -150,6 +150,7 @@
   else()
 set_target_properties(libclang
   PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION}
   VERSION ${LIBCLANG_LIBRARY_VERSION}
   DEFINE_SYMBOL _CINDEX_LIB_)
 # FIXME: _CINDEX_LIB_ affects dllexport/dllimport on Win32.
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,8 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+
+set_target_properties(clang-cpp
+  PROPERTIES
+  VERSION ${LIBCLANG_LIBRARY_VERSION}
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -509,7 +509,7 @@
 "${CLANG_VERSION_MAJOR}" CACHE STRING
 "Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
-"${CLANG_VERSION_MAJOR}" CACHE STRING
+"${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
 "Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/tools/clang-shlib/CMakeLists.txt:54
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})

cuviper wrote:
> tstellar wrote:
> > cuviper wrote:
> > > I think you also need `VERSION` here, or else it symlinks back to 
> > > `.so.11`.
> > I think add_clang_library is setting VERSION.  This is what I get when I 
> > build:
> > 
> > lib/libclang-cpp.so -> libclang-cpp.so.11.1
> > lib/libclang.so -> libclang.so.11.1
> > 
> I see those links, but also `lib/libclang-cpp.so.11.1 -> libclang-cpp.so.11`, 
> with the latter being the real file. If I add `VERSION` explicitly, that goes 
> away.
> (I had to delete `lib/libclang*.so*` before rebuilding too.)
OK, I see it now.  I can fix that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Josh Stone via Phabricator via cfe-commits]

cuviper added inline comments.



Comment at: clang/tools/clang-shlib/CMakeLists.txt:54
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})

tstellar wrote:
> cuviper wrote:
> > I think you also need `VERSION` here, or else it symlinks back to `.so.11`.
> I think add_clang_library is setting VERSION.  This is what I get when I 
> build:
> 
> lib/libclang-cpp.so -> libclang-cpp.so.11.1
> lib/libclang.so -> libclang.so.11.1
> 
I see those links, but also `lib/libclang-cpp.so.11.1 -> libclang-cpp.so.11`, 
with the latter being the real file. If I add `VERSION` explicitly, that goes 
away.
(I had to delete `lib/libclang*.so*` before rebuilding too.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/tools/clang-shlib/CMakeLists.txt:54
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})

cuviper wrote:
> I think you also need `VERSION` here, or else it symlinks back to `.so.11`.
I think add_clang_library is setting VERSION.  This is what I get when I build:

lib/libclang-cpp.so -> libclang-cpp.so.11.1
lib/libclang.so -> libclang.so.11.1



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Josh Stone via Phabricator via cfe-commits]

cuviper added inline comments.



Comment at: clang/tools/clang-shlib/CMakeLists.txt:54
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})

I think you also need `VERSION` here, or else it symlinks back to `.so.11`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

I guess this will be clearly mentioned in the release notes :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru accepted this revision.
sylvestre.ledru added a comment.
This revision is now accepted and ready to land.

Let's do it then :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Josh Stone via Phabricator via cfe-commits]

cuviper added a comment.

If the SONAME is different in any way, it is totally distinct to the dynamic 
linker. There's no semver parsing or anything like that. That's all we can do 
from a technical perspective, and the rest is just communication in the release.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Yeah, my question is more "is that enough for distro?"
https://www.debian.org/doc/manuals/maint-guide/advanced.en.html
https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html

I might be wrong but if 11.1 and 11.0 aren't compatible (not just adding a new 
function), I was wondering if if should not be 12 (I know it is terrible).
Maybe we can expect that packagers will know that 11.1 is like a 12 in term of 
soname


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D94941#2509829 , @sylvestre.ledru 
wrote:

> I might be wrong but if the ABI is incompatible, are we not supposed to 
> update the SONAME itself?

That's what this patch does, it updates the SONAME from *-11 to *-11.1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

I might be wrong but if the ABI is incompatible, are we not supposed to update 
the SONAME itself?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

@sylvestre.ledru removed the minor version from the binary (on purpose, I 
think?) in rGa8b717fda42294d1c8e1f05d71280503e5839f14 
:

  commit a8b717fda42294d1c8e1f05d71280503e5839f14
  Author: Sylvestre Ledru 
  Date:   Thu Mar 29 10:05:46 2018 +
  
  Rename clang link from clang-X.Y to clang-X
  
  Summary:
  As we are only doing X.0.Z releases (not using the minor version), there 
is no need to keep -X.Y in the version.
  So, instead, I propose the following:
  Instead of having clang-7.0 in bin/, we will have clang-7
  
  Since also matches was gcc is doing.
  
  Reviewers: tstellar, dlj, dim, hans
  
  Reviewed By: dim, hans
  
  Subscribers: dim, mgorny, cfe-commits
  
  Differential Revision: https://reviews.llvm.org/D41808
  
  llvm-svn: 328769

But this seems to have been mainly about the clang executable that gets 
installed into `$prefix/bin`, i.e. `clang-11` instead of `clang-11.1`.

I think this doesn't matter for the clang executable here, since nobody uses 
its ABI. But indeed for the shared library it looks like a good solution, if 
you would want to install both versions side-by-side.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94941/new/

https://reviews.llvm.org/D94941

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94941: Add minor version to libclang.so and libclang-cpp.so SONAME

[Tom Stellard via Phabricator via cfe-commits]

tstellar created this revision.
tstellar added a reviewer: sylvestre.ledru.
Herald added a subscriber: mgorny.
tstellar requested review of this revision.
Herald added a project: clang.

This patch is for the release/11.x branch.  We need to bump the SONAME, because
the ABI of the shared library is changing


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94941

Files:
  clang/CMakeLists.txt
  clang/tools/clang-shlib/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -150,6 +150,7 @@
   else()
 set_target_properties(libclang
   PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION}
   VERSION ${LIBCLANG_LIBRARY_VERSION}
   DEFINE_SYMBOL _CINDEX_LIB_)
 # FIXME: _CINDEX_LIB_ affects dllexport/dllimport on Win32.
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,7 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+
+set_target_properties(clang-cpp
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -509,7 +509,7 @@
 "${CLANG_VERSION_MAJOR}" CACHE STRING
 "Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
-"${CLANG_VERSION_MAJOR}" CACHE STRING
+"${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
 "Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -150,6 +150,7 @@
   else()
 set_target_properties(libclang
   PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION}
   VERSION ${LIBCLANG_LIBRARY_VERSION}
   DEFINE_SYMBOL _CINDEX_LIB_)
 # FIXME: _CINDEX_LIB_ affects dllexport/dllimport on Win32.
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,7 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+
+set_target_properties(clang-cpp
+  PROPERTIES
+  SOVERSION ${LIBCLANG_LIBRARY_VERSION})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -509,7 +509,7 @@
 "${CLANG_VERSION_MAJOR}" CACHE STRING
 "Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
-"${CLANG_VERSION_MAJOR}" CACHE STRING
+"${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
 "Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits