Move -install project chaining to a more common place (fixes latest) #556
Conversation
There was a problem hiding this comment.
I don't get it, I need some time to understand what the problem is and what may have changed. CommonFindPackage should not have to manipulate install targets. There is one -install target per project, created by Common.cmake. So it seems to make perfect sense that add_subproject() adds this project-to-project dependency...
|
The issue is with optional dependencies which are not add_subproject()... Concretely in https://github.com/Eyescale/Equalizer/blob/master/examples/eqPly/CMakeLists.txt#L14, I could also add the dependency |
|
Ah of course! There can't be an install dependency on a library for which you don't add the sources... I found more other issues with the uxmal change: And I wonder why uxmal has a private dependency on ZeroEq? and a non-PUBLIC one on ZeroBuf? And what are CULLBUG_HEADERS? A grep on viz/stable gave me no results: |
In latest we can! ;) Let's see tomorrow. I wanted to avoid bloating Eq dependencies, maybe it's time to move uxmal to a separate project. |
eile
force-pushed
the
master
branch
4 times, most recently
from
6631b8b
to
18443f5
Compare
|
+1 |
CommonInstallProject.cmake
Outdated
| EXCLUDE_FROM_DEFAULT_BUILD ON) | ||
| endif() | ||
|
|
||
| # redo dependency chaining each time: a new common_find_package might have been called |
There was a problem hiding this comment.
the first inclusion of this file will be in Common.cmake, before any common_find_package() call. So this will only work if users include DoxygenRule.cmake at the end... A solution I can think of is to make this a function to be called from common_find_package_post() instead.
There was a problem hiding this comment.
? See doc: to be included by every module depending on ${PROJECT_NAME}-install.
CommonInstallProject.cmake
Outdated
| # doxygen and smoke tests. CMake does not provide this, so this is a workaround. | ||
| # install naturally depends on all, which depends on all common_library and | ||
| # common_application targets. Furthermore, ${PROJECT_NAME}-install depends on | ||
| # all subprojects which have a install rule, so that all necessary artefacts in |
CommonInstallProject.cmake
Outdated
| # install naturally depends on all, which depends on all common_library and | ||
| # common_application targets. Furthermore, ${PROJECT_NAME}-install depends on | ||
| # all subprojects which have a install rule, so that all necessary artefacts in | ||
| # a superproject build are installed. |
There was a problem hiding this comment.
document ${PROJECT_NAME}_FIND_PACKAGES_FOUND as input variable filled by common_find_package()
DoxygenRule.cmake
Outdated
| @@ -64,6 +64,7 @@ if(NOT TARGET ${PROJECT_NAME}-all) | |||
| endif() | |||
|
|
|||
| include(CommonDate) | |||
| include(CommonInstallProject) | |||
There was a problem hiding this comment.
There was no newline before, but if it makes you happy.
|
On second thought, I still think that the real problem is Equalizer trying to hide its subproject dependencies. This workaround is for a use case we should not support, because the hidden dependencies bypass the CI checks but may fail later in the super-project case. CommonFindPackage does not have anything to do with -install targets, which are project-to-project relationships for the Subprojects use case. |
|
No, the real problem is that CMake does not provide a Target-install target which installs the target fully. I'm happy to explain the details in person. |
|
ping |
CommonInstallProject.cmake
Outdated
| endif() | ||
|
|
||
| function(COMMON_ADD_INSTALL_DEPENDENCIES) | ||
| # redo dependency chaining each time: a new common_find_package might have been called |
CommonInstallProject.cmake
Outdated
|
|
||
| function(COMMON_ADD_INSTALL_DEPENDENCIES) | ||
| # redo dependency chaining each time: a new common_find_package might have been called | ||
| string(REPLACE " " ";" __deps "${ARGN}") # string-to-list |
There was a problem hiding this comment.
No longer needed I think, CMake expends the list when calling the function
No description provided.