Support INTERFACE include directories in ROOT_GENERATE_DICTIONARY #942
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CMake offers three visibility qualifiers for target include directories, which are populated to target properties as shown in the following table: | | INTERFACE | PUBLIC | PRIVATE | | INCLUDE_DIRECTORIES | | x | x | | INTERFACE_INCLUDE_DIRECTORIES | X | x | | For dictionary generation the PUBLIC and INTERFACE qualifiers and hence the INTERFACE_INCLUDE_DIRECTORIES are to be preferred, because header files meant to be consumed by the user are usually put into PUBLIC and/or INTERFACE qualified directories. Furthermore, the CMake imported targets always have INTERFACE visibility. This commit changes the current behaviour to read the INTERFACE_INCLUDE_DIRECTORIES (as opposed to the INCLUDE_DIRECTORIES) target property which will catch more desired use cases, including imported targets. In other words, this will now ignore PRIVATE include directories, but include INTERFACE include directories - PUBLIC ones stay unchanged. In addition, this commit adds a condition which ignores include directories formulated as a CMake generator expression. Unfortunately, there is currently no way to evaluate those seperately.
|
Can one of the admins verify this patch? |
3 tasks
Turns out, naked absolute include directories as target properties will prohibit those targets from being exported with INSTALL(EXPORT ...). Therefore, one cannot ignore the $<BUILD_INTERFACE:...> type generator expressions. This commit will add another condition which detects BUILD_INTERFACE generator expressions and extracts the value which is then added to the list of include directories for rootcling.
Contributor
|
@phsft-bot build |
|
Starting build on |
Contributor
Author
|
Do not merge yet, I found a bug, will update on Monday. |
Contributor
Author
|
Closing this. There are much bigger and invasive changes needed, so I decided to just implement |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CMake offers three visibility qualifiers for target include
directories, which are populated to target properties as shown in the
following table:
INTERFACEPUBLICPRIVATEINCLUDE_DIRECTORIESINTERFACE_INCLUDE_DIRECTORIESFor dictionary generation the
PUBLICandINTERFACEqualifiers andhence the
INTERFACE_INCLUDE_DIRECTORIESare to be preferred, becauseheader files meant to be consumed by the user are usually put into
PUBLICand/orINTERFACEqualified directories. Furthermore, the CMakeimported targets always have
INTERFACEvisibility.This commit changes the current behaviour to read the
INTERFACE_INCLUDE_DIRECTORIES(as opposed to theINCLUDE_DIRECTORIES)target property which will catch more desired use cases, including
imported targets. In other words, this will now ignore
PRIVATEincludedirectories, but include
INTERFACEinclude directories -PUBLIConesstay unchanged.
In addition, this commit adds a condition which ignores include
directories formulated as a CMake generator expression. Unfortunately,
there is currently no way to evaluate those seperately.