Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fea/move to latest nvbench #417

Merged

Conversation

robertmaynard
Copy link
Contributor

Description

Move to latest version of nvbench which includes a fix for a CMake error when using static fmt with a conda env.

Checklist

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

@robertmaynard robertmaynard added bug Something isn't working non-breaking Introduces a non-breaking change 5 - DO NOT MERGE Hold off on merging; see PR for details and removed 5 - DO NOT MERGE Hold off on merging; see PR for details labels May 31, 2023
@robertmaynard robertmaynard marked this pull request as ready for review May 31, 2023 18:21
@robertmaynard robertmaynard requested a review from a team as a code owner May 31, 2023 18:21
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving up to a couple small comments that can be addressed however you see fit.

rapids_cpm_init()
rapids_cpm_nvbench()

file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/use_fmt.cpp" [=[
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a minimal repro, or can it be smaller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is about the smallest we can make the repro. It needs to do linking so that we verify that the changes dropped nvbench/public_fmt_dep_in_conda.diff aren't lost from upstream

@robertmaynard
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 77435a2 into rapidsai:branch-23.08 May 31, 2023
@robertmaynard robertmaynard deleted the fea/move_to_latest_nvbench branch May 31, 2023 19:32
robertmaynard added a commit to robertmaynard/cudf that referenced this pull request Jun 1, 2023
NVBench has integrated the `public_fmt_dep_in_conda` fixes. So for
23.08 rapids-cmake dropped the patch and moved to newest nvbench
rapidsai/rapids-cmake#417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants