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

feat: add CMakeLists.txt to support CPM #116

Merged
merged 1 commit into from
Mar 31, 2022
Merged

feat: add CMakeLists.txt to support CPM #116

merged 1 commit into from
Mar 31, 2022

Conversation

aminya
Copy link
Owner

@aminya aminya commented Mar 28, 2022

Co-Authored-By: @ClausKlein

Cherry-picked from #105

  • test that Conan works in the projects that use project_options

@ClausKlein
Copy link
Contributor

Great.
By the way, xhy do you not use a symbolic link to Index.cmake?

@aminya
Copy link
Owner Author

aminya commented Mar 29, 2022

Great.
By the way, xhy do you not use a symbolic link to Index.cmake?

Because symlinks on Windows require admin permission

@aminya
Copy link
Owner Author

aminya commented Mar 29, 2022

@ddalcino Could you test this change with Conan? I don't have many Conan projects to test this with

@ddalcino
Copy link
Contributor

ddalcino commented Mar 30, 2022

@ddalcino Could you test this change with Conan? I don't have many Conan projects to test this with

I must be missing some context here. I'm not sure what you're asking for. I can test this manually with CMake Package Manager if that will help. As I understand it, CPM is the package manager that needs this change?

Are you asking me to create a Conan package that uses project_options, and see if I can use it in a dependent project? That's probably a good idea, but I think it should be tested continuously rather than as a one-off.

@ClausKlein
Copy link
Contributor

ClausKlein commented Mar 30, 2022

@ddalcino Could you test this change with Conan? I don't have many Conan projects to test this with

I tested under ubuntu this: https://github.com/ClausKlein/cpp_starter_project/commit/d690b81ec76351218bf8b08d52651fd2ceb3f474

and here
cpp-best-practices/gui_starter_template#207

@ddalcino
Copy link
Contributor

I have tested this, and I can confirm that it works as intended. CI logs are here: https://github.com/ddalcino/cpp_starter_project/actions/runs/2072971834

For completeness, I have also verified that CPM does not load project_options properly without this change.

I think it could be really useful to add directions to the README about how to fetch project_options properly using CPM; I didn't know how to do it myself.

@aminya aminya merged commit 4e2b395 into main Mar 31, 2022
@aminya aminya deleted the CPM-support branch March 31, 2022 19:56
@aminya
Copy link
Owner Author

aminya commented Mar 31, 2022

Thanks for the testing!

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.

3 participants