Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use target_include_directories() in CMakeLists.txt File #190

Closed
no-more-secrets opened this issue Nov 30, 2018 · 1 comment
Closed

Use target_include_directories() in CMakeLists.txt File #190

no-more-secrets opened this issue Nov 30, 2018 · 1 comment

Comments

@no-more-secrets
Copy link
Contributor

no-more-secrets commented Nov 30, 2018 •
edited
Loading

When libucl is included as a submodule of a parent project then one will incorporate it by using CMake's add_subdirectory combined with the target_link_libraries( parent_target ucl ). However, this currently does not work because libucl's top-level CMakeLists.txt file does not handle include folders in a way that supports this: it issues a number of include_directories calls which don't (and shouldn't) propagate to parent projects.

Instead, it should be using target_include_directories on the ucl target. This will produce the correct behavior for all targets in the project as well as parent projects for both standalone builds and builds in a parent project. Furthermore, as is now considered good practice, we should create a target alias ucl::ucl.

Can I submit a PR that does this?
@no-more-secrets
Copy link
Contributor Author

Should I just submit a PR?

no-more-secrets added a commit to no-more-secrets/libucl that referenced this issue Dec 3, 2018
@no-more-secrets
Modernize CMake file with target_include_directories()
7e4e517 
This will associate include directories with individual
targets (and their dependencies, automatically) instead
of setting them at the directory level.  This allows the
library to be used as a subdirectory of a parent project
whose targets depend on this one.

Also, create some namespace'd aliases for the two ucl
targets, which is also in line with modern style.

Fixes vstakhov#190
no-more-secrets added a commit to no-more-secrets/libucl that referenced this issue Dec 3, 2018
@no-more-secrets
Modernize CMake file with target-based includes.
734fe5f 
This will associate include directories with individual targets
using target_include_directories() instead of setting them at the
directory level, which is the old style. This allows the library
to be used as a sub-directory of a parent project whose targets
depend on this one and CMake will handle transitive include
directories based on the PUBLIC/PRIVATE parameter values we give.

Also, create some namespace'd aliases for the two ucl targets,
which is also in line with modern style.

Tested building both the ucl and lua-ucl targets.

Fixes vstakhov#190
@no-more-secrets no-more-secrets mentioned this issue Dec 3, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@no-more-secrets