-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
(#12358): NuRaft recipe #12360
(#12358): NuRaft recipe #12360
Changes from all commits
d3f3c52
3479815
760f049
e15648c
1b7b17a
4508f76
1a4fc0f
6e34aca
54c0077
4cd84fe
3cfbbe0
d99e916
243c5ad
a25ae02
4b4d568
936eea4
5fcba08
38a000e
4a892d2
b625742
a74bd31
5604fd1
a6d1846
4059bda
e756ad6
5f160cf
7ede518
121d34d
e6a2b6e
4d342a3
08b2020
872ea8a
ee6e727
f4798d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
sources: | ||
"2.0.0": | ||
url: https://github.com/ebay/nuraft/archive/refs/tags/v2.0.0.tar.gz | ||
sha256: f7f535f0e5c0417fb9a0ab87514a1b77647fc8e7ed967b85ca611df1cae14023 | ||
patches: | ||
"2.0.0": | ||
- patch_file: "patches/0001-cmake_patches.patch" | ||
patch_description: "Provide CMake correct dependency discovery logic." | ||
patch_type: "conan" | ||
sha256: 8147e2f3b950f944433220539376b1c8e0e93207902f48aa2d3bcbca814a674a |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,78 @@ | ||||||||
from os.path import join | ||||||||
from conan import ConanFile | ||||||||
from conan.errors import ConanInvalidConfiguration | ||||||||
from conan.tools.files import copy, get, apply_conandata_patches | ||||||||
from conan.tools.cmake import CMake, CMakeDeps, CMakeToolchain | ||||||||
from conan.tools.build import check_min_cppstd | ||||||||
|
||||||||
required_conan_version = ">=1.50.0" | ||||||||
|
||||||||
class NuRaftConan(ConanFile): | ||||||||
name = "nuraft" | ||||||||
homepage = "https://github.corp.ebay.com/sds/NuRaft" | ||||||||
description = """Cornerstone based RAFT library.""" | ||||||||
topics = ("raft") | ||||||||
url = "https://github.com/conan-io/conan-center-index" | ||||||||
license = "Apache-2.0" | ||||||||
|
||||||||
settings = "os", "compiler", "arch", "build_type" | ||||||||
|
||||||||
options = { | ||||||||
"asio": ["boost", "standalone"], | ||||||||
"shared": ['True', 'False'], | ||||||||
"fPIC": ['True', 'False'], | ||||||||
} | ||||||||
default_options = { | ||||||||
"asio": "boost", | ||||||||
"shared": False, | ||||||||
"fPIC": True, | ||||||||
} | ||||||||
|
||||||||
def export_sources(self): | ||||||||
for p in self.conan_data.get("patches", {}).get(self.version, []): | ||||||||
copy(self, p["patch_file"], self.recipe_folder, self.export_sources_folder) | ||||||||
|
||||||||
def requirements(self): | ||||||||
if self.options.asio == "boost": | ||||||||
self.requires("boost/1.79.0") | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We have Boost 1.80.0 available on Conan Center. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.1s") | ||||||||
|
||||||||
def validate(self): | ||||||||
if self.info.settings.os in ["Windows"]: | ||||||||
raise ConanInvalidConfiguration("{} Builds are unsupported".format(self.info.settings.os)) | ||||||||
if self.info.settings.os in ["Macos"] and self.options.shared: | ||||||||
raise ConanInvalidConfiguration("Building Shared Object for {} unsupported".format(self.info.settings.os)) | ||||||||
if self.info.settings.compiler.cppstd: | ||||||||
check_min_cppstd(self, 11) | ||||||||
szmyd marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
def configure(self): | ||||||||
if self.options.shared: | ||||||||
del self.options.fPIC | ||||||||
|
||||||||
def source(self): | ||||||||
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self.source_folder) | ||||||||
|
||||||||
def generate(self): | ||||||||
tc = CMakeToolchain(self) | ||||||||
tc.generate() | ||||||||
cmake = CMakeDeps(self) | ||||||||
cmake.generate() | ||||||||
|
||||||||
def build(self): | ||||||||
apply_conandata_patches(self) | ||||||||
cmake = CMake(self) | ||||||||
cmake.configure() | ||||||||
cmake.build() | ||||||||
|
||||||||
def package(self): | ||||||||
cmake = CMake(self) | ||||||||
cmake.install() | ||||||||
copy(self, "LICENSE", self.source_folder, join(self.package_folder, "licenses"), keep_path=False) | ||||||||
|
||||||||
def package_info(self): | ||||||||
self.cpp_info.libs = ["nuraft"] | ||||||||
self.cpp_info.system_libs = ["m"] | ||||||||
if self.settings.os in ["Linux", "FreeBSD"]: | ||||||||
self.cpp_info.system_libs.extend(["pthread"]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
diff --git a/CMakeLists.txt b/CMakeLists.txt | ||
index 980d466..9258209 100644 | ||
--- a/CMakeLists.txt | ||
+++ b/CMakeLists.txt | ||
@@ -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) | ||
+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(ASIO_INCLUDE_DIR ${BOOST_INCLUDE_PATH}) | ||
- set(LIBBOOST_SYSTEM "${BOOST_LIBRARY_PATH}/libboost_system.a") | ||
- | ||
+ set(ASIO_DEP boost::boost) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is? If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought there was |
||
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) | ||
- | ||
+ 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}) | ||
+ set(ASIO_DEP asio::asio) | ||
endif () | ||
|
||
|
||
# === Includes === | ||
include_directories(BEFORE ./) | ||
-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) | ||
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be managed by Conan There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
So it seems fPIC is needed regardless which conan is not managing correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. conan does not pass -fPIC, but the https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html#cmaketoolchain There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 () | ||
|
||
# === Disable SSL === | ||
@@ -250,6 +223,7 @@ set(RAFT_CORE | ||
${ROOT_SRC}/stat_mgr.cxx | ||
) | ||
add_library(RAFT_CORE_OBJ OBJECT ${RAFT_CORE}) | ||
+target_link_libraries(RAFT_CORE_OBJ ${ASIO_DEP} openssl::openssl) | ||
|
||
set(STATIC_LIB_SRC | ||
$<TARGET_OBJECTS:RAFT_CORE_OBJ>) | ||
@@ -257,54 +231,11 @@ set(STATIC_LIB_SRC | ||
# === Executables === | ||
set(LIBRARY_NAME "nuraft") | ||
|
||
-add_library(static_lib ${STATIC_LIB_SRC}) | ||
-set_target_properties(static_lib PROPERTIES OUTPUT_NAME ${LIBRARY_NAME} CLEAN_DIRECT_OUTPUT 1) | ||
- | ||
-add_library(shared_lib SHARED ${STATIC_LIB_SRC}) | ||
-set_target_properties(shared_lib PROPERTIES OUTPUT_NAME ${LIBRARY_NAME} CLEAN_DIRECT_OUTPUT 1) | ||
-if (APPLE) | ||
- target_link_libraries(shared_lib ${LIBRARIES}) | ||
-endif () | ||
- | ||
-if (WIN32) | ||
- set(LIBRARY_OUTPUT_NAME "${LIBRARY_NAME}.lib") | ||
-else () | ||
- set(LIBRARY_OUTPUT_NAME "lib${LIBRARY_NAME}.a") | ||
-endif () | ||
-message(STATUS "Output library file name: ${LIBRARY_OUTPUT_NAME}") | ||
- | ||
-# === Examples === | ||
-add_subdirectory("${PROJECT_SOURCE_DIR}/examples") | ||
+add_library(nuraft ${STATIC_LIB_SRC}) | ||
+target_link_libraries(nuraft ${ASIO_DEP} openssl::openssl) | ||
|
||
|
||
# === Tests === | ||
-add_subdirectory("${PROJECT_SOURCE_DIR}/tests") | ||
- | ||
- | ||
-if (CODE_COVERAGE GREATER 0) | ||
- set(CODE_COVERAGE_DEPS | ||
- raft_server_test | ||
- failure_test | ||
- asio_service_test | ||
- buffer_test | ||
- serialization_test | ||
- timer_test | ||
- strfmt_test | ||
- stat_mgr_test | ||
- ) | ||
- | ||
- # lcov | ||
- SETUP_TARGET_FOR_COVERAGE( | ||
- NAME raft_cov | ||
- EXECUTABLE ./runtests.sh | ||
- DEPENDENCIES ${CODE_COVERAGE_DEPS} | ||
- ) | ||
-endif() | ||
- | ||
- | ||
-# === Install Targets === | ||
-install(TARGETS static_lib ARCHIVE DESTINATION lib) | ||
-install(TARGETS shared_lib LIBRARY DESTINATION lib) | ||
+install(TARGETS nuraft ARCHIVE DESTINATION lib) | ||
+install(TARGETS nuraft LIBRARY DESTINATION lib) | ||
install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/libnuraft DESTINATION include) | ||
- | ||
- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
cmake_minimum_required(VERSION 3.8) | ||
project(test_package LANGUAGES CXX) | ||
|
||
include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake) | ||
conan_basic_setup(TARGETS) | ||
|
||
find_package(nuraft CONFIG REQUIRED) | ||
|
||
add_executable(${PROJECT_NAME}) | ||
target_sources(${PROJECT_NAME} PRIVATE test_package.cpp) | ||
target_link_libraries(${PROJECT_NAME} nuraft::nuraft) | ||
target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_11) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import os | ||
from conan import ConanFile | ||
from conans import CMake | ||
from conan.tools.build import cross_building | ||
|
||
class TestPackageConan(ConanFile): | ||
settings = "os", "compiler", "build_type", "arch" | ||
generators = "cmake", "cmake_find_package_multi" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be the test for the new generators and this can be moved to |
||
|
||
def build(self): | ||
cmake = CMake(self) | ||
cmake.configure() | ||
cmake.build() | ||
|
||
def test(self): | ||
if not cross_building(self): | ||
bin_path = os.path.join("bin", "test_package") | ||
self.run(bin_path, run_environment=True) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
#include <iostream> | ||
#include <libnuraft/nuraft.hxx> | ||
|
||
struct ex_logger : ::nuraft::logger { | ||
void set_level(int) override {} | ||
void debug(const std::string&) override {} | ||
void info(const std::string&) override {} | ||
void warn(const std::string&) override {} | ||
void err(const std::string& log_line) override { | ||
std::cout << log_line << std::endl; | ||
} | ||
|
||
void put_details(int, const char*, const char*, size_t, | ||
const std::string&) override {} | ||
}; | ||
|
||
class echo_state_machine : public nuraft::state_machine { | ||
nuraft::ulong last_commit_idx_; | ||
|
||
public: | ||
echo_state_machine() : last_commit_idx_(0) {} | ||
|
||
nuraft::ptr<nuraft::buffer> commit(const nuraft::ulong log_idx, | ||
nuraft::buffer&) override { | ||
last_commit_idx_ = log_idx; | ||
return nullptr; | ||
} | ||
nuraft::ptr<nuraft::buffer> pre_commit(const nuraft::ulong, | ||
nuraft::buffer&) override { | ||
return nullptr; | ||
} | ||
void rollback(const nuraft::ulong, nuraft::buffer&) override {} | ||
void save_snapshot_data(nuraft::snapshot&, const nuraft::ulong, | ||
nuraft::buffer&) override {} | ||
bool apply_snapshot(nuraft::snapshot&) override { return true; } | ||
int read_snapshot_data(nuraft::snapshot&, const nuraft::ulong, | ||
nuraft::buffer&) override { | ||
return 0; | ||
} | ||
nuraft::ptr<nuraft::snapshot> last_snapshot() override { return nullptr; } | ||
void create_snapshot(nuraft::snapshot&, | ||
nuraft::async_result<bool>::handler_type&) override {} | ||
nuraft::ulong last_commit_index() override { return last_commit_idx_; } | ||
}; | ||
|
||
int main(int argc, char** argv) { | ||
// State machine. | ||
nuraft::ptr<nuraft::state_machine> smachine( | ||
nuraft::cs_new<echo_state_machine>()); | ||
|
||
// Parameters. | ||
nuraft::raft_params params; | ||
|
||
// ASIO service. | ||
nuraft::ptr<nuraft::logger> l = nuraft::cs_new<ex_logger>(); | ||
nuraft::ptr<nuraft::asio_service> asio_svc_ = | ||
nuraft::cs_new<nuraft::asio_service>(); | ||
nuraft::ptr<nuraft::rpc_listener> listener( | ||
asio_svc_->create_rpc_listener((ushort)(9001), l)); | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
versions: | ||
"2.0.0": | ||
folder: all |
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.