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

cmake PROJECT_IS_TOP_LEVEL (#252) #255

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbaldwin
Copy link
Owner

This reverts commit f6b9b7f.

This commit needs to include package manager creation for conan and vcpkg before it can be merged since it broke building for at lesat conan.

This reverts commit f6b9b7f.

This commit needs to include package manager creation for conan and
vcpkg before it can be merged since it broke building for at lesat
conan.
@jbaldwin jbaldwin self-assigned this Feb 14, 2024
@jbaldwin
Copy link
Owner Author

@stripe2933 unfortunately a portion of the PR for PROJECT_IS_TOP_LEVEL broke the conan builds so we reverted that portion of it. I've created a revert revert of it to make sure it isn't lost since it seems like its a good idea still.

What probably needs to happen is a a github actions job to build the conan package needs to be added and pass with this change.

The reported bug user noticed that cmake 3.25 or greater didn't have a problem, but I don't think its a good idea to upgrade the project to that new of a cmake version yet.

@stripe2933
Copy link
Contributor

It's okay. I couldn't test the code validity for CMake < 3.25 since my environment is CMake 3.28. I hope if there is any solution for this PR's intention.

@jbaldwin
Copy link
Owner Author

See #259 CI addition for Conan so once that is merged I believe your original change should be testable and we can see if it'll work with cmake < 3.25

@uilianries
Copy link
Contributor

It's okay. I couldn't test the code validity for CMake < 3.25 since my environment is CMake 3.28. I hope if there is any solution for this PR's intention.

Hello! I see some options to have multiple CMake versions in your environment without breaking your current one.

  • You could use python pip with install cmake, but with a python environment (pyenv)
  • Or, you could use some C++ package manager like Conan to install and run using a temporary path.

@jbaldwin We could add a test with multiple CMake versions in the CI to check it, if you think it's okay.

@jbaldwin
Copy link
Owner Author

I think that is definitely a reasonable solution if we move forward again with this project is top level.

But let me ask another question, what about just switching the defaults to OFF? I had originally had them on since it's super nice if you are just exploring the repo but I think far more users at this point are probably linking to it so perhaps it's better to default to OFF?

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.

None yet

3 participants