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.

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

Add include path to target #100

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gostefan
Copy link
Contributor

@gostefan gostefan commented Aug 7, 2024

This allows users of libcitygml to write simpler cmake files. They will only need target_link_libraries(citygml::citygml). (Plus finding the package of course.)

This PR is only about the last two commits. The others are included in other PRs (#96, #97, and #98). This PR is based on them to avoid conflicts when merging.

If these are public libraries a client build system needs to link
to them again even libcitygml was compiled into a self-contained
dynamic library.
@gostefan
Copy link
Contributor Author

gostefan commented Sep 4, 2024

I just now realized that commit 81e3787 is not good. INCLUDE_INSTALL_BASE_DIR should be INCLUDE_INSTALL_DIR I think.
Additionally commit 1045a00 can lead to wrong paths when installing the cmake files.
Hopefully I can verify this tonight.

@gostefan gostefan force-pushed the addIncludePathToTarget branch from e4d1644 to 68d11ab Compare September 4, 2024 20:09
@gostefan gostefan force-pushed the addIncludePathToTarget branch from 68d11ab to 8783e62 Compare September 15, 2024 13:39
@gostefan
Copy link
Contributor Author

Had to rebase this on the fixed version of PR #98. The two relevant commits still have the same contents.

CMake's install will combine the relative path with the
absolute path passed via CMAKE_INSTALL_PREFIX. This
shouldn't be handled in CMakeLists.txt files.

The state before resulted in absolute paths being
used in citygmlConfig.cmake. That prohibits prebuilding
the library and deploying it on other machines/paths.
It's unclear why the lib install folders worked without
this before. But now the CMake files get correctly
installed.
With this a library user can just use
target_link_libraries(citygml::citygml) and the
include directories are set up automatically.
When it's at the end CMake treats  and
as additional include directories when generating the
citygmlConfigInternal.cmake file. This is clearly wrong.
@gostefan gostefan force-pushed the addIncludePathToTarget branch from 8783e62 to c657724 Compare September 16, 2024 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant