-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade fmt to 10.1.1 (from 8.0.1) #7941
Changes from all commits
d773004
78ff5c3
ca6d5a9
6f8976d
2509d18
8dc32b2
a5e2fc4
68f634e
5b19954
c46a456
b98c8d3
174f4a8
e0eba43
17e63ae
86787f7
328095d
cfb794d
6150585
442b6a9
badddd2
2d72661
4815d4f
93da473
024f507
e74ce8f
1811482
ba22d82
33bc29d
4718b1f
90e6a8a
f309e25
28db132
c48bf1b
d2edfe1
a884cc9
aaacd15
2b58bb2
de9f102
e5d726e
430e425
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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://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 & | ||
|
||
FB_OS_VERSION="v2022.11.14.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/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 | ||
|
||
FB_OS_VERSION="v2023.12.04.00" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional do we plan to upgrade folly and format in this pr ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kind of but not really, only the preinstalled version in the container. This is necessary as mvfast doen't have a version for our old tag and is required for fbthrift. I tried a few older tags of mvfast but I was unable to find a combination of tags of folly and the others that compiled successfully. That's also why I sed out the deprecated annotation from the header further down in the script. |
||
|
||
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. | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment on why we need this dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for fbthirft and was missing previously, not sure how the existing image didn't have this... |
||
cmake_install fbthrift -Denable_tests=OFF | ||
# cmake_install ranges-v3 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -240,3 +240,10 @@ int main(int argc, char* argv[]) { | |
|
||
return 0; | ||
} | ||
|
||
template <> | ||
struct fmt::formatter<TpchBenchmarkCase> : formatter<int> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, All the formatter<...> can be replaced with the much simpler format_as() . I will make those changes too.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that will make it not be backwards compatible with < 9 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (just an fyi as prestissimo uses fmt 8.1 afaik) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok - This was failing in buck with specialization before instantiation , but I think theres another way around it. |
||
auto format(const TpchBenchmarkCase& s, format_context& ctx) { | ||
return formatter<int>::format(static_cast<int>(s), ctx); | ||
} | ||
}; |
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> | ||
assignUser marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the name should contain a date to know when this image was created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change will be reverted as the image will auto-update once this is merged. But I agree that the current state of the images is not optimal but I think that with the move to gha (#7974) we have a good opporunity to rework that a bit.