-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Install generated API headers into LLDB.framework #90666
Conversation
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) ChangesFull diff: https://github.com/llvm/llvm-project/pull/90666.diff 1 Files Affected:
diff --git a/lldb/cmake/modules/LLDBFramework.cmake b/lldb/cmake/modules/LLDBFramework.cmake
index f915839f6b45a5..a461f3ee3c9c59 100644
--- a/lldb/cmake/modules/LLDBFramework.cmake
+++ b/lldb/cmake/modules/LLDBFramework.cmake
@@ -71,6 +71,7 @@ endif()
# At configuration time, collect headers for the framework bundle and copy them
# into a staging directory. Later we can copy over the entire folder.
file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
+file(GLOB built_public_headers ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/../tools/lldb/include/lldb/API/*.h)
file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
list(REMOVE_ITEM root_public_headers ${root_private_headers})
@@ -80,6 +81,7 @@ find_program(unifdef_EXECUTABLE unifdef)
set(lldb_header_staging ${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders)
foreach(header
${public_headers}
+ ${built_public_headers}
${root_public_headers})
get_filename_component(basename ${header} NAME)
|
628576b
to
6950a36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me. Thanks for fixing this!
lldb/cmake/modules/LLDBConfig.cmake
Outdated
if (LLDB_BUILT_STANDALONE) | ||
set(LLDB_OBJ_DIR ${CMAKE_CURRENT_BINARY_DIR}) | ||
else() | ||
set(LLDB_OBJ_DIR ${CMAKE_CURRENT_BINARY_DIR}/tools/lldb) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLDBConfig
shouldn't know about the standalone build. You should set it to CMAKE_CURRENT_BINARY_DIR
here and overwrite it in LLDBStandalone.cmake
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLDBConfig is included after LLDBStandalone, so that can't work right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like when it's evaluated here it actually points to tools/lldb
so just setting the variable to CMAKE_CURRENT_BINARY_DIR here seems to do the trick. Unless it's undefined which directory we get based on context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, thinking about this further that does make sense. CMake will create a subdirectory in your build tree for lldb at $BUILD_DIR/tools/lldb
and will set CMAKE_CURRENT_BINARY_DIR
to that subdirectory in that context. For standalone builds, LLDB is its own standalone project, so it gets set to $BUILD_DIR
in that case. I think you can just set LLDB_OBJ_DIR
to ${CMAKE_CURRENT_BINARY_DIR}
and call it a day.
6950a36
to
9510a08
Compare
I had to replace the GLOB with a hardcoded file name because the GLOB gets evaluated at CMake configure time, when the file doesn't exist yet. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
9510a08
to
d1b444a
Compare
d1b444a
to
88db878
Compare
(cherry picked from commit b88d211) Conflicts: lldb/packages/Python/lldbsuite/test/builders/builder.py
(cherry picked from commit b88d211) Conflicts: lldb/packages/Python/lldbsuite/test/builders/builder.py
No description provided.