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

ProjectOptions_SRC_DIR is now cached as INTERNAL #163

Merged
merged 2 commits into from
Oct 14, 2022

Conversation

SK83RJOSH
Copy link
Contributor

Fixes #161

src/Index.cmake Outdated
@@ -11,7 +11,7 @@ endif()
include_guard()

# only useable here
set(ProjectOptions_SRC_DIR ${CMAKE_CURRENT_LIST_DIR})
set(ProjectOptions_SRC_DIR ${CMAKE_CURRENT_LIST_DIR} CACHE INTERNAL "")
Copy link
Contributor

Choose a reason for hiding this comment

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

This must not a CMakeCache variable because if more than one subproject use this ProjectOptions independently, there is a name clash!

Copy link
Owner

Choose a reason for hiding this comment

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

We should probably add a note about this. If someone uses project options in multiple submodules, it can result in errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue we're running into is that this variable is seemingly out of scope by the time a lot of project_option macros are called. The only solution I can think of would be to call the includes before macro definitions – but I'm not sure what the impact on build times would be since I guess the inclusions are delayed intentionally?

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue we're running into is that this variable is seemingly out of scope by the time a lot of project_option macros are called. The only solution I can think of would be to call the includes before macro definitions – but I'm not sure what the impact on build times would be since I guess the inclusions are delayed intentionally?

you may try to change the project_options/CMakeLists.txt and append the CMAKE_MODULE_PATH ...

@aminya aminya merged commit fbb9056 into aminya:main Oct 14, 2022
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.

Can no longer include project_options as a submodule
3 participants