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

(#12358): NuRaft recipe #12360

Merged
merged 34 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 29 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
14 changes: 14 additions & 0 deletions recipes/nuraft/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
sources:
"cci.20220801":
Copy link
Contributor

Choose a reason for hiding this comment

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

This project has releases https://github.com/eBay/NuRaft/releases is there a reason to use master?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There has not been a release for over a year even though there has been a lot of fixes that are worthwhile to pick up. Author also doesn't use semantic versioning anyways between release numbers, so would likely mis-match with what we call it in the recipe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind asking in the upstream repository for a new release? It is always better if we can use tagged released.

Copy link
Contributor Author

@szmyd szmyd Sep 9, 2022

Choose a reason for hiding this comment

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

I don't mind, but I know historically they have been using X.Y.Z style tags without adhering to semantic versioning rules. I'll check what they want to do (cut a new release with non-semantic tag style, or to start adhering to the guidelines). 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

We do not require semver, we just prefer official releases (they can do anything they'd like for that)

url: "https://github.com/eBay/NuRaft/archive/50031e1b230569ccaa46008fc09d401e3e5c5985.tar.gz"
sha256: 225c30d04d9e067010b4a9b3340961ae89c30bee00e9762fcdc651401f890e41
patches:
"cci.20220801":
- patch_file: "patches/0001-cmake_patches.patch"
patch_description: "Provide CMake correct dependency discovery logic."
patch_type: "conan"
sha256: "2f847610184e680fa2d40e496c2094fc10aa55c4b652e56240c765cfc3a51a75"
- 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"
sha256: "704a08e63422eed7fcb1f9f794f57007862c9540ea7319ec14873813a33eac6e"
78 changes: 78 additions & 0 deletions recipes/nuraft/all/conanfile.py
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
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"

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, []):
self.copy(p["patch_file"])
szmyd marked this conversation as resolved.
Show resolved Hide resolved

def requirements(self):
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):
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

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
def layout(self):
cmake_layout(self, src_folder="src")

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)
szmyd marked this conversation as resolved.
Show resolved Hide resolved

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"])
119 changes: 119 additions & 0 deletions recipes/nuraft/all/patches/0001-cmake_patches.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 87685ff..764b71d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -26,41 +26,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)
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
- $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)
@@ -242,6 +224,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>)
@@ -249,54 +232,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)
-
-
49 changes: 49 additions & 0 deletions recipes/nuraft/all/patches/0002-boost-asio.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
diff --git a/src/asio_service.cxx b/src/asio_service.cxx
index ceb4f70..4623667 100644
--- a/src/asio_service.cxx
+++ b/src/asio_service.cxx
@@ -38,7 +38,11 @@ limitations under the License.
#include "strfmt.hxx"
#include "tracer.hxx"

-#include "asio.hpp"
+#ifdef USE_BOOST_ASIO
+#include <boost/asio.hpp>
+#else
+#include <asio.hpp>
+#endif

#include <atomic>
#include <ctime>
@@ -62,9 +63,13 @@ limitations under the License.
using ssl_socket = mock_ssl_socket;
using ssl_context = mock_ssl_context;
#else
- #include "asio/ssl.hpp"
+#ifdef USE_BOOST_ASIO
+ #include <boost/asio/ssl.hpp>
+#else
+ #include <asio/ssl.hpp>
+#endif
using ssl_socket = asio::ssl::stream<asio::ip::tcp::socket&>;
using ssl_context = asio::ssl::context;
#endif

// Note: both req & resp header structures have been modified by Jung-Sang Ahn.
diff --git a/src/handle_commit.cxx b/src/handle_commit.cxx
index dc47186..b032ef4 100644
--- a/src/handle_commit.cxx
+++ b/src/handle_commit.cxx
@@ -552,9 +552,9 @@ bool raft_server::snapshot_and_compact(ulong committed_idx, bool forced_creation
}
return false;

- } catch (...) {
- p_er( "failed to compact logs at index %llu due to errors",
- committed_idx );
+ } catch (std::exception &e) {
+ p_er( "failed to compact logs at index %llu due to errors %s",
+ committed_idx, e.what());
if (snapshot_in_action) {
bool val = true;
snp_in_progress_.compare_exchange_strong(val, false);
12 changes: 12 additions & 0 deletions recipes/nuraft/all/test_package/CMakeLists.txt
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)
18 changes: 18 additions & 0 deletions recipes/nuraft/all/test_package/conanfile.py
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"
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 the test for the new generators and this can be moved to test_v1_package/


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)
61 changes: 61 additions & 0 deletions recipes/nuraft/all/test_package/test_package.cpp
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 {
ulong last_commit_idx_;

public:
echo_state_machine() : last_commit_idx_(0) {}

nuraft::ptr<nuraft::buffer> commit(const ulong log_idx,
nuraft::buffer&) override {
last_commit_idx_ = log_idx;
return nullptr;
}
nuraft::ptr<nuraft::buffer> pre_commit(const ulong,
nuraft::buffer&) override {
return nullptr;
}
void rollback(const ulong, nuraft::buffer&) override {}
void save_snapshot_data(nuraft::snapshot&, const ulong,
nuraft::buffer&) override {}
bool apply_snapshot(nuraft::snapshot&) override { return true; }
int read_snapshot_data(nuraft::snapshot&, const 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 {}
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;
}
3 changes: 3 additions & 0 deletions recipes/nuraft/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
"cci.20220801":
folder: all