Upgrade to libfmt 12#30171
Merged
Merged
Conversation
…d operator<< Add HasFormatToFreeFunction concept for types that provide a free function format_to(const T&, fmt::iterator) -> fmt::iterator, found via ADL. This is the counterpart of HasFormatToMethod for types that cannot have member functions (e.g. enums). Add auto-generated fmt::formatter and operator<< for both concepts. Add std::unique_ptr<T> formatter.
StephanDollberg
requested review from
BenPope and
Copilot
and removed request for
a team
github-actions
Bot
added
area/build
area/redpanda
area/wasm
StephanDollberg
force-pushed
the
stephan/remove-fmt-deprecated-ostream
branch
3 times, most recently
from
c898e4b to
9f687b5
Compare
Collaborator
CI test resultstest results on build#83185
test results on build#83304 |
Member
Author
|
/microbench |
Collaborator
|
Performance change detected in https://buildkite.com/redpanda/redpanda/builds/83187#019d9223-c89f-429d-9c8b-e2fa3e73a815: See https://redpandadata.atlassian.net/wiki/x/LQAqLg for docs |
dotnwat
reviewed
Apr 16, 2026
Comment on lines
-130
to
-133
| std::ostream& operator<<(std::ostream& o, const partition_path& p) { | ||
| o << ss::format("{}_{}", p.ntp.path(), p.revision_id); | ||
| return o; | ||
| } |
Member
There was a problem hiding this comment.
i think this one might be wrong? it looks like itw as replace din the header with make_string() but that has an extra prefix like {}/{}_{}?
dotnwat
previously approved these changes
Apr 16, 2026
Member
dotnwat
left a comment
dotnwat
left a comment
There was a problem hiding this comment.
it all looks ok, and it's hard to review this with super high confidence, but the tests pass which is a good sign.
i flagged a few things that looked like they might actually be problems, but i also would have expected a test failure if they were problematic, still worth taking a closer look.
Comment on lines
-284
to
288
| "exception thrown while converting AVRO {} schema of type {} to " | ||
| "exception thrown while converting AVRO schema of type {} to " | ||
| "iceberg - {}", | ||
| *node, | ||
| node->type(), | ||
| static_cast<int>(node->type()), | ||
| std::current_exception())); |
Member
There was a problem hiding this comment.
ahh, mismatched argument count? nice catch
Member
Author
There was a problem hiding this comment.
This actually looks like a regression (node removed). Let me check what's going on here. Could be third party type issues.
| fmt::format, | ||
| "topic manifest version {} is not supported", | ||
| handler._version)); | ||
| handler._version.value_or(-1))); |
Member
There was a problem hiding this comment.
where did the value_or come from?
Member
Author
There was a problem hiding this comment.
This is a workaround (probably for some failing test) for the builtin optional formatting.
Previously our formatter used {value/nullopt} but the builtin one does optional(value/none).
No strong opinion on this. Happy to just adjust the test also.
Comment on lines
+2727
to
+2726
| return os; | ||
| return fmt::format_to(out, ""); |
Member
Author
There was a problem hiding this comment.
Not sure what it was smoking in this file. Fixed to just return out.
Comment on lines
-91
to
-98
| * We need to use specific string representations of timestamp_type as this | ||
| * is related with protocol correctness | ||
| */ | ||
| switch (ts) { | ||
| case timestamp_type::append_time: | ||
| return os << "LogAppendTime"; | ||
| case timestamp_type::create_time: | ||
| return os << "CreateTime"; |
Member
There was a problem hiding this comment.
We need to use specific string representations of timestamp_type as this
is related with protocol correctness
Something to double check?
Member
Author
There was a problem hiding this comment.
Looks fine, not sure I like how it moved all the formatters into different places though.
Member
Author
There was a problem hiding this comment.
I'll move things back.
Comment on lines
-85
to
-86
| case proto_incompatibility_type::field_kind_changed: | ||
| return os << "FIELD_KIND_CHANGED"; |
Member
There was a problem hiding this comment.
the capitalization here makes me think these might be used in more than just logging
Member
Author
There was a problem hiding this comment.
Looks translated fine. Seems to be used in proto_incompatibility.
| }; | ||
|
|
||
| static constexpr std::string_view to_string_view(filter_pattern_type f) { | ||
| static inline fmt::iterator |
Member
There was a problem hiding this comment.
probably shouldn't be static in header here... (also like all the others in the file... but it was like this before so nbd...)
Comment on lines
+51
to
+52
| case event_type::num_elements: | ||
| return fmt::format_to(out, "invalid"); |
Member
There was a problem hiding this comment.
i can't find the original, but this one looked sus
Member
Author
There was a problem hiding this comment.
Interesting, this is actually a bug in the current code. num_elements is not listed but there is a default case (so no compiler error).
The translation is hence correct as num_elements will print "invalid" already. Crazy intelligence at work.
I'll fix.
| @@ -269,14 +269,14 @@ actor result_to_actor(const security::auth_result& result) { | |||
| } else if (result.acl.has_value() || result.resource_pattern.has_value()) { | |||
| ss::sstring desc; | |||
| if (result.acl.has_value()) { | |||
| desc += fmt::format("acl: {}", result.acl.value()); | |||
| desc += fmt::format("acl: {}", result.acl.value().get()); | |||
Member
Author
There was a problem hiding this comment.
value() is a reference wrapper so need get(). Don't actually see how this worked before but being more explicit seems fine.
StephanDollberg
force-pushed
the
stephan/remove-fmt-deprecated-ostream
branch
from
9f687b5 to
ddf7e9a
Compare
Collaborator
Retry command for Build#83304please wait until all jobs are finished before running the slash command |
Add base/external_fmt.h with fmt::formatter specializations for boost::beast::http::status/verb, boost::system::error_code, and boost::filesystem::path. Patch redpanda-data/avro fork to add const to format() methods required by fmt v11.
fmt::streamed(x) formats x via operator<<. But the blanket operator<<
in format_to.h delegates back to fmt for types with format_to, creating
infinite recursion:
format_to() → streamed(x) → operator<< → fmt::print → format_to()
Add fmt_streamed(), a checked wrapper that static_asserts the argument
does not satisfy HasFormatToMethod / HasFormatToFreeFunction. Types with
format_to are already directly formattable via {} — there is no reason
to go through streamed().
Migrate all fmt::streamed() call sites (except the auto_fmt.h fallback,
which is already guarded by !is_formattable) to fmt_streamed().
Also serves a reminder to remove streamed usages when implementing
format_to (for better perf).
Remove FMT_DEPRECATED_OSTREAM from redpanda_copts.
StephanDollberg
force-pushed
the
stephan/remove-fmt-deprecated-ostream
branch
from
ddf7e9a to
79c81fd
Compare
dotnwat
approved these changes
Apr 17, 2026
randomizedcoder
added a commit
to randomizedcoder/redpanda
that referenced
this pull request
Apr 28, 2026
Picks up the fmt 12 upgrade (PR redpanda-data#30171), liburing 2.14, rules_cc 0.2.17, Go SDK 1.26, and other upstream changes. Adapts the UDS broker_authn_endpoint formatting to use the new format_to() pattern required by fmt 12, unwrapping std::optional<broker_authn_method> explicitly. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade to libfmt 12. Remove no longer supported FMT_DEPRECATED_OSTREAM (so all types need explicit libfmt support now).
Reviewed by multiple rounds of:
Besides mechanical changes from operator<< to format_to there is also some explicit formatting changes from for example using built-in libfmt formatters for std types instead of our custom ones (e.g.: bool changes from 0/1 to false/true, optional goes from {val} to optional(val)).
Backports Required
Release Notes