Skip to content

Commit

Permalink
Upgrade fmt to 10.1.1 (from 8.0.1) (facebookincubator#8434)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebookincubator#8434

fmt 8.0.1 is not upwards compatible to 9+ but backwards compatibility should not be an issue from what I could see in the logs and have done during facebookincubator#7700 (newst folly is compatible with fmt 10 but we built it with 8.0.1).

Closes facebookincubator#7896

Pull Request resolved: facebookincubator#7941

Reviewed By: Yuhta

Differential Revision: D52880145

Pulled By: kgpai

fbshipit-source-id: 4c8aa99b6be6a7dc1d2f4854145830f1254fbd2c
  • Loading branch information
assignUser authored and facebook-github-bot committed Jan 20, 2024
1 parent 3cf1f34 commit 826ee28
Show file tree
Hide file tree
Showing 75 changed files with 718 additions and 131 deletions.
2 changes: 1 addition & 1 deletion .circleci/dist_compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ commands:
executors:
build:
docker:
- image : ghcr.io/facebookincubator/velox-dev:circleci-avx
- image : ghcr.io/facebookincubator/velox-dev:new-cci
resource_class: 2xlarge
environment:
CC: /opt/rh/gcc-toolset-9/root/bin/gcc
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ jobs:
fetch-depth: 0
submodules: 'recursive'

- name: "Install dependencies"
if: ${{ github.event_name == 'pull_request' }}
run: source velox/scripts/setup-ubuntu.sh

- name: "Checkout Merge Base"
if: ${{ github.event_name == 'pull_request' }}
working-directory: velox
Expand All @@ -90,6 +86,10 @@ jobs:
git submodule update --init --recursive
echo $(git log -n 1)
- name: "Install dependencies"
if: ${{ github.event_name == 'pull_request' }}
run: source velox/scripts/setup-ubuntu.sh

- name: Build Baseline Benchmarks
if: ${{ github.event_name == 'pull_request' }}
working-directory: velox
Expand Down
8 changes: 3 additions & 5 deletions CMake/resolve_dependency_modules/fmt.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.
include_guard(GLOBAL)

set(VELOX_FMT_VERSION 8.0.1)
set(VELOX_FMT_VERSION 10.1.1)
set(VELOX_FMT_BUILD_SHA256_CHECKSUM
b06ca3130158c625848f3fb7418f235155a4d389b2abc3a6245fb01cb0eb1e01)
78b8c0a72b1c35e4443a7e308df52498252d1cefc2b08c9a97bc9ee6cfe61f8b)
set(VELOX_FMT_SOURCE_URL
"https://github.com/fmtlib/fmt/archive/${VELOX_FMT_VERSION}.tar.gz")

Expand All @@ -25,9 +25,7 @@ message(STATUS "Building fmt from source")
FetchContent_Declare(
fmt
URL ${VELOX_FMT_SOURCE_URL}
URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM}
PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/fmt/no-targets.patch)

URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM})
# Force fmt to create fmt-config.cmake which can be found by other dependecies
# (e.g. folly)
set(FMT_INSTALL ON)
Expand Down
12 changes: 0 additions & 12 deletions CMake/resolve_dependency_modules/fmt/no-targets.patch

This file was deleted.

16 changes: 8 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,6 @@ if(VELOX_ENABLE_PARQUET)
set(VELOX_ENABLE_ARROW ON)
endif()

if(VELOX_ENABLE_REMOTE_FUNCTIONS)
# TODO: Move this to use resolve_dependency(). For some reason, FBThrift
# requires clients to explicitly install fizz and wangle.
find_package(fizz CONFIG REQUIRED)
find_package(wangle CONFIG REQUIRED)
find_package(FBThrift CONFIG REQUIRED)
endif()

# define processor variable for conditional compilation
if(${VELOX_CODEGEN_SUPPORT})
add_compile_definitions(CODEGEN_ENABLED=1)
Expand Down Expand Up @@ -457,6 +449,14 @@ add_compile_definitions(FOLLY_HAVE_INT128_T=1)
set_source(folly)
resolve_dependency(folly)

if(VELOX_ENABLE_REMOTE_FUNCTIONS)
# TODO: Move this to use resolve_dependency(). For some reason, FBThrift
# requires clients to explicitly install fizz and wangle.
find_package(fizz CONFIG REQUIRED)
find_package(wangle CONFIG REQUIRED)
find_package(FBThrift CONFIG REQUIRED)
endif()

if(DEFINED FOLLY_BENCHMARK_STATIC_LIB)
set(FOLLY_BENCHMARK ${FOLLY_BENCHMARK_STATIC_LIB})
else()
Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-centos8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog &
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost &
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy &
wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt &
wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt &

wait # For cmake and source downloads to complete.

Expand Down
39 changes: 24 additions & 15 deletions scripts/setup-circleci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function dnf_install {

dnf_install epel-release dnf-plugins-core # For ccache, ninja
dnf config-manager --set-enabled powertools
dnf_install ninja-build ccache gcc-toolset-9 git wget which libevent-devel \
dnf_install ninja-build cmake curl ccache gcc-toolset-9 git wget which libevent-devel \
openssl-devel re2-devel libzstd-devel lz4-devel double-conversion-devel \
libdwarf-devel curl-devel libicu-devel

Expand Down Expand Up @@ -58,29 +58,34 @@ function wget_and_untar {
local URL=$1
local DIR=$2
mkdir -p "${DIR}"
wget -q --max-redirect 3 -O - "${URL}" | tar -xz -C "${DIR}" --strip-components=1
pushd "${DIR}"
curl -L "${URL}" > $2.tar.gz
tar -xz --strip-components=1 -f $2.tar.gz
popd
}

# untar cmake binary release directly to /usr.
wget_and_untar https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz /usr &

# Fetch sources.
wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags &
wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog &
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost &
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy &
wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt &
# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 &
wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags
wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy
wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt

# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3
wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf &
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf

FB_OS_VERSION="v2022.11.14.00"
FB_OS_VERSION="v2023.12.04.00"

wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly &
wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz &
wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle &
wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift &
wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz
wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly
wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle
wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift
wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst

wait # For cmake and source downloads to complete.

Expand Down Expand Up @@ -113,9 +118,13 @@ cmake_install glog -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr
cmake_install snappy -DSNAPPY_BUILD_TESTS=OFF
cmake_install fmt -DFMT_TEST=OFF
cmake_install folly -DFOLLY_HAVE_INT128_T=ON
# remove in #7700
sed -i 's/\[\[deprecated.*\]\]//g' /usr/local/include/folly/init/Init.h


cmake_install fizz/fizz -DBUILD_TESTS=OFF
cmake_install wangle/wangle -DBUILD_TESTS=OFF
cmake_install mvfst -DBUILD_TESTS=OFF
cmake_install fbthrift -Denable_tests=OFF
# cmake_install ranges-v3

Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-macos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function install_build_prerequisites {
}

function install_fmt {
github_checkout fmtlib/fmt 8.0.1
github_checkout fmtlib/fmt 10.1.1
cmake_install -DFMT_TEST=OFF
}

Expand Down
7 changes: 1 addition & 6 deletions scripts/setup-ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ sudo --preserve-env apt update && sudo --preserve-env apt install -y libunwind-d
libboost-all-dev \
libicu-dev \
libdouble-conversion-dev \
libfmt-dev \
libgoogle-glog-dev \
libbz2-dev \
libgflags-dev \
Expand Down Expand Up @@ -86,11 +87,6 @@ function prompt {
) 2> /dev/null
}

function install_fmt {
github_checkout fmtlib/fmt 8.0.1
cmake_install -DFMT_TEST=OFF
}

function install_folly {
github_checkout facebook/folly "${FB_OS_VERSION}"
cmake_install -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
Expand Down Expand Up @@ -119,7 +115,6 @@ function install_conda {
}

function install_velox_deps {
run_and_time install_fmt
run_and_time install_folly
run_and_time install_fizz
run_and_time install_wangle
Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-velox-torcharrow.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ wget_and_untar https://github.com/gflags/gflags/archive/refs/tags/v2.2.2.tar.gz
wget_and_untar https://ftp.openssl.org/source/openssl-1.1.1k.tar.gz openssl &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.69.0/source/boost_1_69_0.tar.gz boost &
wget_and_untar https://github.com/facebook/folly/archive/v2022.11.14.00.tar.gz folly &
wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/8.0.1.tar.gz fmt &
wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz fmt &

wait

Expand Down
9 changes: 9 additions & 0 deletions velox/benchmarks/basic/LikeTpchBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,16 @@ enum class TpchBenchmarkCase {
TpchQuery16Supplier,
TpchQuery20,
};
}

template <>
struct fmt::formatter<TpchBenchmarkCase> : fmt::formatter<int> {
auto format(const TpchBenchmarkCase& s, format_context& ctx) {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};

namespace {
class LikeFunctionsBenchmark : public FunctionBaseTest,
public FunctionBenchmarkBase {
public:
Expand Down
1 change: 1 addition & 0 deletions velox/common/base/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <folly/Conv.h>
#include <folly/Exception.h>
#include <folly/Preprocessor.h>
#include "FmtStdFormatters.h"
#include "velox/common/base/VeloxException.h"

namespace facebook::velox {
Expand Down
106 changes: 106 additions & 0 deletions velox/common/base/FmtStdFormatters.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <fmt/format.h>
#if FMT_VERSION >= 100100
#include <fmt/std.h>
#endif

#include <atomic>
#include <bitset>
#include <string_view>
#include <system_error>
#include <type_traits>
#include <vector>

template <typename Char>
struct fmt::formatter<std::errc, Char>
: formatter<std::underlying_type<std::errc>::type, Char> {
template <typename FormatContext>
auto format(std::errc v, FormatContext& ctx) const -> decltype(ctx.out()) {
using underlying_type = std::underlying_type<std::errc>::type;
return formatter<underlying_type, Char>::format(
static_cast<underlying_type>(v), ctx);
}
};

#if FMT_VERSION < 100100
// This should be 100101 but FMT_VERSION was not bumped in 10.1.1
// but under a month has passed since 10.1.0 release so we can assume 10.1.1
//
// Backport from fmt 10.1.1 see fmtlib/fmt#3574
// Formats std::atomic
template <typename T, typename Char>
struct fmt::formatter<
std::atomic<T>,
Char,
std::enable_if_t<fmt::is_formattable<T, Char>::value>>
: formatter<T, Char> {
template <typename FormatContext>
auto format(const std::atomic<T>& v, FormatContext& ctx) const
-> decltype(ctx.out()) {
return formatter<T, Char>::format(v.load(), ctx);
}
};
#endif

#if FMT_VERSION < 100100
// Backport from fmt 10.1 see fmtlib/fmt#3570
// Formats std::vector<bool>
namespace fmt::detail {
template <typename T, typename Enable = void>
struct has_flip : std::false_type {};

template <typename T>
struct has_flip<T, std::void_t<decltype(std::declval<T>().flip())>>
: std::true_type {};

template <typename T>
struct is_bit_reference_like {
static constexpr const bool value = std::is_convertible<T, bool>::value &&
std::is_nothrow_assignable<T, bool>::value && has_flip<T>::value;
};

#ifdef _LIBCPP_VERSION

// Workaround for libc++ incompatibility with C++ standard.
// According to the Standard, `bitset::operator[] const` returns bool.
template <typename C>
struct is_bit_reference_like<std::__bit_const_reference<C>> {
static constexpr const bool value = true;
};

#endif
} // namespace fmt::detail

// We can't use std::vector<bool, Allocator>::reference and
// std::bitset<N>::reference because the compiler can't deduce Allocator and N
// in partial specialization.
template <typename BitRef, typename Char>
struct fmt::formatter<
BitRef,
Char,
std::enable_if_t<fmt::detail::is_bit_reference_like<BitRef>::value>>
: formatter<bool, Char> {
template <typename FormatContext>
FMT_CONSTEXPR auto format(const BitRef& v, FormatContext& ctx) const
-> decltype(ctx.out()) {
return formatter<bool, Char>::format(v, ctx);
}
};
#endif
6 changes: 6 additions & 0 deletions velox/common/base/RuntimeMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,9 @@ class RuntimeStatWriterScopeGuard {
};

} // namespace facebook::velox
template <>
struct fmt::formatter<facebook::velox::RuntimeCounter::Unit> : formatter<int> {
auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx) {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
1 change: 1 addition & 0 deletions velox/common/caching/AsyncDataCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "velox/common/caching/SsdCache.h"

#include "velox/common/base/Counters.h"
#include "velox/common/base/Exceptions.h"
#include "velox/common/base/StatsReporter.h"
#include "velox/common/base/SuccinctPrinter.h"
#include "velox/common/caching/FileIds.h"
Expand Down
1 change: 1 addition & 0 deletions velox/common/caching/SsdCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "velox/common/caching/SsdCache.h"
#include <folly/Executor.h>
#include <folly/portability/SysUio.h>
#include "velox/common/base/Exceptions.h"
#include "velox/common/caching/FileIds.h"
#include "velox/common/file/FileSystems.h"
#include "velox/common/testutil/TestValue.h"
Expand Down
3 changes: 2 additions & 1 deletion velox/common/compression/Compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ CompressionKind codecTypeToCompressionKind(folly::io::CodecType type) {
case folly::io::CodecType::GZIP:
return CompressionKind_GZIP;
default:
VELOX_UNSUPPORTED("Not support folly codec type {}", type);
VELOX_UNSUPPORTED(
"Not support folly codec type {}", folly::to<std::string>(type));
}
}

Expand Down
Loading

0 comments on commit 826ee28

Please sign in to comment.