[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-25 Thread Tobias Hieta via Phabricator via cfe-commits
thieta added a comment. In D132486#3748546 , @thakis wrote: > Was this reviewed by anyone on the original change? As far as I can tell, > there was no agreement on the original change or here that reverting is the > way to go. Was this discussed elsewhe

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-25 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Was this reviewed by anyone on the original change? As far as I can tell, there was no agreement on the original change or here that reverting is the way to go. Was this discussed elsewhere? (I don't have an opinion on which approach is better myself.) Repository: rG

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-25 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added a comment. I have another question, probably mainly for @tstellar (since I don't understand the `clang/tools/libclang/libclang.{exports,map}` infrastructure). Now that we're defaulting back to the equality case, would we need to reinstate `libclang.exports`? Repository: rG

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-25 Thread Tobias Hieta via Phabricator via cfe-commits
thieta added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:9-12 +else() + # ... unless explicily overridden + set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR}) +endif() h-vetinari wrote: > Sorry I didn't get to comment in time, but now that the

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-25 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:9-12 +else() + # ... unless explicily overridden + set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR}) +endif() Sorry I didn't get to comment in time, but now that the default was swit

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-24 Thread Tobias Hieta via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG0f28d4856630: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION (authored by h-vetinari, committed by thiet

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-24 Thread Tobias Hieta via Phabricator via cfe-commits
thieta added a comment. @jrtc27 What do you think about this patch with the default flipped? I think this is how it should land personally as discussed on discourse. I have been trying to listen in how people want to handle this and I hope this is a good middle ground that we can agree on for 1

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-24 Thread Tobias Hieta via Phabricator via cfe-commits
thieta updated this revision to Diff 455088. thieta added a comment. Fixed variable name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132486/new/ https://reviews.llvm.org/D132486 Files: clang/CMakeLists.txt clang/docs/ReleaseNotes.rst clang

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-23 Thread Tobias Hieta via Phabricator via cfe-commits
thieta updated this revision to Diff 455087. thieta added a comment. Updated the option to default to ON - meaning we keep the current behavior found in main. This rationel for this is explained in my post here: https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-23 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added a comment. > My concerns have already been raised by others in that thread and related > issues, I see no point in restating them yet again. I don't see consensus, I > see a handful of people discussing reverting a change that broke a whole > bunch of assumptions made by real-w

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-23 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment. In D132486#3743461 , @h-vetinari wrote: > Thanks for the review. Given that you have concerns, could you voice them in > a larger forum > (https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground-to-k

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-23 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added a comment. Thanks for the review. Given that you have concerns, could you voice them in a larger forum (https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground-to-keep-it-behind-option/64410), where so far the direction was in favour of going back t

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-08-23 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: clang/CMakeLists.txt:467 +option(CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION + "Force the SOVERSION of libclang to be equal to CLANG_MAJOR" OFF) +