[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment. As this is a big deal for packager, maybe it should be part of the release notes ? (maybe it is and I just missed it :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 __

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. Can you let me know if D106974 fixes the problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. It looks like that function is missing from libclang.map, I think if you add it there it should fix the test. I'm going to check to see if there are any more missing functions and then commit a fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment. In D105527#2910616 , @tstellar wrote: > In D105527#2910319 , @jrtc27 wrote: > >> I'm seeing the test fail locally (on >> ca0fe3447fb85762838468537d93d4ef82c5a1af >>

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. In D105527#2910319 , @jrtc27 wrote: > I'm seeing the test fail locally (on ca0fe3447fb85762838468537d93d4ef82c5a1af > ) with: > > 000d1799 t clang_Co

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-28 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment. I'm seeing the test fail locally with: 000d1799 t clang_CompileCommand_getNumMappedSources being found by grep. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-27 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. In D105527#2908944 , @Bigcheese wrote: > Also, this python script just doesn't work. It's missing a sys import, a "w" > flag, and a new line after each write. It also dumps the output into the > source directory. Was this actua

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-27 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese added a comment. Here's a version that actually works (python 3, not sure if it's valid in 2), although I would much prefer we not write to the source directory during a build. import re import os import sys input_file = open(sys.argv[1]) with open(sys.argv[2], "w") as ou

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-27 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese added a comment. Also, this python script just doesn't work. It's missing a sys import, a "w" flag, and a new line after each write. It also dumps the output into the source directory. Was this actually tested? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:79 + set(LLVM_EXPORTED_SYMBOL_FILE) + set(USE_VERSION_SCRIPT TRUE) +endif() jsji wrote: > I think we should check LLVM_HAVE_LINK_VERSION_SCRIPT here, there are > platforms that does NO

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:79 + set(LLVM_EXPORTED_SYMBOL_FILE) + set(USE_VERSION_SCRIPT TRUE) +endif() I think we should check LLVM_HAVE_LINK_VERSION_SCRIPT here, there are platforms that does NOT support VERSIO

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. In D105527#2906247 , @Bigcheese wrote: > For future reference this was very difficult to merge into external changes. > It looks like you resorted this at the same time as renaming it, and that > messes up git's rename logic.

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments. Comment at: clang/test/LibClang/symbols.test:2 +# Check that there are no unversioned clang symbols in libclang.so +RUN: llvm-nm -Dj --defined-only %libclang | grep -v -e '@@LLVM_[0-9]\+$' | not grep '^clang' + thakis wrote: > I thi

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese added a comment. For future reference this was very difficult to merge into external changes. It looks like you resorted this at the same time as renaming it, and that messes up git's rename logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments. Comment at: clang/test/LibClang/symbols.test:2 +# Check that there are no unversioned clang symbols in libclang.so +RUN: llvm-nm -Dj --defined-only %libclang | grep -v -e '@@LLVM_[0-9]\+$' | not grep '^clang' + I think it's possible

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:67 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.exports) +set(LIBCLANG_VERSION_SCRIPT_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map) thakis wrote: > This is s

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:67 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.exports) +set(LIBCLANG_VERSION_SCRIPT_FILE ${CMAKE_CURRENT_SOURCE_DIR}/libclang.map) This is still set to CURREN

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Tom Stellard via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc7b3a91017d2: libclang.so: Make SONAME independent from LLVM version (authored by tstellar). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 _

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 Thread Tom Stellard via Phabricator via cfe-commits
tstellar updated this revision to Diff 361734. tstellar added a comment. Add test case for local symbols and reformat version script. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 Files: clang/test/CM

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/LibClang/lit.local.cfg:1 + +config.substitutions.append(('%libclang', os.path.join(config.clang_lib_dir, 'libclang.so'))) delete blank line Comment at: clang/test/LibClang/symbols.test:2 +#

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Tom Stellard via Phabricator via cfe-commits
tstellar updated this revision to Diff 360656. tstellar added a comment. Add missing file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 Files: clang/test/CMakeLists.txt clang/test/LibClang/lit.loca

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Tom Stellard via Phabricator via cfe-commits
tstellar updated this revision to Diff 360654. tstellar added a comment. Added a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 Files: clang/test/CMakeLists.txt clang/test/LibClang/lit.loc

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} MaskRay wrote: > tstellar wrote: > > MaskRay wrote: > > > What does this do? > > > > > >

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} tstellar wrote: > MaskRay wrote: > > What does this do? > > > > A hard-coded list cannot

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} MaskRay wrote: > What does this do? > > A hard-coded list cannot catch up with the real

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Tom Stellard via Phabricator via cfe-commits
tstellar updated this revision to Diff 360517. tstellar marked 3 inline comments as done. tstellar added a comment. Address some review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527 Files: c

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} What does this do? A hard-coded list cannot catch up with the real dynamic symbol list.

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-06 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment. I tried to keep this simple and only add the Linux support for now. There seem to be some subtle differences of how different operating systems handle export lists and linker scripts, and I wasn't confident about how to make this work on those systems. Repository:

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-06 Thread Tom Stellard via Phabricator via cfe-commits
tstellar created this revision. tstellar added reviewers: dim, MaskRay. Herald added a subscriber: mgorny. tstellar requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D105527 Files: clang/tools/libclang/CMakeLists.tx