Fix autotools build

Summary: The OSS autotools build has been nonfunctional for about a year. While facebook/mcrouter#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 facebook/mcrouter#453, fixes facebook/mcrouter#444, fixes facebook/mcrouter#447 etc. X-link: facebook/mcrouter#457 Reviewed By: lenar-f Differential Revision: D67428119 Pulled By: disylh fbshipit-source-id: a22e7616d9b82875611e45bf1df40a771db569d9
facebook · Dec 19, 2024 · 8148af7 · 8148af7
1 parent 5715ae5
commit 8148af7
Show file tree

Hide file tree

Showing 13 changed files with 127 additions and 57 deletions.
diff --git 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
@@ -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"
@@ -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)),

diff --git a/third-party/mcrouter/src/mcrouter/ServerOnRequest.h b/third-party/mcrouter/src/mcrouter/ServerOnRequest.h
@@ -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"
@@ -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<std::string>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 .
diff --git 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
@@ -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
@@ -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 .