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

Fmt folly update #10924

Closed
wants to merge 3 commits into from
Closed

Conversation

yingsu00
Copy link
Collaborator

@yingsu00 yingsu00 commented Sep 4, 2024

Following up the discussion in #10904
and depend on #10670

Use a common file to download and install external dependencies.
Extract versions for each library.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 4, 2024
Copy link

netlify bot commented Sep 4, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 57073ff
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66d7cc59665dab0008af0b5b

Recently brew updated fmt version from 10.1 to 11.0. This caused
build breaks. This commit contains 3 fixes:

1. Upgrade fmt to 11.0 from brew install. Remove the install_fmt call
to avoid mismatching versions.

2. Error "'this' argument to member function 'format' has type 'const
fmt::formatter<facebook::velox::exec::Spiller::Type>', but function
is not marked const".
The new fmt v11 forces the const check. To fix this error, the
"const" keyword was added to the `format()` function declarations.

3. Error "error: static assertion failed due to requirement
'formattable_char': Mixing character types is disallowed."
The new fmt v11 added a check if the arguments of fmt::make_format_args()
have mixing different character types (e.g., char vs wchar_t or
std::string vs folly::Range<const char*>). To fix this, we need to
convert the folly::StringPiece to string.

4. Update folly to "v2024.08.26.00", this is needed because the previous
version depends on fmt v10. To do this fast_float is added as one of
MACOS_VELOX_DEPS.

Note that if users see issues when building the new folly, they need to
make sure the CMake cache is cleaned and clean the build directory first.
@yingsu00
Copy link
Collaborator Author

yingsu00 commented Sep 5, 2024

closing for now

@yingsu00 yingsu00 closed this Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants