Exclude cpptrace and its deps from install to avoid copying lib/include#3199
Exclude cpptrace and its deps from install to avoid copying lib/include#3199schell244 wants to merge 1 commit intovmangos:developmentfrom
Conversation
| include(cmake/InstallRules.cmake) | ||
| endif() | ||
| # Disabled: cpptrace is statically linked and not needed at runtime | ||
| # if(NOT CMAKE_SKIP_INSTALL_RULES) |
There was a problem hiding this comment.
Can we just CMAKE_SKIP_INSTALL_RULES=TRUE inside <root>/dep/CMakeLists.txt:24, so we don't modify the cpptrace tree.
It's hard to keep track of those changes if we upgrade the version of cpptrace.
There was a problem hiding this comment.
That's what I've tried with the first commit, but it broke the install process.
There was a problem hiding this comment.
Then let us try this in dep/CMakeLists.txt
set(PREV_SKIP_INSTALL_RULES ${CMAKE_SKIP_INSTALL_RULES})
set(CMAKE_SKIP_INSTALL_RULES TRUE)
add_subdirectory(cpptrace)
set(CMAKE_SKIP_INSTALL_RULES ${PREV_SKIP_INSTALL_RULES})( And also please add a comment explaining why we are doing that :P )
There was a problem hiding this comment.
Setting CMAKE_SKIP_INSTALL_RULES = TRUE prevents CMake from generating cpptrace/cmake_install.cmake. But it seems the parent install script dep/cmake_install.cmake (auto-generated) always tries to include install scripts from all subdirectories
There was a problem hiding this comment.
So what are we settling on? Is the current solution the best approach?
There was a problem hiding this comment.
@schell244 can you please document your current change in dep/cpptrace/README.md under "## Manual Changes"
|
Hmm, this doesn't seem to be working as intended for me; but maybe I'm misunderstanding something. I am currently already manually removing In preparation for this PR, I added some logging to confirm my manual cleanup would no longer be necessary when it gets merged. But when I build a Docker image from your PR branch, I am using a custom |
|
Oh ok, thanks for pointing that out. The |
5e533af to
c972274
Compare
c972274 to
44d2a9e
Compare
Ah, sorry, that would make sense. I somehow didn't consider checking what is actually inside these directories, otherwise I would've probably realized myself that what's left in there is not targeted by your original changes here. 😅 Have a nice vacation! |
🍰 Pullrequest
Skip install rules for cpptrace and its dependencies to avoid copying unnecessary
libandincludefolders.FetchContent_MakeAvailable so its install rules are not executed
Proof
Issues
How2Test
Todo / Checklist