xiaobai requested changes to this revision.
xiaobai added a comment.
This revision now requires changes to proceed.
I think this might actually not do what we want it to do. I've commented inline
with my concerns.
================
Comment at: cmake/modules/LLDBFramework.cmake:21
add_custom_command(TARGET lldb-framework POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_directory
${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders $<TARGET_FILE_DIR:liblldb>/Headers
COMMAND ${CMAKE_COMMAND} -E create_symlink Versions/Current/Headers
${LLDB_FRAMEWORK_DIR}/LLDB.framework/Headers
----------------
keith wrote:
> xiaobai wrote:
> > This line shouldn't be necessary anymore then, right?
> It still is, the change below doesn't actually do the copying, it only runs
> the script on the location the headers have already been copied too.
Are you saying that it will fix the framework headers that have already been
copied? If so, then I don't think this will work, because the copy occurs after
lldb-framework has been built, and lldb-framework depends on
lldb-framework-headers.
================
Comment at: cmake/modules/LLDBFramework.cmake:41
+add_custom_target(lldb-framework-headers
+ DEPENDS ${framework_headers}
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/framework-header-fix.sh
----------------
This should not just depend on `framework_headers`, because those are in the
location `${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders`. You want to run the
script on `$<TARGET_FILE_DIR:liblldb>/Headers`, which is not guaranteed to
exist or have been populated with headers when this actually gets run. See my
comment above.
https://reviews.llvm.org/D49779
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
