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

Pin fmt version at 8.0.1 #196

Closed
wants to merge 1 commit into from
Closed

Pin fmt version at 8.0.1 #196

wants to merge 1 commit into from

Conversation

wonglkd
Copy link
Contributor

@wonglkd wonglkd commented Feb 22, 2023

Change build script to pin fmt version to same as folly's to minimize future breaks.

Test Plan: Built successfully on a fresh clone of CacheLib. (Also had to change external_git_branch=dev for zstd to deal with the cmake/zstd issue in #194, but that should resolve when gets merged into release)

Context: OSS build broke since 3-5 Jan 2023, likely due to changes in folly. While switching to v9.1.0 or 9.0.0 fixes the issue at hand, it seems sensible to match folly, which specifies fmt v8.0.1: https://github.com/facebook/folly/blob/main/build/fbcode_builder/manifests/fmt

#62 @agordon: For the other packages, you'll notice we do use a specific git tag or branch… I notice fmt is an exception - not pinned to a specific git tag or revision - likely an omission that can be fixed.

Related CacheLib issues: #186, #189, #107, #97, #62
Possibly related CacheLib commit: 67cc11a

Last working (Jan 3): https://github.com/facebook/CacheLib/actions/runs/3826992478
First failed (Jan 5): https://github.com/facebook/CacheLib/actions/runs/3844002307/jobs/6546742348
Error: error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt

Change build script to pin fmt version at same version that folly uses to minimize future breaks.

_Context:_ OSS build broke between 3-5 Jan 2023, likely due to changes in folly. While switching to v9.1.0 or 9.0.0 fixes the issue at hand, it seems sensible to match folly, which specifies fmt v8.0.1: https://github.com/facebook/folly/blob/main/build/fbcode_builder/manifests/fmt

Changed it on a fresh clone of CacheLib and got it to build. (Also had to change `external_git_branch=dev` for zstd to deal with the cmake/zstd issue in facebook#194, but that should resolve when gets merged into release)

> facebook#62 @agordon: For the other packages, you'll notice we do use a specific git tag or branch… I notice `fmt` is an exception - not pinned to a specific git tag or revision - likely an omission that can be fixed.

Related CacheLib issues: facebook#186, facebook#189, facebook#107, facebook#97, facebook#62
Related CacheLib commit: 67cc11a

Last working (Jan 3): https://github.com/facebook/CacheLib/actions/runs/3826992478
First failed (Jan 5): https://github.com/facebook/CacheLib/actions/runs/3844002307/jobs/6546742348
```error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt```
@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 Feb 22, 2023
@facebook-github-bot
Copy link
Contributor

@jiayuebao has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jiayuebao merged this pull request in 6357906.

@wonglkd wonglkd mentioned this pull request Feb 28, 2023
4 tasks
@wonglkd wonglkd deleted the patch-1 branch March 2, 2023 01:21
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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants