Skip to content

Fix CMake targets include path propagation - #414

Open
FredeHoey wants to merge 2 commits into
MicrochipTech:mainfrom
FredeHoey:main
Open

Fix CMake targets include path propagation#414
FredeHoey wants to merge 2 commits into
MicrochipTech:mainfrom
FredeHoey:main

Conversation

@FredeHoey

Copy link
Copy Markdown

In all places where the cmake files call include_directories, the first and second parameters are a target and a scope specifier. For include_directories, that means literally adding those parameters to the include path via -I flags, ie.

include_directories(targetPUBLIC...) ->-Itarget-IPUBLIC

This was probably intended to be a target_include_directories instead, such that any library or executable calling target_link_libraries would also have those includes propagated

see:
https://cmake.org/cmake/help/latest/command/include_directories.htmlhttps://cmake.org/cmake/help/latest/command/target_include_directories.html#command:target_include_directories

Checklist

In all places where the cmake files call include_directories, the first
and second parameters are a target and a scope specifier. For
include_directories, that means literally adding those parameters to the
include path via -I flags, ie.
```cmake
include_directories(target PUBLIC ...) -> -Itarget -IPUBLIC
```
This was probably intended to be a target_include_directories instead,
such that any library or executable calling target_link_libraries would
also have those includes propagated
see:
https://cmake.org/cmake/help/latest/command/include_directories.htmlhttps://cmake.org/cmake/help/latest/command/target_include_directories.html#command:target_include_directories
@github-actions

Copy link
Copy Markdown

This PR has been marked as stale and will be automatically closed in 7 days.

@github-actions

Copy link
Copy Markdown

This PR has been marked as stale and will be automatically closed in 7 days.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@FredeHoey