-
Notifications
You must be signed in to change notification settings - Fork 141
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
Fix benchmark build #196
Fix benchmark build #196
Conversation
… which currently fails with BenchAll.hs:173:55: No instance for (Random Word8) arising from a use of `random' Possible fix: add an instance declaration for (Random Word8) In the second argument of `(.)', namely `random' In the second argument of `S.unfoldrN', namely `(Just . random)' In the second argument of `(.)', namely `S.unfoldrN 10000 (Just . random)'
blaze-builder-0.3.* is incompatible with GHC >= 8.4 With --allow-newer it fails to build with: /tmp/stack26342/blaze-builder-0.3.3.4/Blaze/ByteString/Builder/Internal/Types.hs:71:10: error: • No instance for (Semigroup Builder) arising from the superclasses of an instance declaration • In the instance declaration for ‘Monoid Builder’ | 71 | instance Monoid Builder where | ^^^^^^^^^^^^^^
At this point benchmarks build with GHC 7.2 up and including to 8.2. For GHC >= 8.4 there is no build plan due to incompatibility with bytestring/bench/bench-bytestring.cabal Lines 43 to 46 in 95fe6bd
I'm not yet sure how to best resolve that. |
bed3386
to
bc8cc8b
Compare
Lines 282 to 354 in 95fe6bd
#181 removes |
|
LGTM. Thanks! |
if impl(ghc >= 8.4) | ||
build-depends: blaze-builder >= 0.3 && < 0.5 | ||
else | ||
build-depends: blaze-builder == 0.3.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, it seemed suboptimal to disable so many benchmarks when building with GHC-8.4+. Instead, I have relaxed the version constraint on blaze-builder
when building with those GHC versions.
The comparison with blaze-builder
isn't going to be very useful when using GHC-8.4+, but it shouldn't hurt either.
@sjakobi whats needed to make this good to merge? lets catsup on the deb :) |
*debt |
@cartazio I believe this PR is ready to merge. |
Cool. |
Many thanks, @cartazio! :) |
Based on work by @hsyl20. (Thanks!)
Fixes #52.
TODO:
Fix deprecation warnings.