fmt: bump up version + related changes by Matan-B · Pull Request #58990 · ceph/ceph

Matan-B · 2024-08-01T13:30:39Z

For now I commented out all the problematic fmt usages that caused FTBFS on fmt 11.
This version compiles and we should fix the comments gradually.

I have separated commits into components:

Seastar submodule (Fixed, cherry-picked relevant fmt 11 fixes - https://github.com/ceph/seastar/commits/wip-seastar-fmt-11-fixes/)
common,include,msg (Fixed)
crimson (Fixed)
osd (pulled https://github.com/ronen-fr/ceph/commits/wip-rf-matan-fmt/)

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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
jenkins test rook e2e

Matan-B · 2024-08-01T14:30:59Z

jenkins test make check

Svelar · 2024-08-02T08:02:08Z

jenkins test make check

Matan-B · 2024-08-06T09:44:53Z

@ronen-fr, I've pulled https://github.com/ronen-fr/ceph/commits/wip-rf-matan-fmt/. Thank you!

src/common/fmt_common.h

src/crimson/osd/osd_operations/scrub_events.cc

src/crimson/osd/pg.cc

Matan-B · 2024-08-06T15:58:18Z

@ronen-fr, I think we should try to merge this in a single PR (instead of #58960). What do you think?
Can I cherry-pick latest #59012 as well?

ronen-fr · 2024-08-06T16:58:25Z

@ronen-fr, I think we should try to merge this in a single PR (instead of #58960). What do you think?
Can I cherry-pick latest #59012 as well?

No problem with cherry-picking 59012, but I think there is a good reason to have it run in parallel as a dedicated PR, so that we can run it thru (among others) some upgrade tests. BTW - I am already running some Teu tests for it (https://pulpito.ceph.com/?branch=wip-rf-hex-snapid)

ronen-fr · 2024-08-06T17:03:47Z

a single PR (instead of #58960).

merging with 58960 is a very good idea

Matan-B · 2024-08-07T13:20:59Z

jenkins test make check

Matan-B · 2024-08-13T11:13:57Z

fb56eaec6348d2e7b26d35a642df19812e0ee334 debuginfo(build-id) = ff40a944543d0a9f33231f5e439da948d4904786
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Recommends: ceph-debugsource(x86-64) = 2:19.0.0-5307.g40acc1af.el9
Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos9/DIST/centos9/MACHINE_SIZE/gigantic/release/19.0.0-5307-g40acc1af/rpm/el9/BUILDROOT/ceph-19.0.0-5307.g40acc1af.el9.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/include/fmt/args.h
   /usr/include/fmt/base.h
   /usr/include/fmt/chrono.h
   /usr/include/fmt/color.h
   /usr/include/fmt/compile.h
   /usr/include/fmt/core.h
   /usr/include/fmt/format-inl.h
   /usr/include/fmt/format.h
   /usr/include/fmt/os.h
   /usr/include/fmt/ostream.h
   /usr/include/fmt/printf.h
   /usr/include/fmt/ranges.h
   /usr/include/fmt/std.h
   /usr/include/fmt/xchar.h
   /usr/lib64/cmake/fmt/fmt-config-version.cmake
   /usr/lib64/cmake/fmt/fmt-config.cmake
   /usr/lib64/cmake/fmt/fmt-targets-debug.cmake
   /usr/lib64/cmake/fmt/fmt-targets.cmake
   /usr/lib64/libfmtd.a
   /usr/lib64/pkgconfig/fmt.pc


RPM build errors:
    extra tokens at the end of %else directive in line 118:  %else # not fedora/rhel

    extra tokens at the end of %else directive in line 121:  %else # not x86_64

    line 1173: It's not recommended to have unversioned Obsoletes: Obsoletes:	ceph-libcephfs
    Signature not supported. Hash algorithm SHA1 not available.
    Signature not supported. Hash algorithm SHA1 not available.
    Ignoring invalid regex ^%{_scl_prefix}/.*|%{_root_sysconfdir}/rpm/macros.gcc-toolset-12-config$
    Ignoring invalid regex ^%{_root_sysconfdir}/rpm/macros.gcc-toolset-12-config$

Matan-B · 2024-08-14T09:34:42Z

@cbodley, Centos builds are failing to create package due to unpackaged files. I'm not sure why as the current fmt we use include them as well:
https://github.com/ceph/fmt/tree/a33701196adfad74917046096bf5a2aa0ab0bb50/include/fmt
Do you perhaps know what's missing? Thanks!

Relevant pieces:
CMakeLists:

option(WITH_FMT_HEADER_ONLY "use header-only version of fmt library" OFF)
option(WITH_SYSTEM_FMT "build against system fmt" OFF)
if(WITH_SYSTEM_FMT)
  find_package(fmt 8.1.1...<10.0.0 REQUIRED)
endif()
if (WITH_FMT_HEADER_ONLY)
  message(STATUS "Using fmt header-only.")
  set(FMT_LIB fmt::fmt-header-only)
else()
  message(STATUS "Linking to fmt library.")
  set(FMT_LIB fmt::fmt)
endif()
if(fmt_FOUND)
  message(STATUS "Building with system fmt.")
else()
  message(STATUS "Building fmt as submodule")
  set(old_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
  set(BUILD_SHARED_LIBS FALSE)
  add_subdirectory(fmt)
  set(BUILD_SHARED_LIBS ${old_BUILD_SHARED_LIBS})
  unset(old_BUILD_SHARED_LIBS)
  include_directories(SYSTEM "${CMAKE_SOURCE_DIR}/src/fmt/include")
endif()

-- Using fmt header-only.
-- Building fmt as submodule
-- {fmt} version: 11.0.2

Note: rebasing

Move fmt submodule branch to https://github.com/ceph/fmt/commits/wip-fmt-11.0.2/ Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

Adding missing 'const', and placing in the fmt namespace. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Update seastar submodule to: https://github.com/ceph/seastar/commits/wip-seastar-fmt-11-fixes/ Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

i.e. - without the optional text representation of NOSNAP & SNAPDIR Signed-off-by: Ronen Friedman <rfriedma@redhat.com> (cherry picked from commit 547b67f) Signed-off-by: Matan Breizman <mbreizma@redhat.com>

``` RPM build errors: extra tokens at the end of %else directive in line 118: %else # not fedora/rhel extra tokens at the end of %else directive in line 121: %else # not x86_64 ``` Signed-off-by: Matan Breizman <mbreizma@redhat.com>

cbodley · 2024-08-14T13:09:01Z

@Matan-B i guess this is the relevant change from the diff in ceph/fmt@a337011...0c9fce2

-option(FMT_INSTALL "Generate the install target." ${FMT_MASTER_PROJECT})
+option(FMT_INSTALL "Generate the install target." ON)

we can either disable that option in src/CMakeLists.txt with:

+  set(FMT_INSTALL OFF)
   add_subdirectory(fmt)

or this should also work:

-  add_subdirectory(fmt)
+  add_subdirectory(fmt EXCLUDE_FROM_ALL)

Matan-B · 2024-08-14T15:04:39Z

@Matan-B i guess this is the relevant change from the diff in ceph/fmt@a337011...0c9fce2

Missed that diff. Thank you!

See: fmtlib/fmt#3264 Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Matan-B · 2024-08-15T12:41:02Z

jenkins test make check

Matan-B · 2024-08-15T12:41:07Z

jenkins test api

Matan-B · 2024-08-19T14:18:18Z

https://shaman.ceph.com/builds/ceph/wip-matanb-fmt-draft-install-off/1b9c60e5eeee58be68530de5a220a86fff6280de/

Matan-B · 2024-08-20T11:21:05Z

rados:
https://pulpito.ceph.com/matan-2024-08-22_08:42:00-rados-wip-matanb-fmt-draft-install-off-distro-default-smithi/
crimson-rados:
https://pulpito.ceph.com/matan-2024-08-20_09:46:04-crimson-rados-wip-matanb-fmt-draft-install-off-distro-crimson-smithi/

Matan-B · 2024-08-22T09:50:30Z

Testing look sane, beside infra/tracked failures. @rzarzynski, can I merge this one?

rzarzynski

LGTM with one single nit.

src/common/fmt_common.h

rzarzynski · 2024-08-22T11:51:39Z

Let's merge!

Matan-B changed the title ~~fmt 11 adjustments~~ [WIP] fmt 11 adjustments Aug 1, 2024

github-actions bot added common core crimson labels Aug 1, 2024

Matan-B assigned ronen-fr Aug 1, 2024

Matan-B requested review from Svelar and ronen-fr August 1, 2024 13:32

Matan-B mentioned this pull request Aug 1, 2024

fmt: bump up version to 11.0.2 #58960

Closed

14 tasks

Matan-B force-pushed the wip-matanb-fmt-draft branch from 46f0381 to 977c2a8 Compare August 6, 2024 09:43

ronen-fr reviewed Aug 6, 2024

View reviewed changes

src/common/fmt_common.h Show resolved Hide resolved

ronen-fr reviewed Aug 6, 2024

View reviewed changes

src/crimson/osd/osd_operations/scrub_events.cc Outdated Show resolved Hide resolved

ronen-fr reviewed Aug 6, 2024

View reviewed changes

src/crimson/osd/pg.cc Outdated Show resolved Hide resolved

Matan-B force-pushed the wip-matanb-fmt-draft branch 4 times, most recently from 174b9af to 9f681d6 Compare August 6, 2024 15:51

Matan-B changed the title ~~[WIP] fmt 11 adjustments~~ fmt: bump up version + related changes Aug 6, 2024

Matan-B marked this pull request as ready for review August 6, 2024 15:58

Matan-B requested review from a team as code owners August 6, 2024 15:58

Matan-B force-pushed the wip-matanb-fmt-draft branch from 9f681d6 to 2ecec8e Compare August 6, 2024 16:00

github-actions bot added the tests label Aug 7, 2024

Matan-B force-pushed the wip-matanb-fmt-draft branch from c88fc05 to 40acc1a Compare August 8, 2024 11:15

Matan-B added the wip-matan-testing label Aug 12, 2024

Matan-B requested a review from cbodley August 14, 2024 09:30

Matan-B requested a review from tchaikov August 14, 2024 09:44

Matan-B and others added 10 commits August 14, 2024 09:56

fmt: bump up version to 11.0.2

e5af3c6

Move fmt submodule branch to https://github.com/ceph/fmt/commits/wip-fmt-11.0.2/ Signed-off-by: Matan Breizman <mbreizma@redhat.com>

common,include,msg: fmt 11 fixes

7f77d50

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

osd,common: add explicit formatter for shard_id_t

3db48ac

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

osd: fix scrub_backend formatters

0e8da23

Adding missing 'const', and placing in the fmt namespace. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

osd/scrub: missing 'const' in scrubber_common.h formatter

bafdf39

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

seastar: cherry-pick fmt 11 fixes

755767f

Update seastar submodule to: https://github.com/ceph/seastar/commits/wip-seastar-fmt-11-fixes/ Signed-off-by: Matan Breizman <mbreizma@redhat.com>

crimson: fmt 11 fixes

63594b2

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

test/common/test_hobject: fmt fixes

4e3146c

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

osd: use an hex-only format for snap_id when creating snapmapper entries

21fe526

i.e. - without the optional text representation of NOSNAP & SNAPDIR Signed-off-by: Ronen Friedman <rfriedma@redhat.com> (cherry picked from commit 547b67f) Signed-off-by: Matan Breizman <mbreizma@redhat.com>

ceph.spec.in: spec files comments

d6c51a7

``` RPM build errors: extra tokens at the end of %else directive in line 118: %else # not fedora/rhel extra tokens at the end of %else directive in line 121: %else # not x86_64 ``` Signed-off-by: Matan Breizman <mbreizma@redhat.com>

Matan-B force-pushed the wip-matanb-fmt-draft branch from 40acc1a to d6c51a7 Compare August 14, 2024 09:57

github-actions bot added the build/ops label Aug 14, 2024

CMakeLists: set FMT_INSTALL OFF

ed0caa4

See: fmtlib/fmt#3264 Signed-off-by: Matan Breizman <mbreizma@redhat.com>

rzarzynski approved these changes Aug 22, 2024

View reviewed changes

src/common/fmt_common.h Show resolved Hide resolved

Matan-B merged commit f88d0c9 into ceph:main Aug 22, 2024

wlritchi mentioned this pull request Sep 16, 2024

ceph 18.2.4-3 bazaah/aur-ceph#26

Closed

Matan-B mentioned this pull request Mar 30, 2025

cmake: Allow building with fmt <= 11.1.4 #62564

Merged

14 tasks

Conversation

Matan-B commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

Matan-B commented Aug 1, 2024

Uh oh!

Svelar commented Aug 2, 2024

Uh oh!

Matan-B commented Aug 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Matan-B commented Aug 6, 2024

Uh oh!

ronen-fr commented Aug 6, 2024

Uh oh!

ronen-fr commented Aug 6, 2024

Uh oh!

Matan-B commented Aug 7, 2024

Uh oh!

Matan-B commented Aug 13, 2024

Uh oh!

Matan-B commented Aug 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbodley commented Aug 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Matan-B commented Aug 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Matan-B commented Aug 15, 2024

Uh oh!

Matan-B commented Aug 15, 2024

Uh oh!

Matan-B commented Aug 19, 2024

Uh oh!

Matan-B commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Matan-B commented Aug 22, 2024

Uh oh!

rzarzynski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rzarzynski commented Aug 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Matan-B commented Aug 1, 2024 •

edited

Loading

Matan-B commented Aug 14, 2024 •

edited

Loading

cbodley commented Aug 14, 2024 •

edited

Loading

Matan-B commented Aug 14, 2024 •

edited

Loading

Matan-B commented Aug 20, 2024 •

edited

Loading