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

Fix cmake installation #334

Closed
wants to merge 3 commits into from
Closed

Conversation

ToKiNoBug
Copy link
Contributor

Issue #324 mentioned that installation fails on many platforms because
komputeConfigVersion.cmake is missing. I tried and found out that this issue happens on may platforms, (for example, on Windows11 with msvc and clang16, and on archlinux with gcc13 and clang16).

So I add some lines to generate it, and I believe this will fix #324 .

@ToKiNoBug
Copy link
Contributor Author

Commit 8ea8cd226df82716f57f65651bcb24b9e12e63f5 aims to fix header installation. An extra install command in src/include/CMakeLists.txt is removed because it requires non-existing directory src/include/logger, but logger headers are in src/include/kompute/logger. This line is redundant because logger headers are already installed with line 30.

install(DIRECTORY kompute DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})

@ToKiNoBug ToKiNoBug changed the title Generate komputeConfigVersion.cmake to fix cmake installation Fix cmake installation Oct 25, 2023
write_basic_package_version_file(
"${PROJECT_BINARY_DIR}/kompute/komputeConfigVersion.cmake"
VERSION ${CMAKE_PROJECT_VERSION}
COMPATIBILITY AnyNewerVersion # Other possible values: SameMajorVersion, SameMinorVersion, ExactVersion
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably limiting to Major version would make sense

@axsaucedo
Copy link
Member

Thank you for the contribution, added a minor comment but overall looks good, if you can update (+ update DCO) should be good to merge

@ToKiNoBug
Copy link
Contributor Author

Thank you for the contribution, added a minor comment but overall looks good, if you can update (+ update DCO) should be good to merge

Thanks for your appearciation, I will work on this.

@ToKiNoBug
Copy link
Contributor Author

I will start a new pr with verified commits.

@ToKiNoBug ToKiNoBug closed this Oct 25, 2023
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.

After compilation, only the library file has no header file
2 participants