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

Provide vcpkg support #662

Open
geiseri opened this issue Jul 6, 2020 · 3 comments
Open

Provide vcpkg support #662

geiseri opened this issue Jul 6, 2020 · 3 comments
Labels
core Themis Core written in C, its packages enhancement infrastructure Automated building and packaging O-Windows 🖥️ Operating system: Windows W-ThemisPP ⚔️ Wrapper: ThemisPP, C++ API

Comments

@geiseri
Copy link

geiseri commented Jul 6, 2020

Currently, the custom makefile doesn't work well with things like conan or vcpkg. If the CMakeLists.txt was able to properly build the library it would be a great help to integrate with that tooling.

@ilammy ilammy added core Themis Core written in C, its packages enhancement infrastructure Automated building and packaging O-Windows 🖥️ Operating system: Windows W-ThemisPP ⚔️ Wrapper: ThemisPP, C++ API labels Jul 6, 2020
@vixentael
Copy link
Contributor

Hi @geiseri! Thank you a lot for the suggestion!

Would you mind to share some details on how you would see expected behaviour? Currently we don't have users that would like to use Themis as conan / vcpkg, and it makes hard for us to understand the best practice and expected behaviour. Especially, since Themis depends on OpenSSL/BoringSSL, it opens questions like "dynamic linking vs static linking". Maybe you have some preferences or best practice scenarios that could help us?

Thank you!

@geiseri
Copy link
Author

geiseri commented Jul 8, 2020

Both vcpkg and conan leverage CMake quite deeply and are more or less like NPM for C/C++. In most cases, I use it on Windows and Linux for static linking internally projects in a single bundle. I like vcpkg just because if you have even the most basic CMakeLists.txt it is trivial to integrate. I was looking at the existing Makefile and the CMakeLists.txt and it looks like the only real things missing from the CMakeLists.txt would be the configuration flags. Currently, both have the ability to resolve dependencies, and both have support for OpenSSL/BoringSSL. The big issue would be that it would only really be of use for C/C++ shared libraries, so it would be a bit redundant from the current makefile infrastructure. I can take a whack at it and PR a change to the CMakeLists.txt and maybe we can see what makes sense to integrate.

@geiseri
Copy link
Author

geiseri commented Jul 8, 2020

This is a very naive approach, but it compiles on debian buster, and the resulting tests pass:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 80de2dba..f909d294 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -2,6 +2,12 @@ project(themis)
 cmake_minimum_required(VERSION 3.8)
 include_directories(src)
 
+find_package(OpenSSL REQUIRED)
+
+include_directories(
+    ${OPENSSL_INCLUDE_DIR}
+    tests
+)
 
 file(GLOB SOTER_SOURCE_FILES src/soter/*.c src/soter/openssl/*.c src/soter/ed25519/*)
 add_library(soter ${SOTER_SOURCE_FILES})
@@ -22,11 +28,11 @@ file(GLOB SOTER_TEST_SOURCE tests/soter/*.c tests/common/*.c )
 file(GLOB THEMIS_TEST_SOURCE tests/themis/*.c tests/common/*.c)
 
 add_executable(soter_test ${SOTER_TEST_SOURCE} )
-target_link_libraries(soter_test soter crypto)
+target_link_libraries(soter_test soter OpenSSL::Crypto)
 
 add_executable(themis_test ${THEMIS_TEST_SOURCE} ${SOTER_SOURCE_FILES})
 target_include_directories(themis_test PRIVATE tests)
-target_link_libraries(themis_test soter crypto themis)
+target_link_libraries(themis_test soter OpenSSL::Crypto themis)
 
 file(GLOB THEMISPP_SOURCE src/wrappers/themis/themispp/*.hpp)
 add_library(themispp ${THEMISPP_SOURCE})
@@ -36,5 +42,5 @@ set_target_properties(themispp PROPERTIES LINKER_LANGUAGE CXX)
 file(GLOB THEMISPP_TEST_SOURCE tests/themispp/*.hpp tests/themispp/*.cpp tests/common/*)
 add_executable(themispp_test ${THEMISPP_TEST_SOURCE})
 target_include_directories(themispp_test PUBLIC tests src/wrappers/themis)
-target_link_libraries(themispp_test themispp themis soter crypto)
+target_link_libraries(themispp_test themispp themis soter OpenSSL::Crypto)
 set_target_properties(themispp_test PROPERTIES LINKER_LANGUAGE CXX)

I think with some better understanding of the flags in the Makefile I could improve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Themis Core written in C, its packages enhancement infrastructure Automated building and packaging O-Windows 🖥️ Operating system: Windows W-ThemisPP ⚔️ Wrapper: ThemisPP, C++ API
Projects
None yet
Development

No branches or pull requests

3 participants