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.

Sign up for GitHub

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

Jump to bottom

(#12358): NuRaft recipe #12360

Merged
merged 34 commits into from
Nov 25, 2022
Merged

(#12358): NuRaft recipe #12360

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d3f3c52
(#12358): NuRaft recipe
szmyd Aug 16, 2022
3479815
Remove old test runner.
szmyd Aug 18, 2022
760f049
Adjust tools imports
szmyd Aug 18, 2022
e15648c
New tools defined as member methods.
szmyd Aug 18, 2022
1b7b17a
Adjust variable name.
szmyd Aug 18, 2022
4508f76
Remove shared library option from Macos.
szmyd Aug 18, 2022
1a4fc0f
Do not remove options later referenced.
szmyd Aug 18, 2022
6e34aca
Fix import for v2.
szmyd Aug 18, 2022
54c0077
Remove running unit tests.
szmyd Aug 19, 2022
4cd84fe
Remove fPIC option on Windows.
szmyd Aug 19, 2022
3cfbbe0
Cleanup recipe with 2.x migrations.
szmyd Aug 19, 2022
d99e916
Disable Windows builds entirely.
szmyd Aug 19, 2022
243c5ad
Typo
szmyd Aug 19, 2022
a25ae02
Add libm to systemlibs linkages.
szmyd Aug 20, 2022
4b4d568
Provide patch descriptions.
szmyd Aug 26, 2022
936eea4
Simplify test_package.
szmyd Aug 26, 2022
5fcba08
Use CONFIG mode for test_package.
szmyd Aug 26, 2022
38a000e
Update recipes/nuraft/all/conanfile.py
szmyd Aug 30, 2022
4a892d2
Remove example code from library.
szmyd Aug 30, 2022
b625742
Adjust install path for headers.
szmyd Aug 30, 2022
a74bd31
Use built-on install target.
szmyd Aug 30, 2022
5604fd1
Cleanup test_package some more.
szmyd Aug 31, 2022
a6d1846
Merge branch 'upstream_master' into nuraft
szmyd Sep 12, 2022
4059bda
Allow both Boost::ASIO and standalone ASIO libraries.
szmyd Sep 12, 2022
e756ad6
Remove unused toolchain file.
szmyd Sep 12, 2022
5f160cf
Remove conans.tools import
szmyd Sep 12, 2022
7ede518
Remove unused import
szmyd Sep 12, 2022
121d34d
Cleanup patchwork
szmyd Sep 12, 2022
e6a2b6e
Remove unused recipe property.
szmyd Sep 12, 2022
4d342a3
Fix missing namespace.
szmyd Sep 14, 2022
08b2020
Add back original fPIC option from upstream CMake.
szmyd Sep 14, 2022
872ea8a
Review comments addressed.
szmyd Sep 15, 2022
ee6e727
Update to v2.0.0
szmyd Nov 2, 2022
f4798d4
Remove old reference.
szmyd Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions recipes/nuraft/all/CMakeLists.txt
View file
Open in desktop

This file was deleted.

6 changes: 2 additions & 4 deletions recipes/nuraft/all/conandata.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ patches:
- patch_file: "patches/0001-cmake_patches.patch"
patch_description: "Provide CMake correct dependency discovery logic."
patch_type: "conan"
base_path: "source_subfolder"
sha256: "2f847610184e680fa2d40e496c2094fc10aa55c4b652e56240c765cfc3a51a75"
sha256: "e52dcb59238276e21388463e45d41a34d7d6bbda29f4582d995640018a6ba2e1"
- patch_file: "patches/0002-boost-asio.patch"
patch_description: "Use Boost::Asio rather than Asio solo project."
szmyd marked this conversation as resolved.
Show resolved Hide resolved
patch_type: "portability"
base_path: "source_subfolder"
sha256: "704a08e63422eed7fcb1f9f794f57007862c9540ea7319ec14873813a33eac6e"
sha256: "1614ca0e647f313ca83d443f91c86e8e2647c3a6480998971d7c2e9c45434a38"
50 changes: 19 additions & 31 deletions recipes/nuraft/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from conan import ConanFile
from conan.errors import ConanInvalidConfiguration
from conan.tools.files import copy, get, apply_conandata_patches
from conans import CMake
from conans.tools import check_min_cppstd
from conan.tools.cmake import CMake, CMakeDeps, CMakeToolchain
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from conan.tools.cmake import CMake, CMakeDeps, CMakeToolchain
from conan.tools.cmake import CMake, CMakeDeps, CMakeToolchain, cmake_layout

from conan.tools.build import check_min_cppstd

required_conan_version = ">=1.50.0"

Expand All @@ -18,27 +18,25 @@ class NuRaftConan(ConanFile):
settings = "os", "compiler", "arch", "build_type"

options = {
"asio": ["boost", "standalone"],
"shared": ['True', 'False'],
"fPIC": ['True', 'False'],
}
default_options = {
"asio": "boost",
"shared": False,
"fPIC": True,
}

generators = "cmake", "cmake_find_package"

@property
def _source_subfolder(self):
return "source_subfolder"

def export_sources(self):
self.copy("CMakeLists.txt")
for p in self.conan_data.get("patches", {}).get(self.version, []):
self.copy(p["patch_file"])
szmyd marked this conversation as resolved.
Show resolved Hide resolved

def requirements(self):
self.requires("boost/1.79.0")
if self.options.asio == "boost":
self.requires("boost/1.79.0")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.requires("boost/1.79.0")
self.requires("boost/1.80.0")

We have Boost 1.80.0 available on Conan Center.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it not better to target a lower MINOR since it won’t require recompile should someone want 1.80.0 as opposed to the reverse which would?

Copy link
Member

Choose a reason for hiding this comment

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

soon or later, all recipes will use 1.80.0, then your recipe will need to be updated too. Doing it now, it are just doing what should be done later.

Copy link
Contributor

Choose a reason for hiding this comment

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

In ConanCenter we always try to stay on latest to avoid conflicts... two recipes using boost should work together (hopefully)

else:
self.requires("asio/1.22.1")
self.requires("openssl/1.1.1q")

def validate(self):
Expand All @@ -54,34 +52,24 @@ def configure(self):
del self.options.fPIC

def source(self):
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self._source_subfolder)
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self.source_folder)

def cmakeGet(self):
cmake = CMake(self)
cmake.configure()
return cmake
def generate(self):
tc = CMakeToolchain(self)
tc.generate()
cmake = CMakeDeps(self)
cmake.generate()

def build(self):
apply_conandata_patches(self)
cmake = self.cmakeGet()
cmake = CMake(self)
cmake.configure()
cmake.build()

def package(self):
lib_dir = join(self.package_folder, "lib")
copy(self, "LICENSE", join(self.source_folder, self._source_subfolder), join(self.package_folder, "licenses/"), keep_path=False)
copy(self, "*.lib", self.build_folder, lib_dir, keep_path=False)
copy(self, "*.a", self.build_folder, lib_dir, keep_path=False)
copy(self, "*.so*", self.build_folder, lib_dir, keep_path=False)
copy(self, "*.dylib*", self.build_folder, lib_dir, keep_path=False)
copy(self, "*.dll*", self.build_folder, join(self.package_folder, "bin"), keep_path=False)

hdr_src = join(self.source_folder, join(self._source_subfolder, "include"))
hdr_dir = join(self.package_folder, join("include", "libnuraft"))

copy(self, "*.hxx", hdr_src, join(self.package_folder, "include"), keep_path=True)
copy(self, "*in_memory_log_store.hxx", self.source_folder, hdr_dir, keep_path=False)
copy(self, "*callback.h", self.source_folder, hdr_dir, keep_path=False)
copy(self, "*event_awaiter.h", self.source_folder, hdr_dir, keep_path=False)
cmake = CMake(self)
cmake.install()
copy(self, "LICENSE", self.source_folder, join(self.package_folder, "licenses/"), keep_path=False)
szmyd marked this conversation as resolved.
Show resolved Hide resolved

def package_info(self):
self.cpp_info.libs = ["nuraft"]
Expand Down
219 changes: 52 additions & 167 deletions recipes/nuraft/all/patches/0001-cmake_patches.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,35 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 87685ff..1fc9199 100644
index 87685ff..efbc7fb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -26,41 +44,41 @@ endif()
@@ -1,5 +1,6 @@
cmake_minimum_required(VERSION 3.5)
project(NuRaft VERSION 1.0.0 LANGUAGES CXX)
+set (CMAKE_CXX_STANDARD 11)

# === Build type (default: RelWithDebInfo, O2) ===========
if (NOT CMAKE_BUILD_TYPE)
@@ -26,41 +27,23 @@ endif()


# === Find ASIO ===
-if (BOOST_INCLUDE_PATH AND BOOST_LIBRARY_PATH)
- # If Boost path (both include and library) is given,
- # use Boost's ASIO.
+find_package(OpenSSL CONFIG REQUIRED)
+find_package(Boost CONFIG)
+if (Boost_FOUND)
# If Boost path (both include and library) is given,
# use Boost's ASIO.
- message(STATUS "Boost include path: " ${BOOST_INCLUDE_PATH})
- message(STATUS "Boost library path: " ${BOOST_LIBRARY_PATH})
-
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUSE_BOOST_ASIO")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUSE_BOOST_ASIO")
-
- set(ASIO_INCLUDE_DIR ${BOOST_INCLUDE_PATH})
- set(LIBBOOST_SYSTEM "${BOOST_LIBRARY_PATH}/libboost_system.a")
-
-else ()
- # If not, ASIO standalone mode.
+ set(ASIO_DEP boost::boost)
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this was replaced below 🤔

Should we not use the asio target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is? If Boost_FOUND tests false I find the asio package and set ASIO_DEP to asio::asio and use ${ASIO_DEP} as the target_link_library. Am I missing the point?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought there was Boost::asio but it's not official now that I double check https://cmake.org/cmake/help/latest/module/FindBoost.html#imported-targets

else ()
# If not, ASIO standalone mode.
- FIND_PATH(ASIO_INCLUDE_DIR
- NAME asio.hpp
- HINTS ${PROJECT_SOURCE_DIR}/asio/asio/include
Expand All @@ -27,72 +38,55 @@ index 87685ff..1fc9199 100644
- /usr/local/include
- /usr/include)
-
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DASIO_STANDALONE")
+ find_package(Asio CONFIG REQUIRED)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DASIO_STANDALONE")
-
-endif ()
-
-if (NOT ASIO_INCLUDE_DIR)
- message(FATAL_ERROR "Can't find ASIO header files")
-else ()
- message(STATUS "ASIO include path: " ${ASIO_INCLUDE_DIR})
-endif ()
+#if (BOOST_INCLUDE_PATH AND BOOST_LIBRARY_PATH)
+# # If Boost path (both include and library) is given,
+# # use Boost's ASIO.
+# message(STATUS "Boost include path: " ${BOOST_INCLUDE_PATH})
+# message(STATUS "Boost library path: " ${BOOST_LIBRARY_PATH})
+#
+# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUSE_BOOST_ASIO")
+#
+# set(ASIO_INCLUDE_DIR ${BOOST_INCLUDE_PATH})
+# set(LIBBOOST_SYSTEM "${BOOST_LIBRARY_PATH}/libboost_system.a")
+#
+#else ()
+# # If not, ASIO standalone mode.
+# FIND_PATH(ASIO_INCLUDE_DIR
+# NAME asio.hpp
+# HINTS ${PROJECT_SOURCE_DIR}/asio/asio/include
+# $ENV{HOME}/local/include
+# /opt/local/include
+# /usr/local/include
+# /usr/include)
+#
+# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DASIO_STANDALONE")
+#
+#endif ()
+
+#if (NOT ASIO_INCLUDE_DIR)
+# message(FATAL_ERROR "Can't find ASIO header files")
+#else ()
+# message(STATUS "ASIO include path: " ${ASIO_INCLUDE_DIR})
+#endif ()
+ set(ASIO_DEP asio::asio)
endif ()


# === Includes ===
include_directories(BEFORE ./)
-include_directories(BEFORE ${ASIO_INCLUDE_DIR})
+#include_directories(BEFORE ${ASIO_INCLUDE_DIR})
include_directories(BEFORE ${PROJECT_SOURCE_DIR}/include)
include_directories(BEFORE ${PROJECT_SOURCE_DIR}/include/libnuraft)
include_directories(BEFORE ${PROJECT_SOURCE_DIR}/examples)
@@ -213,6 +231,7 @@ if (NOT (DISABLE_SSL GREATER 0))
)
@@ -81,20 +64,10 @@ endif()
if (NOT WIN32)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-pessimizing-move")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be managed by Conan

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I let it, then the shared library option does not include PIC and I end up with:

/usr/bin/ld: CMakeFiles/RAFT_CORE_OBJ.dir/src/handle_commit.cxx.o: relocation R_X86_64_TPOFF32 against `_ZZN6nuraft11raft_server11reconfigureERKSt10shared_ptrINS_14cluster_configEEE8temp_buf' can not be used when making a shared object; recompile with -fPIC

So it seems fPIC is needed regardless which conan is not managing correctly?

Copy link
Member

Choose a reason for hiding this comment

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

conan does not pass -fPIC, but the CMAKE_POSITION_INDEPENDENT_CODE definition

https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html#cmaketoolchain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, and it seems to not be present with the shared option enabled (I’ll double check), so I had to leave the original author’s -fPIC flag in the cmake rules.

Copy link
Contributor

Choose a reason for hiding this comment

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

😕 by default CMake does the right thing https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html#prop_tgt:POSITION_INDEPENDENT_CODE

So there must be some hackery in the build script tripping up the sensible default 🙈

- if (APPLE)
- include_directories(BEFORE /usr/local/opt/openssl/include)
- link_directories(/usr/local/opt/openssl/lib)
- else()
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
- endif ()
-
else ()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd5045 /wd4571 /wd4774 /wd4820 /wd5039 /wd4626 /wd4625 /wd5026 /wd5027 /wd4623 /wd4996 /wd4530 /wd4267 /wd4244 /W3")
message(STATUS "---- WIN32 ----")
- set(DISABLE_SSL 1)
endif ()

+include_directories(BEFORE "${PROJECT_SOURCE_DIR}/include/libnuraft")
# === Source files ===
set(RAFT_CORE
${ROOT_SRC}/asio_service.cxx
@@ -240,6 +259,8 @@ set(RAFT_CORE
${ROOT_SRC}/snapshot_sync_req.cxx
${ROOT_SRC}/srv_config.cxx
# === Disable SSL ===
@@ -242,6 +214,7 @@ set(RAFT_CORE
${ROOT_SRC}/stat_mgr.cxx
+ examples/in_memory_log_store.cxx
)
add_library(RAFT_CORE_OBJ OBJECT ${RAFT_CORE})
+target_link_libraries(RAFT_CORE_OBJ Boost::Boost OpenSSL::OpenSSL)
+target_link_libraries(RAFT_CORE_OBJ ${ASIO_DEP} openssl::openssl)

@@ -249,54 +269,26 @@ set(STATIC_LIB_SRC
set(STATIC_LIB_SRC
$<TARGET_OBJECTS:RAFT_CORE_OBJ>)
@@ -249,54 +222,11 @@ set(STATIC_LIB_SRC
# === Executables ===
set(LIBRARY_NAME "nuraft")

Expand All @@ -114,30 +108,12 @@ index 87685ff..1fc9199 100644
-
-# === Examples ===
-add_subdirectory("${PROJECT_SOURCE_DIR}/examples")
-
+add_library(nuraft ${STATIC_LIB_SRC})
+target_link_libraries(nuraft Boost::Boost OpenSSL::OpenSSL)
+
+include_directories(BEFORE ${EXAMPLES_SRC})
+#set(TEST_SRCS
+# "tests/src/test_async_result.cxx"
+# "tests/src/test_buffer.cxx"
+# "tests/src/test_everything_together.cxx"
+# "tests/src/test_impls.cxx"
+# "tests/src/test_logger.cxx"
+# "tests/src/test_log_store.cxx"
+# "tests/src/test_ptr.cxx"
+# "tests/src/test_runner.cxx"
+# "tests/src/test_scheduler.cxx"
+# "tests/src/test_serialization.cxx"
+# "tests/src/test_strfmt.cxx"
+# )
+#add_executable(test_runner ${TEST_SRCS})
+#target_link_libraries(test_runner nuraft)
+#add_test(NAME TestRunner COMMAND test_runner)
+target_link_libraries(nuraft ${ASIO_DEP} openssl::openssl)


# === Tests ===
add_subdirectory("${PROJECT_SOURCE_DIR}/tests")
-add_subdirectory("${PROJECT_SOURCE_DIR}/tests")
-
-
-if (CODE_COVERAGE GREATER 0)
Expand All @@ -164,99 +140,8 @@ index 87685ff..1fc9199 100644
-# === Install Targets ===
-install(TARGETS static_lib ARCHIVE DESTINATION lib)
-install(TARGETS shared_lib LIBRARY DESTINATION lib)
-install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/libnuraft DESTINATION include)
-
-
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 5ceccce..f6dc2f0 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -1,83 +1,9 @@
# === Basic Raft server functionality test ===
+include_directories(BEFORE "${PROJECT_SOURCE_DIR}/tests")
+include_directories(BEFORE "${PROJECT_SOURCE_DIR}/src")
add_executable(raft_server_test
unit/raft_server_test.cxx
unit/fake_network.cxx
- ${EXAMPLES_SRC}/logger.cc
- ${EXAMPLES_SRC}/in_memory_log_store.cxx)
-add_dependencies(raft_server_test
- static_lib)
-target_link_libraries(raft_server_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
-
-# === Failure recovery & conflict resolution test ===
-add_executable(failure_test
- unit/failure_test.cxx
- unit/fake_network.cxx
- ${EXAMPLES_SRC}/logger.cc
- ${EXAMPLES_SRC}/in_memory_log_store.cxx)
-add_dependencies(failure_test
- static_lib)
-target_link_libraries(failure_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
-
-# === ASIO network stuff test ===
-add_executable(asio_service_test
- unit/asio_service_test.cxx
- ${EXAMPLES_SRC}/logger.cc
- ${EXAMPLES_SRC}/in_memory_log_store.cxx)
-add_dependencies(asio_service_test
- static_lib
- build_ssl_key)
-target_link_libraries(asio_service_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME}
- ${LIBRARIES})
-
-# === Benchmark ===
-add_executable(raft_bench
- bench/raft_bench.cxx
- ${EXAMPLES_SRC}/logger.cc
- ${EXAMPLES_SRC}/in_memory_log_store.cxx)
-add_dependencies(raft_bench
- static_lib)
-target_link_libraries(raft_bench
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME}
- ${LIBRARIES})
-
-
-# === Other modules ===
-add_executable(buffer_test
- unit/buffer_test.cxx)
-add_dependencies(buffer_test
- static_lib)
-target_link_libraries(buffer_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
-
-add_executable(serialization_test
- unit/serialization_test.cxx)
-add_dependencies(serialization_test
- static_lib)
-target_link_libraries(serialization_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
-
-add_executable(timer_test
- unit/timer_test.cxx)
-add_dependencies(timer_test
- static_lib)
-target_link_libraries(timer_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME}
- ${LIBRARIES})
-
-add_executable(strfmt_test
- unit/strfmt_test.cxx)
-add_dependencies(strfmt_test
- static_lib)
-target_link_libraries(strfmt_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
+install(TARGETS nuraft ARCHIVE DESTINATION lib)
+install(TARGETS nuraft LIBRARY DESTINATION lib)
install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/libnuraft DESTINATION include)
-
-add_executable(stat_mgr_test
- unit/stat_mgr_test.cxx)
-add_dependencies(stat_mgr_test
- static_lib)
-target_link_libraries(stat_mgr_test
- ${BUILD_DIR}/${LIBRARY_OUTPUT_NAME})
-
+ ${EXAMPLES_SRC}/logger.cc)
+add_test(NAME RaftServerTest COMMAND raft_server_test)
+target_link_libraries(raft_server_test nuraft)
Loading