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

Do not restrict CMAKE_MODULE_PATH to a single path #2805

Open
vaavaav opened this issue Oct 30, 2024 · 3 comments · May be fixed by #2806
Open

Do not restrict CMAKE_MODULE_PATH to a single path #2805

vaavaav opened this issue Oct 30, 2024 · 3 comments · May be fixed by #2806

Comments

@vaavaav
Copy link

vaavaav commented Oct 30, 2024

Is your feature request related to a problem? (你需要的功能是否与某个问题有关?)

More or less. It makes it harder to link bRPC against other projects.

Describe the solution you'd like (描述你期望的解决方法)

Instead of set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake), allow the user to provide other paths while still maintaining this one:
set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH}).
EDIT: list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake")

With this, developers can provide other paths where Find<library>.cmake files exist, in the command line (for example, cmake -DCMAKE_MODULE_PATH=opt/lib/cmake.

Describe alternatives you've considered (描述你想到的折衷方案)

I had to manually copy Find<library>.cmake files to the path indicated by ${PROJECT_SOURCE_DIR}/cmake...

Additional context/screenshots (更多上下文/截图)

@chenBright
Copy link
Contributor

Maybe it's better to use user specific paths first, like this: set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PROJECT_SOURCE_DIR}/cmake).

@vaavaav
Copy link
Author

vaavaav commented Oct 31, 2024

After searching a bit more, I stumble upon the suggestion of treating CMAKE_MODULE_PATH as a list of paths.
As such, list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake") is a much better practice.

@chenBright chenBright linked a pull request Oct 31, 2024 that will close this issue
@chenBright
Copy link
Contributor

chenBright commented Oct 31, 2024

Good idea! Please check #2806.

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 a pull request may close this issue.

2 participants