Fix for GHC 9.2 / template-haskell-2.18 by brandon-leapyear · Pull Request #1338 · yesodweb/persistent

brandon-leapyear · 2021-11-15T17:32:39Z

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

Forked off of #1335 to fix Database.Persist.TH errors and CI errors. Feel free to merge this back into the other PR

brandon-leapyear · 2021-11-15T20:33:45Z

.github/workflows/haskell.yml

+          # When the following PRs are merged, add a source-repository-package
+          # stanza in cabal.project for them.
+          #   * https://github.com/yesodweb/shakespeare/pull/260
+          #   * https://github.com/DanBurton/haskell-src-meta/pull/23
+          #   * https://github.com/haskell-foundation/foundation/pull/555
+          #   * https://github.com/vincenthz/hs-memory/pull/87
+          #   * https://github.com/haskell-crypto/cryptonite/pull/354
+          # - "9.2"


Cabal doesn't support this like Stack does: haskell/cabal#7821. I don't want to specify the forked repos in cabal.project, though, because when these PRs are merged, the forked repos could be deleted

I've tested this manually with stack build --test --no-run-tests --bench --no-run-benchmarks and it builds

hololeap · 2021-11-29T01:41:22Z

I'm getting a test failure on this branch with ghc-9.2.1 and aeson-2.0.2.0:

Failures:

  test/Database/Persist/ClassSpec.hs:14:21:
  1) Database.Persist.Class.PersistField.UTCTime fromPersistValue with format
       expected: Right 2018-02-27 10:49:42.123 UTC
        but got: Right 2018-02-27 10:49:42 UTC

  To rerun use: --match "/Database/Persist/Class/PersistField/UTCTime/fromPersistValue with format/"

Randomized with seed 947093686

brandon-leapyear · 2021-11-29T02:20:38Z

✨ _**This is an old work account. Please reference @brandonchinn178 for all future communication**_ ✨

…

--- Is it reproducible on master? Doesnt look related to these changes

On Sun, Nov 28, 2021, 5:41 PM hololeap ***@***.***> wrote: I'm getting a test failure on this branch with ghc-9.2.1 and aeson-2.0.2.0 : Failures: test/Database/Persist/ClassSpec.hs:14:21: 1) Database.Persist.Class.PersistField.UTCTime fromPersistValue with format expected: Right 2018-02-27 10:49:42.123 UTC but got: Right 2018-02-27 10:49:42 UTC To rerun use: --match "/Database/Persist/Class/PersistField/UTCTime/fromPersistValue with format/" Randomized with seed 947093686 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1338 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGUC75IK62CYLTCTLE5SR4DUOLK43ANCNFSM5ICH2UOA> .

hololeap · 2021-11-29T08:20:12Z

It's hard to know because master will not build with ghc-9.2 and aeson-2. I can say that master builds fine with ghc-8.10.6 and aeson-1.5.6.0 and all the tests pass. I am not necessarily saying that the changes made in this PR are the source of this failing test, but I wanted to point it out.

brandon-leapyear · 2021-11-29T18:23:08Z

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

Seems like ghc-9.2.1 is bundled with time-1.11.1.1, which changed the Read UTCTime instance.

>>> reads "2018-02-27 10:49:42.123 UTC" :: [(UTCTime, String)]
[(2018-02-27 10:49:42 UTC,"123"),(2018-02-27 10:49:42.1 UTC,"23"),(2018-02-27 10:49:42.12 UTC,"3"),(2018-02-27 10:49:42.123 UTC,"")]

Updated the code and tested that the test now passes.

andreasabel · 2021-12-17T08:13:54Z

persistent/Database/Persist/Class/PersistField.hs

                case parse8601 s <|> parsePretty s of
                    Nothing -> Left $ fromPersistValueParseError "UTCTime" x
                    Just x' -> Right x'
+            matches -> Right $ fst $ last matches


Leafing through this PR, this was the only change that stuck out as not obvious.

Why switch from first match to last match?

Could this application of last be made statically error-free by using Data.List.NonEmpty.last?

I'd say a comment in the code, justifying the choice of last match, would be in order.

Yeah, I mentioned this in the commit message, but the new time package bundled with GHC 9.2 changed the Read UTCTime instance to be more forgiving and provide partial matches.

-- old time >>> reads "2018-02-27 10:49:42.12 UTC" :: [(UTCTime, String)] [(2018-02-27 10:49:42.12 UTC,"")] -- new time >>> reads "2018-02-27 10:49:42.12" :: [(UTCTime, String)] [(2018-02-27 10:49:42 UTC,"12"),(2018-02-27 10:49:42.1 UTC,"2"),(2018-02-27 10:49:42.12 UTC,"")]

I'll add a comment and use NonEmpty

bergmark · 2022-01-21T09:10:24Z

This can be rebased now since the aeson 2 changes were merged separately.

brandon-leapyear · 2022-01-21T17:07:40Z

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

@bergmark thanks! Done

stack.yaml

parsonsmatt · 2022-01-22T00:01:21Z

stack.yaml

+  - github: haskell-foundation/foundation
+    commit: 0bb195e1fea06d144dafc5af9a0ff79af0a5f4a0
+    subdirs:
+      - basement
+  # https://github.com/vincenthz/hs-memory/pull/87
+  - github: vincenthz/hs-memory
+    commit: 3cf661a8a9a8ac028df77daa88e8d65c55a3347a
+  # https://github.com/haskell-crypto/cryptonite/pull/354
+  - github: haskell-crypto/cryptonite
+    commit: 3b081e3ad027b0550fc87f171dffecbb20dedafe


This is unfortunate. I wonder if we can drop these dependencies...

It seems like commenting out persistent-redis and persistent-mongoDB from stack.yaml builds without these dependencies, but I can't figure out how those libraries use these libraries

using stack dot --external and grepping the file, looks like persistent-mongoDB depends on mongoDB depends on bson depends on cryptohash-md5 depends on crpyothash depends on cryptonite.

Ah and persistent-redis depends on hedis which depends on tls which depends on cryptonite

Maybe those two should be broken out into a separate repo?

Oh, no, it's mongoDB -> cryptohash -> cryptonite. cryptohash-md5 appears to be factored out to avoid that dependency entirely, but mongoDB appears to use hmac and sha1 as well

There is cryptohash-sha1. And there is crypto-api which is maintained by @TomMD , I think.

parsonsmatt · 2022-03-14T18:54:46Z

Closing from #1367 - thanks for the contribution! ❤️

brandon-leapyear mentioned this pull request Nov 15, 2021

Support aeson-2 and template-haskell-2.18 (ghc-9.2.1) #1335

Closed

brandon-leapyear commented Nov 15, 2021

View reviewed changes

bergmark mentioned this pull request Nov 28, 2021

Initial GHC 9.2 bounds commercialhaskell/stackage#6318

Merged

andreasabel reviewed Dec 17, 2021

View reviewed changes

bergmark mentioned this pull request Jan 15, 2022

aeson 2.0 support (only) #1351

Merged

Remove redundant comment

5833d96

brandon-leapyear changed the title ~~Fix for aeson-2 and template-haskell-2.18~~ Fix for GHC 9.2 / template-haskell-2.18 Jan 21, 2022

bergmark reviewed Jan 21, 2022

View reviewed changes

stack.yaml Outdated Show resolved Hide resolved

brandon-leapyear and others added 6 commits January 21, 2022 11:17

Add GHC 9.2 support

8060493

Fix for GHC 9.2

1e16884

Fix for template-haskell-2.18

ea5433d

Add comment in CI for 9.2

53f81d7

[refactor] Reduce code in CPP conditional

622f034

Fix for new 'Read UTCTime' instance

ef4d8a2

parsonsmatt reviewed Jan 22, 2022

View reviewed changes

This was referenced Feb 7, 2022

GHC 9.2 support NorfairKing/sydtest#35

Closed

Add GHC 9.2 support NorfairKing/validity#100

Closed

ysangkok mentioned this pull request Mar 9, 2022

Allow GHC 9.2 for compatible packages #1366

Merged

parsonsmatt closed this Mar 14, 2022

brandon-leapyear deleted the chinn/aeson2 branch March 14, 2022 19:00

Conversation

brandon-leapyear commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hololeap commented Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandon-leapyear commented Nov 29, 2021 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hololeap commented Nov 29, 2021

Uh oh!

brandon-leapyear commented Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreasabel Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergmark commented Jan 21, 2022

Uh oh!

brandon-leapyear commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandon-leapyear Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parsonsmatt commented Mar 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

brandon-leapyear commented Nov 15, 2021 •

edited

Loading

hololeap commented Nov 29, 2021 •

edited

Loading

brandon-leapyear commented Nov 29, 2021 via email •

edited

Loading

brandon-leapyear commented Nov 29, 2021 •

edited

Loading

andreasabel Dec 17, 2021 •

edited

Loading

brandon-leapyear commented Jan 21, 2022 •

edited

Loading

brandon-leapyear Jan 22, 2022 •

edited

Loading