Skip to content

Commit

Permalink
Fix autotools build (#457)
Browse files Browse the repository at this point in the history
Summary:
The OSS autotools build has been nonfunctional for about a year. While #449 would be a better long-term approach to fixing the build since it would also bring it in sync with other Meta OSS projects and enable testing in CI, fixing the existing build until there's movement on that front would at least provide a signal on whether the OSS version is buildable at all.

So:
* Introduce new Ubuntu 24.04 build files and use them in CI. I'll remove build files, Dockerfiles etc. for ancient Ubuntu and CentOS versions in a followup PR.
* Pin fmtlib to 11.0.2, like fbcode_builder does, and skip building its test suite to not waste time.
* Use Ninja to generate and build CMake-based dependencies.
* Use googletest, gmock and zstd as packaged by Ubuntu rather than building their older versions from source.
* Introduce and use a new define `MCROUTER_OSS_BUILD` to mark OSS-specific conditional compilation blocks. This could probably replace the existing `LIBMC_FBTRACE_DISABLE` and `DISABLE_COMPRESSION` defines too, as long as those aren't used outside of the OSS build.
* Fix OSS build failures that have gone undetected due to CI being broken:
  * Add a shim for the SR-specific method `HostInfoLocation::hostUniqueKey` (used since 85facb9).
  * Remove redundant CarbonRouterInstance deletion logging that was using a Meta-specific include (used since 6c2142a).
  * Disable client identity propagation in the OSS build since it relies on Meta-specific code (used since 2f32271).

Fixes #453, fixes #444, fixes #447 etc.

Pull Request resolved: #457

Reviewed By: lenar-f

Differential Revision: D67428119

Pulled By: disylh

fbshipit-source-id: a22e7616d9b82875611e45bf1df40a771db569d9
  • Loading branch information
mszabo-wikia authored and facebook-github-bot committed Dec 19, 2024
1 parent 0d6f0a8 commit 60a96f5
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 57 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ on:

jobs:
build:
runs-on: ubuntu-20.04
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v2
uses: actions/checkout@v4
- name: Build dependencies
run: |
./mcrouter/scripts/install_ubuntu_20.04.sh "$(pwd)"/mcrouter-install deps
./mcrouter/scripts/install_ubuntu_24.04.sh "$(pwd)"/mcrouter-install deps
- name: Build mcrouter
run: |
mkdir -p "$(pwd)"/mcrouter-install/install
./mcrouter/scripts/install_ubuntu_20.04.sh "$(pwd)"/mcrouter-install mcrouter
./mcrouter/scripts/install_ubuntu_24.04.sh "$(pwd)"/mcrouter-install mcrouter
9 changes: 2 additions & 7 deletions mcrouter/CarbonRouterInstance-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
#include <folly/io/async/EventBase.h>
#include <folly/json/DynamicConverter.h>

#include <common/logging/logging.h>

#include "mcrouter/AsyncWriter.h"
#include "mcrouter/CarbonRouterInstanceBase.h"
#include "mcrouter/ExecutorObserver.h"
Expand Down Expand Up @@ -129,11 +127,8 @@ CarbonRouterInstance<RouterInfo>* CarbonRouterInstance<RouterInfo>::createRaw(
initFailureLogger();
}

// Deleter is only called if unique_ptr::get() returns non-null.
auto deleter = [](CarbonRouterInstance<RouterInfo>* inst) {
LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance";
delete inst;
};
// Custom deleter since ~CarbonRouterInstance() is private.
auto deleter = [](CarbonRouterInstance<RouterInfo>* inst) { delete inst; };
auto router =
std::unique_ptr<CarbonRouterInstance<RouterInfo>, decltype(deleter)>(
new CarbonRouterInstance<RouterInfo>(std::move(input_options)),
Expand Down
8 changes: 7 additions & 1 deletion mcrouter/ServerOnRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
#include <cassert>
#include <memory>

#ifndef MCROUTER_OSS_BUILD
#include "core_infra_security/thrift_authentication_module/ClientIdentifierHelper.h"
#endif

#include "mcrouter/CarbonRouterClient.h"
#include "mcrouter/RequestAclChecker.h"
#include "mcrouter/config.h"
Expand Down Expand Up @@ -165,7 +168,8 @@ class ServerOnRequest {
auto& reqRef = rctx->req;
auto& ctxRef = rctx->ctx;

// Set hashed TLS client identities on request to propogate from proxy ->
#ifndef MCROUTER_OSS_BUILD
// Set hashed TLS client identities on request to propagate from proxy ->
// memcache server only IF enableKeyClientBinding_ is enabled.
if (FOLLY_UNLIKELY(enableKeyClientBinding_) &&
ctxRef.getThriftRequestContext()) {
Expand All @@ -180,6 +184,8 @@ class ServerOnRequest {
std::get<std::string>(mayBeHashedIdentities.value()));
}
}
#endif

// if we are reusing the request buffer, adjust the start offset and set
// it to the request.
if (reusableRequestBuffer) {
Expand Down
6 changes: 3 additions & 3 deletions mcrouter/configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ LT_INIT
CXXFLAGS="-fno-strict-aliasing -std=c++20 $CXXFLAGS"
CXXFLAGS="-W -Wall -Wextra -Wno-unused-parameter $CXXFLAGS"
CXXFLAGS=" -Wno-missing-field-initializers -Wno-deprecated-declarations $CXXFLAGS"
CXXFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION $CXXFLAGS"
CXXFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION -DMCROUTER_OSS_BUILD $CXXFLAGS"

CFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION $CFLAGS"
CFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION -DMCROUTER_OSS_BUILD $CFLAGS"

# Checks for glog and gflags
# There are no symbols with C linkage, so we do a try-run
Expand Down Expand Up @@ -199,7 +199,7 @@ AC_CHECK_FUNCS([gettimeofday \
LIBS="$LIBS $BOOST_LDFLAGS $BOOST_CONTEXT_LIB $BOOST_FILESYSTEM_LIB \
$BOOST_PROGRAM_OPTIONS_LIB $BOOST_SYSTEM_LIB $BOOST_REGEX_LIB \
$BOOST_THREAD_LIB -lpthread -pthread -ldl -lunwind \
-lbz2 -llz4 -llzma -lsnappy -lzstd -latomic"
-lbz2 -llz4 -llzma -lsnappy -lzstd -latomic -lxxhash"

AM_PATH_PYTHON([2.6],, [:])
AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :])
Expand Down
3 changes: 3 additions & 0 deletions mcrouter/mcrouter_sr_deps-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class HostInfoLocation {
const uint16_t* getTWTaskID() const {
return nullptr;
}
uint64_t hostUniqueKey() const noexcept {
return 0;
}

private:
const std::string ip_ = "127.0.0.1";
Expand Down
41 changes: 41 additions & 0 deletions mcrouter/scripts/Makefile_ubuntu-24.04
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

RECIPES_DIR := ./recipes

all: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done .fbthrift-done .gtest-done
${RECIPES_DIR}/mcrouter.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

mcrouter:
${RECIPES_DIR}/mcrouter.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

deps: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done .fbthrift-done
touch $@

.folly-done: .fmt-done
${RECIPES_DIR}/folly.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

.fizz-done: .folly-done
${RECIPES_DIR}/fizz.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

.wangle-done: .folly-done .fizz-done
${RECIPES_DIR}/wangle.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

.fmt-done:
${RECIPES_DIR}/fmtlib.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

.mvfst-done: .wangle-done
${RECIPES_DIR}/mvfst.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@

.fbthrift-done: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done
${RECIPES_DIR}/fbthrift.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR)
touch $@
54 changes: 54 additions & 0 deletions mcrouter/scripts/install_ubuntu_24.04.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#!/usr/bin/env bash
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

set -ex

BASE_DIR="$1"
TARGET="${2:-all}"

[ -n "$BASE_DIR" ] || ( echo "Base dir missing"; exit 1 )

sudo apt-get update

sudo apt-get install -y \
autoconf \
binutils-dev \
bison \
cmake \
flex \
g++ \
gcc \
git \
libboost1.83-all-dev \
libbz2-dev \
libdouble-conversion-dev \
libevent-dev \
libfast-float-dev \
libgflags-dev \
libgoogle-glog-dev \
libgmock-dev \
libgtest-dev \
libjemalloc-dev \
liblz4-dev \
liblzma-dev \
liblzma5 \
libsnappy-dev \
libsodium-dev \
libssl-dev \
libtool \
libunwind8-dev \
libxxhash-dev \
libzstd-dev \
make \
ninja-build \
pkg-config \
python3-dev \
ragel \
sudo

cd "$(dirname "$0")" || ( echo "cd fail"; exit 1 )

./get_and_build_by_make.sh "Makefile_ubuntu-24.04" "$TARGET" "$BASE_DIR"
4 changes: 2 additions & 2 deletions mcrouter/scripts/recipes/fbthrift.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ fi
cd "$PKG_DIR/fbthrift/build" || die "cd fbthrift failed"

CXXFLAGS="$CXXFLAGS -fPIC" \
cmake .. -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR"
make $MAKE_ARGS && make install $MAKE_ARGS
cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR"
cmake --build . && cmake --install .
4 changes: 2 additions & 2 deletions mcrouter/scripts/recipes/fizz.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ fi

cd "$PKG_DIR/fizz/fizz/" || die "cd fail"

cmake . -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
make $MAKE_ARGS && make install $MAKE_ARGS
cmake . -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
cmake --build . && cmake --install .
7 changes: 3 additions & 4 deletions mcrouter/scripts/recipes/fmtlib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
source common.sh

if [[ ! -d "$PKG_DIR/fmt" ]]; then
git clone https://github.com/fmtlib/fmt
git clone --depth 1 -b 11.0.2 https://github.com/fmtlib/fmt.git
cd "$PKG_DIR/fmt" || die "cd failed"
mkdir "$PKG_DIR/fmt/build"
fi

cd "$PKG_DIR/fmt/build" || die "cd fmt failed"

CXXFLAGS="$CXXFLAGS -fPIC" \
cmake .. -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR"
make $MAKE_ARGS && make install $MAKE_ARGS

cmake .. -G Ninja -DFMT_TEST=Off -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR"
cmake --build . && cmake --install .
32 changes: 2 additions & 30 deletions mcrouter/scripts/recipes/folly.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,42 +18,14 @@ if [[ ! -d folly ]]; then
fi
fi

if [ ! -d /usr/include/double-conversion ]; then
if [ ! -d "$PKG_DIR/double-conversion" ]; then
cd "$PKG_DIR" || die "cd fail"
git clone https://github.com/google/double-conversion.git
fi
cd "$PKG_DIR/double-conversion" || die "cd fail"

# Workaround double-conversion CMakeLists.txt changes that
# are incompatible with cmake-2.8
git checkout ea970f69edacf66bd3cba2892be284b76e9599b0
cmake . -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR"
make $MAKE_ARGS && make install $MAKE_ARGS

export LDFLAGS="-L$INSTALL_DIR/lib -ldl $LDFLAGS"
export CPPFLAGS="-I$INSTALL_DIR/include $CPPFLAGS"
fi

if [ ! -d "$PKG_DIR/zstd" ]; then
cd "$PKG_DIR" || die "cd fail"
git clone https://github.com/facebook/zstd

cd "$PKG_DIR/zstd" || die "cd fail"

# Checkout zstd-1.4.9 release
git checkout e4558ffd1dc49399faf4ee5d85abed4386b4dcf5
cmake -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" build/cmake/
make $MAKE_ARGS && make install $MAKE_ARGS
fi

cd "$PKG_DIR/folly/folly/" || die "cd fail"

CXXFLAGS="$CXXFLAGS -fPIC" \
LD_LIBRARY_PATH="$INSTALL_DIR/lib:$LD_LIBRARY_PATH" \
LD_RUN_PATH="$INSTALL_DIR/lib:$LD_RUN_PATH" \
cmake .. \
-G Ninja \
-DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" \
-DCMAKE_INCLUDE_PATH="$INSTALL_DIR/lib" \
-DCMAKE_LIBRARY_PATH="$INSTALL_DIR/lib"
make $MAKE_ARGS && make install $MAKE_ARGS
cmake --build . && cmake --install .
4 changes: 2 additions & 2 deletions mcrouter/scripts/recipes/mvfst.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ if [ ! -d "$PKG_DIR/mvfst" ]; then
cd "$PKG_DIR/mvfst" || die "cd fail"

cmake . \
-DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
make $MAKE_ARGS && make install $MAKE_ARGS
-G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
cmake --build . && cmake --install .
fi
4 changes: 2 additions & 2 deletions mcrouter/scripts/recipes/wangle.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ fi

cd "$PKG_DIR/wangle/wangle/" || die "cd fail"

cmake . -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
make $MAKE_ARGS && make install $MAKE_ARGS
cmake . -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF
cmake --build . && cmake --install .

0 comments on commit 60a96f5

Please sign in to comment.