Skip to content

Commit

Permalink
Fix fmt build errors for MacOS
Browse files Browse the repository at this point in the history
Recently brew updated fmt version from 10.1 to 11.0. This caused
build breaks. This commit contains 3 fixes:

1. Upgrade fmt to 11.0 from brew install. Remove the install_fmt call
to avoid mismatching versions.

2. Error "'this' argument to member function 'format' has type 'const
fmt::formatter<facebook::velox::exec::Spiller::Type>', but function
is not marked const".
The new fmt v11 forces the const check. To fix this error, the
"const" keyword was added to the `format()` function declarations.

3. Error "error: static assertion failed due to requirement
'formattable_char': Mixing character types is disallowed."
The new fmt v11 added a check if the arguments of fmt::make_format_args()
have mixing different character types (e.g., char vs wchar_t or
std::string vs folly::Range<const char*>). To fix this, we need to
convert the folly::StringPiece to string.

4. Update folly to "v2024.08.26.00", this is needed because the previous
version depends on fmt v10. To do this fast_float is added as one of
MACOS_VELOX_DEPS.

Note that if users see issues when building the new folly, they need to
make sure the CMake cache is cleaned and clean the build directory first.
  • Loading branch information
yingsu00 committed Sep 1, 2024
1 parent c74b5e1 commit 8262174
Show file tree
Hide file tree
Showing 36 changed files with 68 additions and 71 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ if(${VELOX_ENABLE_DUCKDB})
endif()

set_source(fmt)
resolve_dependency(fmt 9.0.0)
resolve_dependency(fmt)

if(${VELOX_BUILD_MINIMAL_WITH_DWIO} OR ${VELOX_ENABLE_HIVE_CONNECTOR})
# DWIO needs all sorts of stream compression libraries.
Expand Down
11 changes: 2 additions & 9 deletions scripts/setup-macos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ PYTHON_VENV=${PYHTON_VENV:-"${SCRIPTDIR}/../.venv"}
NPROC=$(getconf _NPROCESSORS_ONLN)

DEPENDENCY_DIR=${DEPENDENCY_DIR:-$(pwd)}
MACOS_VELOX_DEPS="bison boost double-conversion flex fmt gflags glog googletest icu4c libevent libsodium lz4 lzo openssl protobuf@21 simdjson snappy thrift xz xsimd zstd"
MACOS_VELOX_DEPS="bison boost double-conversion fast_float flex fmt gflags glog googletest icu4c libevent libsodium lz4 lzo openssl protobuf@21 simdjson snappy thrift xz xsimd zstd"
MACOS_BUILD_DEPS="ninja cmake ccache"
FB_OS_VERSION="v2024.05.20.00"
FMT_VERSION="10.1.1"
FB_OS_VERSION="v2024.08.26.00"

function update_brew {
DEFAULT_BREW_PATH=/usr/local/bin/brew
Expand Down Expand Up @@ -86,11 +85,6 @@ function install_velox_deps_from_brew {
done
}

function install_fmt {
wget_and_untar https://github.com/fmtlib/fmt/archive/${FMT_VERSION}.tar.gz fmt
cmake_install fmt -DFMT_TEST=OFF
}

function install_folly {
wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly
cmake_install folly -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
Expand Down Expand Up @@ -136,7 +130,6 @@ function install_velox_deps {
run_and_time install_ranges_v3
run_and_time install_double_conversion
run_and_time install_re2
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 velox/benchmarks/basic/LikeTpchBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ enum class TpchBenchmarkCase {

template <>
struct fmt::formatter<TpchBenchmarkCase> : fmt::formatter<int> {
auto format(const TpchBenchmarkCase& s, format_context& ctx) {
auto format(const TpchBenchmarkCase& s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
Expand Down
3 changes: 2 additions & 1 deletion velox/common/base/RuntimeMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ 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) {
auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx)
const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
5 changes: 2 additions & 3 deletions velox/common/base/SpillStats.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,8 @@ SpillStats globalSpillStats();
template <>
struct fmt::formatter<facebook::velox::common::SpillStats>
: fmt::formatter<std::string> {
auto format(
const facebook::velox::common::SpillStats& s,
format_context& ctx) {
auto format(const facebook::velox::common::SpillStats& s, format_context& ctx)
const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
4 changes: 2 additions & 2 deletions velox/common/base/Status.h
Original file line number Diff line number Diff line change
Expand Up @@ -519,15 +519,15 @@ using Expected = folly::Expected<T, Status>;

template <>
struct fmt::formatter<facebook::velox::Status> : fmt::formatter<std::string> {
auto format(const facebook::velox::Status& s, format_context& ctx) {
auto format(const facebook::velox::Status& s, format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};

template <>
struct fmt::formatter<facebook::velox::StatusCode>
: fmt::formatter<std::string_view> {
auto format(facebook::velox::StatusCode code, format_context& ctx) {
auto format(facebook::velox::StatusCode code, format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::toString(code), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/common/caching/AsyncDataCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ struct fmt::formatter<facebook::velox::cache::CoalescedLoad::State>
: formatter<int> {
auto format(
facebook::velox::cache::CoalescedLoad::State s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/common/compression/Compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct fmt::formatter<facebook::velox::common::CompressionKind>
: fmt::formatter<std::string> {
auto format(
const facebook::velox::common::CompressionKind& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::common::compressionKindToString(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ struct fmt::formatter<facebook::velox::memory::MemoryAllocator::InjectedFailure>
: fmt::formatter<int> {
auto format(
facebook::velox::memory::MemoryAllocator::InjectedFailure s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryArbitrator.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ struct fmt::formatter<facebook::velox::memory::MemoryArbitrator::Stats>
: formatter<std::string> {
auto format(
facebook::velox::memory::MemoryArbitrator::Stats s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
Expand Down
5 changes: 2 additions & 3 deletions velox/common/memory/MemoryPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -1107,9 +1107,8 @@ class StlAllocator {
template <>
struct fmt::formatter<facebook::velox::memory::MemoryPool::Kind>
: formatter<std::string> {
auto format(
facebook::velox::memory::MemoryPool::Kind s,
format_context& ctx) {
auto format(facebook::velox::memory::MemoryPool::Kind s, format_context& ctx)
const {
return formatter<std::string>::format(
facebook::velox::memory::MemoryPool::kindString(s), ctx);
}
Expand Down
4 changes: 2 additions & 2 deletions velox/connectors/fuzzer/FuzzerConnectorSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct fmt::formatter<facebook::velox::connector::fuzzer::FuzzerConnectorSplit>
: formatter<std::string> {
auto format(
facebook::velox::connector::fuzzer::FuzzerConnectorSplit s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
Expand All @@ -46,7 +46,7 @@ struct fmt::formatter<
auto format(
std::shared_ptr<facebook::velox::connector::fuzzer::FuzzerConnectorSplit>
s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s->toString(), ctx);
}
};
4 changes: 2 additions & 2 deletions velox/connectors/hive/HiveDataSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ struct fmt::formatter<facebook::velox::connector::hive::HiveDataSink::State>
: formatter<int> {
auto format(
facebook::velox::connector::hive::HiveDataSink::State s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
Expand All @@ -628,7 +628,7 @@ struct fmt::formatter<
: formatter<int> {
auto format(
facebook::velox::connector::hive::LocationHandle::TableType s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
1 change: 1 addition & 0 deletions velox/connectors/hive/HiveDataSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "velox/connectors/hive/HiveDataSource.h"

#include <fmt/ranges.h>
#include <string>
#include <unordered_map>

Expand Down
2 changes: 1 addition & 1 deletion velox/connectors/tpch/TpchConnectorSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct fmt::formatter<
: formatter<std::string> {
auto format(
std::shared_ptr<facebook::velox::connector::tpch::TpchConnectorSplit> s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s->toString(), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/core/PlanFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ struct fmt::formatter<facebook::velox::core::ExecutionStrategy>
: formatter<int> {
auto format(
const facebook::velox::core::ExecutionStrategy& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
4 changes: 2 additions & 2 deletions velox/core/PlanNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -2402,15 +2402,15 @@ struct fmt::formatter<facebook::velox::core::PartitionedOutputNode::Kind>
: formatter<std::string> {
auto format(
facebook::velox::core::PartitionedOutputNode::Kind s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::core::PartitionedOutputNode::kindString(s), ctx);
}
};

template <>
struct fmt::formatter<facebook::velox::core::JoinType> : formatter<int> {
auto format(facebook::velox::core::JoinType s, format_context& ctx) {
auto format(facebook::velox::core::JoinType s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
5 changes: 2 additions & 3 deletions velox/dwio/common/Options.h
Original file line number Diff line number Diff line change
Expand Up @@ -633,9 +633,8 @@ template <>
struct fmt::formatter<facebook::velox::dwio::common::FileFormat>
: fmt::formatter<std::string_view> {
template <typename FormatContext>
auto format(
facebook::velox::dwio::common::FileFormat fmt,
FormatContext& ctx) {
auto format(facebook::velox::dwio::common::FileFormat fmt, FormatContext& ctx)
const {
return formatter<std::string_view>::format(
facebook::velox::dwio::common::toString(fmt), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/dwrf/common/FileMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ class FooterWrapper : public ProtoWrapperBase {

template <>
struct fmt::formatter<facebook::velox::dwrf::DwrfFormat> : formatter<int> {
auto format(facebook::velox::dwrf::DwrfFormat s, format_context& ctx) {
auto format(facebook::velox::dwrf::DwrfFormat s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
14 changes: 7 additions & 7 deletions velox/dwio/parquet/thrift/ParquetThriftTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -3749,7 +3749,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::Type::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::Type::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3760,7 +3760,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::CompressionCodec::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::CompressionCodec::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3771,7 +3771,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::ConvertedType::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::ConvertedType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3783,7 +3783,7 @@ struct fmt::formatter<
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::FieldRepetitionType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3794,7 +3794,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::Encoding::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::Encoding::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3805,7 +3805,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::PageType::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::PageType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3816,7 +3816,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::BoundaryOrder::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::BoundaryOrder::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ DriverThreadContext* driverThreadContext();
template <>
struct fmt::formatter<facebook::velox::exec::StopReason>
: formatter<std::string> {
auto format(facebook::velox::exec::StopReason s, format_context& ctx) {
auto format(facebook::velox::exec::StopReason s, format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::exec::stopReasonString(s), ctx);
}
Expand Down
3 changes: 2 additions & 1 deletion velox/exec/HashBuild.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ inline std::ostream& operator<<(std::ostream& os, HashBuild::State state) {
template <>
struct fmt::formatter<facebook::velox::exec::HashBuild::State>
: formatter<std::string> {
auto format(facebook::velox::exec::HashBuild::State s, format_context& ctx) {
auto format(facebook::velox::exec::HashBuild::State s, format_context& ctx)
const {
return formatter<std::string>::format(
facebook::velox::exec::HashBuild::stateName(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/HashTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ struct fmt::formatter<facebook::velox::exec::BaseHashTable::HashMode>
: formatter<std::string> {
auto format(
facebook::velox::exec::BaseHashTable::HashMode s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::exec::BaseHashTable::modeString(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ class SourceOperator : public Operator {

template <>
struct fmt::formatter<std::thread::id> : formatter<std::string> {
auto format(std::thread::id s, format_context& ctx) {
auto format(std::thread::id s, format_context& ctx) const {
std::ostringstream oss;
oss << s;
return formatter<std::string>::format(oss.str(), ctx);
Expand Down
5 changes: 2 additions & 3 deletions velox/exec/ProbeOperatorState.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,8 @@ std::string probeOperatorStateName(ProbeOperatorState state);
template <>
struct fmt::formatter<facebook::velox::exec::ProbeOperatorState>
: formatter<int> {
auto format(
facebook::velox::exec::ProbeOperatorState s,
format_context& ctx) {
auto format(facebook::velox::exec::ProbeOperatorState s, format_context& ctx)
const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
3 changes: 2 additions & 1 deletion velox/exec/Spill.h
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ struct hash<::facebook::velox::exec::SpillPartitionId> {
template <>
struct fmt::formatter<facebook::velox::exec::SpillPartitionId>
: formatter<std::string> {
auto format(facebook::velox::exec::SpillPartitionId s, format_context& ctx) {
auto format(facebook::velox::exec::SpillPartitionId s, format_context& ctx)
const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
3 changes: 2 additions & 1 deletion velox/exec/Spiller.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ class Spiller {

template <>
struct fmt::formatter<facebook::velox::exec::Spiller::Type> : formatter<int> {
auto format(facebook::velox::exec::Spiller::Type s, format_context& ctx) {
auto format(facebook::velox::exec::Spiller::Type s, format_context& ctx)
const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
5 changes: 2 additions & 3 deletions velox/exec/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -1211,9 +1211,8 @@ std::ostream& operator<<(std::ostream& out, Task::ExecutionMode mode);
template <>
struct fmt::formatter<facebook::velox::exec::Task::ExecutionMode>
: formatter<std::string> {
auto format(
facebook::velox::exec::Task::ExecutionMode m,
format_context& ctx) {
auto format(facebook::velox::exec::Task::ExecutionMode m, format_context& ctx)
const {
return formatter<std::string>::format(
facebook::velox::exec::executionModeString(m), ctx);
}
Expand Down
Loading

0 comments on commit 8262174

Please sign in to comment.