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 fixes #86

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

CMake fixes #86

wants to merge 3 commits into from

Conversation

vizzerdrix
Copy link

HI
I was trying to add your project to mine and I've met some CMake problems. I've fixed them in my way (I had to keep company standards) and I would like to share it.

Generally, there are three problems.

  1. It's impossible to build a project from the directory that is outside of your source code tree. I've fixed path to your cmake_scripts directory, it's a trivial change (359058f)
  2. My project configuration forbids to place any files outside of PROJECT_BINARY_PATH, in past, that kind of files causes a lot of headaches because of naming collisions. Our build dir contains many subprojects and sometimes some of them try to put files with the same names in CMAKE_BINARY_PATH. That's why the global policy works as works. (5adf687)
  3. We also cannot break the shadow build idea. All generated, compiled, build-time-created files shall be placed in PROJECT_BINARY_PATH. I've removed LINK_DIRECTORIES line because I don't know why is needed in that context but maybe I'm wrong and it's needed, however.

So thanks for sharing your library, feel free if You want to use (or not :-) this pull request.

PS: I'm not CMake specialist, frankly speaking, I hate it, so my changes may not be the best.
Michal

E.g: When it's added as 3rd library using ADD_SUBDIRECTORY CMake command
It prevents placing any project specific build artifacts in
common 'build' directory.
To prevent placing any build artifacts in original source dir (breaking
shadow build idea)
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.

1 participant