Skip to content

Commit

Permalink
Fix Open/R oss build with the getdeps tool
Browse files Browse the repository at this point in the history
Summary:
As titled. Open/R's OSS build has been failing for quite a while. By tracing back, I assume the first failure was introduced by this coro migration diff: D46549163

This diff fixes this.

NOTE: unfortunately there are not enough folly::coro support with open source build and we hit co_* method can't be found in thrift service when linking the lib. Temporarily disabled this `-fcoroutines` option.

The steps to build openr OSS with getdeps is
```
sudo ./opensource/fbcode_builder/getdeps.py install-system-deps --recursive openr
./opensource/fbcode_builder/getdeps.py build openr
```
 {F1937790594}
NOTE: oss test needs to be handled separately

Reviewed By: shih-hao-tseng

Differential Revision: D64451955

fbshipit-source-id: 971ba0ffd119b15853a6ca49e66f30eb283b38d9
  • Loading branch information
Xiang Xu authored and facebook-github-bot committed Oct 23, 2024
1 parent c9dc699 commit fbdfe10
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 11 deletions.
7 changes: 5 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ find_package(Threads REQUIRED)
find_library(ZSTD zstd)
find_library(BENCHMARK follybenchmark PATHS)
find_package(range-v3 REQUIRED)
find_package(Xxhash REQUIRED)

find_path(RE2_INCLUDE_DIR re2/re2.h)

Expand All @@ -70,9 +71,10 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
if (COMPILER_HAS_F_COROUTINES)
message(
STATUS
"GCC has support for C++ coroutines, setting flag for Folly build."
"GCC has support for C++ coroutines, "
"disabling since fbthrift service is not ready"
)
add_compile_options(-fcoroutines)
# add_compile_options(-fcoroutines)
else()
message(
STATUS
Expand All @@ -88,6 +90,7 @@ include_directories(
${FBTHRIFT_INCLUDE_DIR}
${FOLLY_INCLUDE_DIR}
${RE2_INCLUDE_DIR}
${Xxhash_INCLUDE_DIR}
)

#
Expand Down
13 changes: 7 additions & 6 deletions openr/ctrl-server/tests/OpenrCtrlHandlerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,7 @@ TEST_F(OpenrCtrlFixture, subscribeAndGetKvStoreFilteredWithKeysNoTtlUpdate) {
folly::getEventBase(), [&received, &diff, key](auto&& t) {
// Consider publication only if `key` is present
// NOTE: There can be updates to prefix or adj keys
if (!t.hasValue() or !t->keyVals()->contains(key)) {
if (!t.hasValue() || !t->keyVals()->count(key)) {
return;
}
auto& pub = *t;
Expand Down Expand Up @@ -1461,7 +1461,7 @@ TEST_F(OpenrCtrlFixture, subscribeAndGetKvStoreFilteredWithKeysNoTtlUpdate) {
.subscribeExTry(folly::getEventBase(), [&received, key](auto&& t) {
// Consider publication only if `key` is present
// NOTE: There can be updates to prefix or adj keys
if (!t.hasValue() or !t->keyVals()->contains(key)) {
if (!t.hasValue() || !t->keyVals()->count(key)) {
return;
}
auto& pub = *t;
Expand All @@ -1476,7 +1476,7 @@ TEST_F(OpenrCtrlFixture, subscribeAndGetKvStoreFilteredWithKeysNoTtlUpdate) {
.toClientStreamUnsafeDoNotUse()
.subscribeExTry(
folly::getEventBase(), [&received, random_key](auto&& t) {
if (!t.hasValue() or !t->keyVals()->contains(random_key)) {
if (!t.hasValue() || !t->keyVals()->count(random_key)) {
return;
}
auto& pub = *t;
Expand Down Expand Up @@ -1553,7 +1553,7 @@ TEST_F(OpenrCtrlFixture, subscribeAndGetKvStoreFilteredWithKeysNoTtlUpdate) {
.subscribeExTry(folly::getEventBase(), [&received, key](auto&& t) {
// Consider publication only if `key` is present
// NOTE: There can be updates to prefix or adj keys
if (!t.hasValue() or !t->keyVals()->contains(key)) {
if (!t.hasValue() || !t->keyVals()->count(key)) {
return;
}
auto& pub = *t;
Expand Down Expand Up @@ -1876,7 +1876,7 @@ TEST_F(OpenrCtrlFixture, subscribeAndGetKvStoreFilteredWithKeysNoTtlUpdate) {
std::move(responseAndSubscription.stream)
.toClientStreamUnsafeDoNotUse()
.subscribeExTry(folly::getEventBase(), [&received, key](auto&& t) {
if (!t.hasValue() or !t->keyVals()->contains(key)) {
if (!t.hasValue() || !t->keyVals()->count(key)) {
return;
}
auto& pub = *t;
Expand Down Expand Up @@ -2636,7 +2636,8 @@ TEST_F(OpenrCtrlFixture, SubscribeAndGetKvStore) {
}
auto& pub = *t;
XLOG(INFO) << fmt::format(
"Check publication for update: {}.", received);
"Check publication for update: {}.",
std::to_string(received));
checkPublications(expectedDeltas[received], pub);
received++;
});
Expand Down
3 changes: 1 addition & 2 deletions openr/decision/Decision.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ Decision::Decision(
*config->getConfig().decision_config()->save_rib_policy_max_ms()),
[this]() noexcept { saveRibPolicy(); }),
unblockInitialRoutesTimeout_(folly::AsyncTimeout::make(
*getEvb(),
std::bind_front(&Decision::forceInitialRoutesBuild, this))) {
*getEvb(), [this]() noexcept { forceInitialRoutesBuild(); })) {
// Create SpfSolver instance for best path calculation/selection
spfSolver_ = std::make_unique<SpfSolver>(
config->getNodeName(),
Expand Down
2 changes: 1 addition & 1 deletion openr/kvstore/tests/KvStoreTtlTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ TEST_P(KvStoreTestTtlFixture, Graph) {

VLOG(1) << "Adjacency list: ";
for (const auto&& [index, set] : adjacencyList | ranges::views::enumerate) {
VLOG(1) << index << ": " << fmt::format("{}", fmt::join(set, ", "));
VLOG(1) << index << ": " << fmt::format("{}", folly::join(", ", set));
}
performKvStoreSyncTest(
adjacencyList, "kv_store_graph::store", kNumStores, kNumStores + 1);
Expand Down

0 comments on commit fbdfe10

Please sign in to comment.