-
Notifications
You must be signed in to change notification settings - Fork 36
Avoid installing libraries multiple times or issueing wrong message #90
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
Conversation
Some libraries call `boost_install` themselves which adds `INSTALL_INTERFACE` to their `INTERFACE_INCLUDE_DIRETORIES` property. That then makes the install check in `__boost_auto_install` fail and print a message that the library won't be installed while it will be. Use a label to detect targets for which `boost_install` was already called and skip the logic in `__boost_auto_install` for those.
|
I don't think the LABELS property is the one to use. It means something different entirely. |
|
To me it sounds like they are arbitrary tags: https://cmake.org/cmake/help/v3.13/prop_tgt/LABELS.html
Using a Boost prefix to avoid them clashing with anything in the future |
|
This property holds the test labels. It also doesn't work at least on CMake 3.16 (LABELS is not a whiteisted property.) |
Since CMake 3.11 properties starting with underscores are considered custom properties and allowed.
|
According to the docs you can have labels for targets, source files, directories and tests. That's 2 different results in the end: Labels on tests and labels on targets with the 2nd not having any meaning. But seems like they didn't add that to the whitelist for INTERFACE targets. I found this in CMake 3.11:
That should be OK for our use case. If the property isn't set it will be false-ish so the check is easy |
|
I was going to suggest just checking the INTERFACE_INCLUDE_DIRECTORIES property for the value placed by |
|
That wasn't safe enough for me because an individual library could set those pretty conventional paths already, e.g. if it has a standalone version. Unfortunately there isn't any check if a specific export set exists or if a target is part of one, which would be a direct way to check for installation Anyway, CI looks good. |
It definitely would be. |
|
Adding it to this repo: #92 It could make sense to have it in the main Boost repo such that it is run on every submodule update. But here it also makes sense to verify that changes like this one work across CMake versions |
Some libraries call
boost_installthemselves which addsINSTALL_INTERFACEto theirINTERFACE_INCLUDE_DIRETORIESproperty. That then makes the install check in__boost_auto_installfail and print a message that the library won't be installed while it will be.Example:
Use a label to detect targets for which
boost_installwas already called and skip the logic in__boost_auto_installfor those.Libs possibly affected: