This is an archive of the discontinued LLVM Phabricator instance.

Differential D145181

Revert "Remove the LINK_COMPONENTS entry from lldb-instr CMakery"
ClosedPublic

Authored by aheejin on Mar 2 2023, 1:15 PM.

Details

Reviewers
jingham
JDevlieghere
Commits
rGc4125a37806a: Revert "Remove the LINK_COMPONENTS entry from lldb-instr CMakery"
Summary

This reverts commit e12a950d90f88aeddaa97d6e7c8fd0bfedc42f73.

D142241 broke -sBUILD_SHARED_LIBS=ON build. I commented on that CL
about this issue, and also asked the same question on the LLVM discourse
(https://discourse.llvm.org/t/build-shared-libs-on-doesnt-work-with-lldb-instr/68793),
but neither was answered.

As I asked on the post, it looks the CL description says having
CLANG_LIBS and LINK_COMPONENTS Support within a single file is a
problem, but there are many files in lldb that use both, so I don't
understand what was the issue:

./lldb/source/Core/CMakeLists.txt
./lldb/source/Plugins/TypeSystem/Clang/CMakeLists.txt
./lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeLists.txt
./lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
./lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
./lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
./lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
./lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
./lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt

I'd hope to at least get the answers why D142241 was necessary this
time.

Diff Detail

Event Timeline

aheejin created this revision.Mar 2 2023, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 1:15 PM
Herald added a subscriber: wingo. · View Herald Transcript
aheejin requested review of this revision.Mar 2 2023, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 1:15 PM
Herald added a subscriber: lldb-commits. · View Herald Transcript
aheejin edited the summary of this revision. (Show Details)Mar 2 2023, 1:15 PM
Harbormaster completed remote builds in B217038: Diff 501958.Mar 2 2023, 1:18 PM
JDevlieghere added a comment.Mar 2 2023, 3:01 PM

This will break the Xcode build again. It's being discussed here: https://github.com/llvm/llvm-project/issues/60314

aheejin added a comment.Mar 8 2023, 3:49 PM

Thanks for the info. I asked questions in https://github.com/llvm/llvm-project/issues/60314 but they were not answered either. What should I do with this CL? Can you at least let me know why this wouldn't work given that there are many files like this?

JDevlieghere accepted this revision.EditedMar 20 2023, 6:43 PM

LGTM (see https://github.com/llvm/llvm-project/issues/60314#issuecomment-1477168717)

This revision is now accepted and ready to land.Mar 20 2023, 6:43 PM
Closed by commit rGc4125a37806a: Revert "Remove the LINK_COMPONENTS entry from lldb-instr CMakery" (authored by aheejin). · Explain WhyMar 22 2023, 10:54 AM
This revision was automatically updated to reflect the committed changes.
aheejin added a commit: rGc4125a37806a: Revert "Remove the LINK_COMPONENTS entry from lldb-instr CMakery".
aheejin added a comment.Mar 22 2023, 10:54 AM

Thanks for the investigation!

Revision Contents

PathSize
lldb/
tools/
lldb-instr/
2 lines

Diff 507429

lldb/tools/lldb-instr/CMakeLists.txt

add_lldb_tool(lldb-instr add_lldb_tool(lldb-instr
Instrument.cpp Instrument.cpp
CLANG_LIBS CLANG_LIBS
clangAST clangAST
clangBasic clangBasic
clangCodeGen clangCodeGen
clangFrontend clangFrontend
clangLex clangLex
clangRewrite clangRewrite
clangSerialization clangSerialization
clangTooling clangTooling
LINK_COMPONENTS
Support
) )