From 8148af719186ba9fed3249bef945ff83661eabcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Wed, 18 Dec 2024 21:58:16 -0800 Subject: [PATCH] Fix autotools build Summary: The OSS autotools build has been nonfunctional for about a year. While https://github.com/facebook/mcrouter/issues/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 85facb9200dea75e661447ed11e05806f42de65f). * Remove redundant CarbonRouterInstance deletion logging that was using a Meta-specific include (used since 6c2142acd8e69edd40eed70a93ea17ee2909287d). * Disable client identity propagation in the OSS build since it relies on Meta-specific code (used since 2f32271533fdf54ce71cfb65f6fda3c621f076a4). Fixes https://github.com/facebook/mcrouter/issues/453, fixes https://github.com/facebook/mcrouter/issues/444, fixes https://github.com/facebook/mcrouter/issues/447 etc. X-link: https://github.com/facebook/mcrouter/pull/457 Reviewed By: lenar-f Differential Revision: D67428119 Pulled By: disylh fbshipit-source-id: a22e7616d9b82875611e45bf1df40a771db569d9 --- .../mcrouter/src/.github/workflows/build.yml | 8 +-- .../src/mcrouter/CarbonRouterInstance-inl.h | 9 +--- .../mcrouter/src/mcrouter/ServerOnRequest.h | 8 ++- .../mcrouter/src/mcrouter/configure.ac | 6 +-- .../src/mcrouter/mcrouter_sr_deps-impl.h | 3 ++ .../mcrouter/scripts/Makefile_ubuntu-24.04 | 41 ++++++++++++++ .../mcrouter/scripts/install_ubuntu_24.04.sh | 54 +++++++++++++++++++ .../src/mcrouter/scripts/recipes/fbthrift.sh | 4 +- .../src/mcrouter/scripts/recipes/fizz.sh | 4 +- .../src/mcrouter/scripts/recipes/fmtlib.sh | 7 ++- .../src/mcrouter/scripts/recipes/folly.sh | 32 +---------- .../src/mcrouter/scripts/recipes/mvfst.sh | 4 +- .../src/mcrouter/scripts/recipes/wangle.sh | 4 +- 13 files changed, 127 insertions(+), 57 deletions(-) create mode 100644 third-party/mcrouter/src/mcrouter/scripts/Makefile_ubuntu-24.04 create mode 100755 third-party/mcrouter/src/mcrouter/scripts/install_ubuntu_24.04.sh diff --git a/third-party/mcrouter/src/.github/workflows/build.yml b/third-party/mcrouter/src/.github/workflows/build.yml index 0b3fcafd249e7e..837c5160590f87 100644 --- a/third-party/mcrouter/src/.github/workflows/build.yml +++ b/third-party/mcrouter/src/.github/workflows/build.yml @@ -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 diff --git a/third-party/mcrouter/src/mcrouter/CarbonRouterInstance-inl.h b/third-party/mcrouter/src/mcrouter/CarbonRouterInstance-inl.h index ccbd4f30bb148f..bfb385f05155bd 100644 --- a/third-party/mcrouter/src/mcrouter/CarbonRouterInstance-inl.h +++ b/third-party/mcrouter/src/mcrouter/CarbonRouterInstance-inl.h @@ -18,8 +18,6 @@ #include #include -#include - #include "mcrouter/AsyncWriter.h" #include "mcrouter/CarbonRouterInstanceBase.h" #include "mcrouter/ExecutorObserver.h" @@ -129,11 +127,8 @@ CarbonRouterInstance* CarbonRouterInstance::createRaw( initFailureLogger(); } - // Deleter is only called if unique_ptr::get() returns non-null. - auto deleter = [](CarbonRouterInstance* inst) { - LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance"; - delete inst; - }; + // Custom deleter since ~CarbonRouterInstance() is private. + auto deleter = [](CarbonRouterInstance* inst) { delete inst; }; auto router = std::unique_ptr, decltype(deleter)>( new CarbonRouterInstance(std::move(input_options)), diff --git a/third-party/mcrouter/src/mcrouter/ServerOnRequest.h b/third-party/mcrouter/src/mcrouter/ServerOnRequest.h index 35738e67573419..045b985b335ed8 100644 --- a/third-party/mcrouter/src/mcrouter/ServerOnRequest.h +++ b/third-party/mcrouter/src/mcrouter/ServerOnRequest.h @@ -11,7 +11,10 @@ #include #include +#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" @@ -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()) { @@ -180,6 +184,8 @@ class ServerOnRequest { std::get(mayBeHashedIdentities.value())); } } +#endif + // if we are reusing the request buffer, adjust the start offset and set // it to the request. if (reusableRequestBuffer) { diff --git a/third-party/mcrouter/src/mcrouter/configure.ac b/third-party/mcrouter/src/mcrouter/configure.ac index 8d64cb75bdd011..a84c8b4a00bdb8 100644 --- a/third-party/mcrouter/src/mcrouter/configure.ac +++ b/third-party/mcrouter/src/mcrouter/configure.ac @@ -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 @@ -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" != :]) diff --git a/third-party/mcrouter/src/mcrouter/mcrouter_sr_deps-impl.h b/third-party/mcrouter/src/mcrouter/mcrouter_sr_deps-impl.h index 97468f6a72af21..21e6e7808fca12 100644 --- a/third-party/mcrouter/src/mcrouter/mcrouter_sr_deps-impl.h +++ b/third-party/mcrouter/src/mcrouter/mcrouter_sr_deps-impl.h @@ -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"; diff --git a/third-party/mcrouter/src/mcrouter/scripts/Makefile_ubuntu-24.04 b/third-party/mcrouter/src/mcrouter/scripts/Makefile_ubuntu-24.04 new file mode 100644 index 00000000000000..3dfe67b7f9aef5 --- /dev/null +++ b/third-party/mcrouter/src/mcrouter/scripts/Makefile_ubuntu-24.04 @@ -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 $@ diff --git a/third-party/mcrouter/src/mcrouter/scripts/install_ubuntu_24.04.sh b/third-party/mcrouter/src/mcrouter/scripts/install_ubuntu_24.04.sh new file mode 100755 index 00000000000000..fe29a0e56ae294 --- /dev/null +++ b/third-party/mcrouter/src/mcrouter/scripts/install_ubuntu_24.04.sh @@ -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" diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/fbthrift.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/fbthrift.sh index a4e84bef89f3e8..eecb1a05fbfc69 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/fbthrift.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/fbthrift.sh @@ -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 . diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/fizz.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/fizz.sh index d97c4c20826501..b5495fcd1f9adf 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/fizz.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/fizz.sh @@ -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 . diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/fmtlib.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/fmtlib.sh index 0bd074deadc8cc..63b63acdd83611 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/fmtlib.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/fmtlib.sh @@ -7,7 +7,7 @@ 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 @@ -15,6 +15,5 @@ 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 . diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/folly.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/folly.sh index 4ff8f9f1aca3dd..b351677b07c210 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/folly.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/folly.sh @@ -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 . diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/mvfst.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/mvfst.sh index 73c69a1c3953dc..aa7c023272373f 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/mvfst.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/mvfst.sh @@ -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 diff --git a/third-party/mcrouter/src/mcrouter/scripts/recipes/wangle.sh b/third-party/mcrouter/src/mcrouter/scripts/recipes/wangle.sh index 38e028e620d4f2..e700d73ebc7aba 100755 --- a/third-party/mcrouter/src/mcrouter/scripts/recipes/wangle.sh +++ b/third-party/mcrouter/src/mcrouter/scripts/recipes/wangle.sh @@ -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 .