Update to spdlog>=1.11.0, fmt>=9.1.0. #1177

bdice · 2022-12-07T20:32:46Z

Description

Updates to spdlog>=1.11.0 and fmt>=9.1.0. Also resolves some issues with spdlog in the librmm conda packages. Thanks @robertmaynard for helping advise me on this PR.

We need to test this downstream before merging. Perhaps with cuML or some other library.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

codecov-commenter · 2022-12-07T20:56:50Z

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (62c1bf1) compared to base (f733e6f).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff              @@
##           branch-23.02   #1177   +/-   ##
============================================
  Coverage          0.00%   0.00%           
============================================
  Files                 6       6           
  Lines               421     421           
============================================
  Misses              421     421

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

dependencies.yaml

conda/recipes/librmm/meta.yaml

kkraus14 · 2023-01-12T17:28:11Z

@bdice gentle ping on this. Anything I can do to help move this forward?

ajschmidt8 · 2023-01-12T17:34:45Z

gentle ping on this. Anything I can do to help move this forward?

@kkraus14, for awareness, Bradley is OoO until 1/17.

bdice · 2023-01-12T17:51:53Z

This is on my radar. I’ve been working on other CI migration and platform support issues since the holidays but will return to it as soon as I can.

kkraus14 · 2023-01-19T02:02:00Z

Just want to make sure this doesn't slip to 23.04 as this is a growing blocker as the conda-forge pinnings are at spdlog 1.11.0 and fmt 9, which means all new packages that rely on either of these libraries at runtime won't be able to be used in an environment with RAPIDS libraries.

I'm happy to help drive this to completion if I can help in any way.

bdice · 2023-01-19T02:46:04Z

@kkraus14 Thanks, I didn’t fully recognize the severity. I’ll prioritize this first thing tomorrow.

robertmaynard · 2023-01-19T16:18:44Z

@kkraus14 Thanks, I didn’t fully recognize the severity. I’ll prioritize this first thing tomorrow.

To help keep conda and non conda builds in sync I have opened a rapids-cmake PR ( rapidsai/rapids-cmake#342 ) to update the spdlog version when not using conda.

…og-1.11.0

bdice · 2023-01-19T16:39:00Z

@kkraus14 This is ready for additional review. I'll run some tests with downstream packages to ensure cuml and others can build with these changes.

(edit: looks like you approved as I was writing this comment!)

ajschmidt8

Approving ops-codeowner file changes

bdice · 2023-02-03T19:03:21Z

@kkraus14 I’m leaning towards bringing in fmt explicitly in rapids-cmake. The different bundling approaches in conda packages and CPM fetches are not ideal. Thoughts @robertmaynard?

kkraus14 · 2023-02-06T23:08:27Z

@kkraus14 I’m leaning towards bringing in fmt explicitly in rapids-cmake. The different bundling approaches in conda packages and CPM fetches are not ideal. Thoughts @robertmaynard?

Opened a PR (rapidsai/rapids-cmake#364) regardless of the short term decision here. My plan is once that lands then RMM can be moved to using fmt explicitly instead of via the spdlog headers.

One thing that I'm not clear on is that the spdlog conda packages currently use the SPDLOG_FMT_EXTERNAL option of spdlog (https://github.com/gabime/spdlog/blob/v1.x/CMakeLists.txt#L88) which uses an external fmt library via the fmt::fmt target. There's also an option of SPDLOG_FMT_EXTERNAL_HO (https://github.com/gabime/spdlog/blob/v1.x/CMakeLists.txt#L89) which then uses a header only fmt via the fmt::fmt-header-only target. I presume the SPDLOG_FMT_EXTERNAL_HO option makes more sense for us in the context where we're fetching spdlog as opposed to finding it?

bdice · 2023-02-06T23:11:11Z

@kkraus14 Yes, that sounds fine: conda packages can use the fmt library (dependency of spdlog) and CPM fetches of spdlog can rely on a header-only fmt that is also fetched by rapids-cmake.

kkraus14 · 2023-02-06T23:19:57Z

@kkraus14 Yes, that sounds fine: conda packages can use the fmt library (dependency of spdlog) and CPM fetches of spdlog can rely on a header-only fmt that is also fetched by rapids-cmake.

Great, I'll tackle that in a follow up PR to rapidsai/rapids-cmake#364 once we bump the spdlog version to 1.11.0 since we'll need to change the rapids_cpm_spdlog logic a bit to find/fetch fmt first.

Adds fmt 9.1.0 to rapids-cmake via `rapids_cpm_fmt` based on discussion in rapidsai/rmm#1177. Nothing should be using `rapids_cpm_fmt` yet so we don't need to version align it with spdlog until spdlog is updated to `1.11.0`. Depends on #366 Authors: - Keith Kraus (https://github.com/kkraus14) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) URL: #364

kkraus14 · 2023-02-09T00:17:57Z

Next rapids-cmake PR that bumps spdlog to 1.11.0 and handles the fmt dependency is here: rapidsai/rapids-cmake#368

kkraus14 · 2023-02-10T17:50:39Z

@bdice the rapids-cmake PR should be good to go where this PR should probably be updated to use fmt explicitly instead of indirectly via spdlog. I'm happy to make these changes if you'd like, but I'd need you to give me the ability to write to your branch (since it seems I'm no longer considered a maintainer of this repo which is a bit disappointing but a separate issue to be discussed).

…te-spdlog-1.11.0

CMakeLists.txt

python/CMakeLists.txt

cmake/thirdparty/get_fmt.cmake

PR #1177 was merged a little too early when CI passed due to the presence of a `/merge` comment and sufficient approvals. This reverts a temporary change to the rapids-cmake repo that is no longer needed because rapidsai/rapids-cmake#368 has been merged. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Robert Maynard (https://github.com/robertmaynard) URL: #1209

This PR adds the `fmt` conda package to `rapids-build-env`. This change is similar to the changes introduced in rapidsai/rmm#1177 and should help resolve the `devel` image build errors that have been occurring lately (e.g. [these](https://gpuci.gpuopenanalytics.com/job/rapidsai/job/docker/job/rapidsai-core-devel/1888/BUILD_IMAGE=rapidsai%2Frapidsai-core-dev-nightly,CUDA_VER=11.8,FROM_IMAGE=gpuci%2Frapidsai,IMAGE_TYPE=devel,LINUX_VER=ubuntu22.04,PYTHON_VER=3.10,RAPIDS_VER=23.04/console)).

bdice added 2 commits December 7, 2022 14:28

Update to spdlog>=1.11.0, fmt>=9.1.0.

876cf91

Add comment to explain dependencies.

99bade8

github-actions bot added the conda label Dec 7, 2022

bdice requested a review from robertmaynard December 7, 2022 20:33

bdice added non-breaking Non-breaking change 5 - DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function labels Dec 7, 2022

bdice self-assigned this Dec 7, 2022

bdice added this to PR-WIP in v23.02 Release via automation Dec 7, 2022

robertmaynard approved these changes Dec 7, 2022

View reviewed changes

bdice marked this pull request as ready for review December 7, 2022 20:54

bdice requested a review from a team as a code owner December 7, 2022 20:54

harrism approved these changes Dec 7, 2022

View reviewed changes

bdice mentioned this pull request Dec 8, 2022

Revert "Upgrade to spdlog 1.10 (#1173)" #1176

Merged

3 tasks

kkraus14 reviewed Dec 8, 2022

View reviewed changes

dependencies.yaml Outdated Show resolved Hide resolved

dependencies.yaml Outdated Show resolved Hide resolved

conda/recipes/librmm/meta.yaml Show resolved Hide resolved

conda/recipes/librmm/meta.yaml Outdated Show resolved Hide resolved

bdice added 4 commits January 19, 2023 10:32

Merge remote-tracking branch 'upstream/branch-23.02' into update-spdl…

9a197ca

…og-1.11.0

Remove run pins (covered by run_exports of the package).

8d4d42a

Use a host section.

e5272cb

Add pins to conda_build_config.yaml.

bc66b58

kkraus14 approved these changes Jan 19, 2023

View reviewed changes

ajschmidt8 approved these changes Jan 19, 2023

View reviewed changes

v23.02 Release automation moved this from PR-WIP to PR-Reviewer approved Jan 19, 2023

kkraus14 mentioned this pull request Feb 6, 2023

Add fmt 9.1.0 rapidsai/rapids-cmake#364

Merged

7 tasks

Merge branch 'branch-23.02' into update-spdlog-1.11.0

8781eb1

bdice changed the base branch from branch-23.02 to branch-23.04 February 8, 2023 18:57

Merge branch 'branch-23.04' into update-spdlog-1.11.0

3534fd5

kkraus14 added 2 commits February 10, 2023 17:40

depend on fmt explicitly, temporarily use rapids-cmake fork for testing

06ec0f1

Merge branch 'update-spdlog-1.11.0' of github.com:bdice/rmm into upda…

b3dd596

…te-spdlog-1.11.0

kkraus14 requested a review from a team as a code owner February 10, 2023 22:41

github-actions bot added the CMake label Feb 10, 2023

kkraus14 reviewed Feb 10, 2023

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

temporarily point python cmake to fork as well

56fa4d4

github-actions bot added the Python Related to RMM Python API label Feb 10, 2023

kkraus14 reviewed Feb 10, 2023

View reviewed changes

python/CMakeLists.txt Show resolved Hide resolved

fix unused header, fix export_set name for fmt

dbd525a

kkraus14 reviewed Feb 10, 2023

View reviewed changes

cmake/thirdparty/get_fmt.cmake Outdated Show resolved Hide resolved

bdice added 2 commits February 13, 2023 11:51

Merge branch 'branch-23.04' into update-spdlog-1.11.0

2b70941

Don't export fmt manually.

bcf6e7c

rapids-bot bot merged commit db92493 into rapidsai:branch-23.04 Feb 13, 2023

bdice mentioned this pull request Feb 13, 2023

Revert changes overriding rapids-cmake repo. #1209

Merged

3 tasks

vyasr mentioned this pull request Feb 16, 2023

[BUG] librmm 22.10 and 22.12 conda packages ship spdlog as part of the package #1205

Closed

ajschmidt8 mentioned this pull request Feb 21, 2023

Add fmt and update spdlog in rapids-build-env rapidsai/integration#624

Merged

jameslamb mentioned this pull request Apr 22, 2024

WIP: Prevent path conflict in builds #1508

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to spdlog>=1.11.0, fmt>=9.1.0. #1177

Update to spdlog>=1.11.0, fmt>=9.1.0. #1177

bdice commented Dec 7, 2022 •

edited

Loading

codecov-commenter commented Dec 7, 2022 •

edited

Loading

kkraus14 commented Jan 12, 2023

ajschmidt8 commented Jan 12, 2023

bdice commented Jan 12, 2023

kkraus14 commented Jan 19, 2023

bdice commented Jan 19, 2023

robertmaynard commented Jan 19, 2023

bdice commented Jan 19, 2023 •

edited

Loading

ajschmidt8 left a comment

bdice commented Feb 3, 2023

kkraus14 commented Feb 6, 2023

bdice commented Feb 6, 2023

kkraus14 commented Feb 6, 2023

kkraus14 commented Feb 9, 2023

kkraus14 commented Feb 10, 2023

Update to spdlog>=1.11.0, fmt>=9.1.0. #1177

Update to spdlog>=1.11.0, fmt>=9.1.0. #1177

Conversation

bdice commented Dec 7, 2022 • edited Loading

Description

Checklist

codecov-commenter commented Dec 7, 2022 • edited Loading

Codecov Report

kkraus14 commented Jan 12, 2023

ajschmidt8 commented Jan 12, 2023

bdice commented Jan 12, 2023

kkraus14 commented Jan 19, 2023

bdice commented Jan 19, 2023

robertmaynard commented Jan 19, 2023

bdice commented Jan 19, 2023 • edited Loading

ajschmidt8 left a comment

Choose a reason for hiding this comment

bdice commented Feb 3, 2023

kkraus14 commented Feb 6, 2023

bdice commented Feb 6, 2023

kkraus14 commented Feb 6, 2023

kkraus14 commented Feb 9, 2023

kkraus14 commented Feb 10, 2023

bdice commented Dec 7, 2022 •

edited

Loading

codecov-commenter commented Dec 7, 2022 •

edited

Loading

bdice commented Jan 19, 2023 •

edited

Loading