-
Notifications
You must be signed in to change notification settings - Fork 18
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
RCPP-41 Add Conan support #174
Conversation
7735490
to
2e4a070
Compare
39a874f
to
3fc91f2
Compare
conanfile.py
Outdated
git = Git(self) | ||
git.clone(url="https://github.com/realm/realm-cpp", target=".") | ||
git.folder = "." | ||
git.checkout(commit="316c31841b56fab7fe1f28f0aa0138e00f10a8cb") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This temporarily points to the changelog commit in this PR. This should point to a v2.0.0 tag in the future
f729133
to
4d3d3f5
Compare
0ab633c
to
507162c
Compare
CMakeLists.txt
Outdated
@@ -127,7 +127,12 @@ else() | |||
endif() | |||
|
|||
if(MSVC AND NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) | |||
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") | |||
if(NOT DEFINED BUILD_SHARED_LIBS OR NOT BUILD_SHARED_LIBS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(NOT DEFINED BUILD_SHARED_LIBS OR NOT BUILD_SHARED_LIBS) | |
if(NOT BUILD_SHARED_LIBS) |
If BUILD_SHARED_LIBS
is undefined it will evaluate to false anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case, Conan already seems to define CMAKE_MSVC_RUNTIME_LIBRARY
, why is the extra condition here necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for building with plain cmake or from vcpkg, conan handles this for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the intention is to default to the shared runtime library instead of the static one? The shared one is default behavior, so I think you could simplify this to if (not defined CMAKE_MSVC_RUNTIME_LIBRARY) set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL") endif()
, but we can also do this in a second pass just to clarify our behavior around CMAKE_MSVC_RUNTIME_LIBRARY
.
This PR adds the relevant conanfiles and is a prerequisite before being added to the Conan Centre Index.
You can install
cpprealm
locally withconan install .
from the project source dir.