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

[c++] update to fmt 11.1.2, fast_double_parser 0.8.0 #6802

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jan 27, 2025

Contributes to #3763.
Contributes to #5691.

Proposes updated the projects vendored in here as git submodules to their latest versions:

How I did that

# fast_double_parser
cd ./external_libs/fast_double_parser
git fetch origin --tags
git checkout v0.8.0
cd ../../

# fmt
cd ./external_libs/fmt
git fetch origin 11.1.2
git checkout 11.1.2
cd ../../

# commit update
git add ./external_libs

Benefits of this change

Pulls in bugfixes, security patches, performance improvements.

The last update like this was #6074, over a year ago.

@jameslamb
Copy link
Collaborator Author

jameslamb commented Jan 27, 2025

The only compilation failure left is this one on Appveyor:

c:\projects\lightgbm\lightgbm-python\external_libs\fmt\include\fmt\base.h(743): error C2338: (compiling source file
C:\projects\lightgbm\lightgbm-python\src\io\tree.cpp) [C:\Users\appveyor\tmp\tmpaktrbpjy\build\lightgbm_objs.vcxproj]
c:\projects\lightgbm\lightgbm-python\external_libs\fmt\include\fmt\base.h(743): error C2327:
'fmt::v11::basic_specs::data_': is not a type name, static, or enumerator (compiling source file
C:\projects\lightgbm\lightgbm-python\src\metric\dcg_calculator.cpp

(build link)

That code was JUST introduced into fmt 2 weeks ago: fmtlib/fmt@01914f0

Note that that is failing with MSVC 2015... other builds with later MSVC versions do not have this compilation error, for example this one with Visual Studio 2019: https://github.com/microsoft/LightGBM/actions/runs/12981353281/job/36199682684?pr=6802

I'm going to try rolling back one minor version to fmt 11.1.2 to see if that's enough to avoid it. Then maybe separately try to file a bug report with fmt and see if they'd be willing to support such an old version of MSVC.

@jameslamb jameslamb changed the title update to fmt 11.1.3, fast_double_parser 0.8.0 update to fmt 11.1.2, fast_double_parser 0.8.0 Jan 27, 2025
@jameslamb jameslamb marked this pull request as ready for review January 27, 2025 05:54
@jameslamb jameslamb changed the title update to fmt 11.1.2, fast_double_parser 0.8.0 [c++] update to fmt 11.1.2, fast_double_parser 0.8.0 Jan 27, 2025
@jameslamb
Copy link
Collaborator Author

/gha run r-valgrind

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants