Skip to content

common, include, osd: compile with fmt v9#47830

Merged
tchaikov merged 3 commits intoceph:mainfrom
tchaikov:wip-fmtlib-v9
Aug 30, 2022
Merged

common, include, osd: compile with fmt v9#47830
tchaikov merged 3 commits intoceph:mainfrom
tchaikov:wip-fmtlib-v9

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Aug 27, 2022

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@tchaikov tchaikov marked this pull request as ready for review August 27, 2022 03:52
@tchaikov tchaikov requested a review from a team as a code owner August 27, 2022 03:52
ronen-fr
ronen-fr requested changes Aug 27, 2022
View reviewed changes
Copy link
Contributor

@ronen-fr ronen-fr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (LVGTM...) apart from that extra #include

@ronen-fr
Copy link
Contributor

ronen-fr commented Aug 27, 2022

commit message description?

@tchaikov
common/Journald: include msg/msg_fmt.h
6e223a6 
so we can use the formatter defined for `entity_name_t`. in fmtlib v9,
it is required to define a specialization for the formatted type even
the type has an override of operator<<(). now that we already have a
formatter for `entity_name_t`, let's just use it.

this change should address the FTBFS when building with fmtlib v9.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
common/LogEntry: specialize fmt::formatter<LogEntry>
2901943 
so we can use the formatter defined for `LogEntry` in fmtlib v9.
in this new version of fmtlib, it is required to define a specialization
for the formatted type even when it comes to the types with an override of
operator<<(). since we already have an override for `LogEntry`, let's define
the specialization for `fmt::formatter<LogEntry>`.

this change should address the FTBFS when building with fmtlib v9.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
include/object_fmt: mark fmt::formatter<>::format() const
3d7a895 
so these formatters can be used in methods with `const` specifier.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 27, 2022 •
edited
Loading

commit message description?

@ronen-fr could you please be more specific? i think each commit in this PR has commit message. if any of them is missing or just wrong, please let me know.

@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 27, 2022

@ronen-fr fixed and repushed. could you take another look?

@ronen-fr
Copy link
Contributor

ronen-fr commented Aug 27, 2022

Sorry - meant the PR message, not the specific commits.

@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 28, 2022

jenkins test make check

@ronen-fr
Copy link
Contributor

ronen-fr commented Aug 29, 2022

jenkins retest this please

@cbodley
Copy link
Contributor

cbodley commented Aug 29, 2022

jenkins test api

1 similar comment
@tchaikov
Copy link
Contributor Author

tchaikov commented Aug 29, 2022

jenkins test api

@tchaikov tchaikov merged commit f68dbea into ceph:main Aug 30, 2022
@tchaikov tchaikov deleted the wip-fmtlib-v9 branch August 30, 2022 14:19
sseshasa added a commit to sseshasa/ceph that referenced this pull request Feb 5, 2024
@sseshasa
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
49b0aa5 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
Fixes: https://tracker.ceph.com/issues/64314
@sseshasa sseshasa mentioned this pull request Feb 5, 2024
Merged
14 tasks
sseshasa added a commit to sseshasa/ceph that referenced this pull request Feb 5, 2024
@sseshasa
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
9d7446c 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
Fixes: https://tracker.ceph.com/issues/64314
sseshasa added a commit to sseshasa/ceph that referenced this pull request Feb 5, 2024
@sseshasa
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
0bd01b9 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Fixes: https://tracker.ceph.com/issues/64314
Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
@sseshasa sseshasa mentioned this pull request Feb 26, 2024
Merged
14 tasks
bill-scales pushed a commit to bill-scales/ceph that referenced this pull request Mar 13, 2024
@sseshasa @bill-scales
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
e3f5f68 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Fixes: https://tracker.ceph.com/issues/64314
Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
sseshasa added a commit to sseshasa/ceph that referenced this pull request Mar 26, 2024
@sseshasa
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
69c2a89 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Fixes: https://tracker.ceph.com/issues/64314
Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
(cherry picked from commit 0bd01b9)
mkogan1 pushed a commit to mkogan1/ceph that referenced this pull request Aug 7, 2024
@sseshasa @bigjust
common/LogEntry: Add log level to str helper for fmt::formatter<LogEn…
be80006 
…try>

The Ceph cluster logs were missing the string equivalent [INF|WRN|ERR|DBG]
representation of the 'prio' field. This was broken since the introduction
of commit 2901943 of
PR: ceph#47830. This probably caused false
positives in teuthology testing and particularly for those tests that check
for cluster badness by parsing the cluster logs.

The fix involves adding a static helper function to the LogEntry struct.
This function returns the string appropriate representation of the log
level similar to the operator<<() for LogEntry.

Fixes: https://tracker.ceph.com/issues/64314
Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>
(cherry picked from commit 0bd01b9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ronen-fr ronen-fr ronen-fr approved these changes

@rzarzynski rzarzynski Awaiting requested review from rzarzynski

@cbodley cbodley Awaiting requested review from cbodley

Assignees

No one assigned

Labels

common core needs-qa

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tchaikov @ronen-fr @cbodley