haskell / text-icu

This package provides the Haskell Data.Text.ICU library, for performing complex manipulation of Unicode text.
BSD 2-Clause "Simplified" License
47 stars 41 forks source link

v0.8.0.1: relax bound on `deepseq` to the one shipped with GHC 8.0 #72

Closed andreasabel closed 2 years ago

andreasabel commented 2 years ago

The current lower bound deepseq >= 1.4.3 for v0.8.0.1 is needlessly restrictive and excludes building with GHC 8.0 in some cases, e.g.: https://github.com/agda/agda/runs/5495421153?check_suite_focus=true

Agda-2.6.3$ cabal build -w ghc-8.0.2 -f +enable-cluster-counting
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
...
[__1] trying: Agda:+enable-cluster-counting
[__2] trying: text-icu-0.8.0.1 (dependency of Agda +enable-cluster-counting)
[__3] trying: template-haskell-2.11.1.0/installed-2.11.1.0 (dependency of
Agda)
[__4] next goal: pretty (dependency of Agda)
[__4] rejecting: pretty-1.1.3.3/installed-1.1.3.3 (conflict: text-icu =>
deepseq>=1.4.3.0, pretty => deepseq==1.4.2.0/installed-1.4.2.0)
[__4] rejecting: pretty-1.1.3.6, ..., pretty-1.0.0.0 (conflict: template-haskell =>
pretty==1.1.3.3/installed-1.1.3.3)
[__4] fail (backjumping, conflict set: Agda, pretty, template-haskell, text-icu)

text-icu-0.8.0.1 builds fine with deepseq-1.4.2 (shipped with GHC 8.0):

$ cabal install -w ghc-8.0.2 text-icu-0.8.0.1 --constraint='deepseq==1.4.2.*' --allow-older=text-icu:deepseq
...
[38 of 38] Compiling Data.Text.ICU    ( Data/Text/ICU.hs, dist/build/Data/Text/ICU.o )
ld: warning: directory not found for option '-L/opt/homebrew/opt/icu4c/lib'
Installing library in ...

So the lower bound should be relaxed...

vshabanov commented 2 years ago

Tests won't build with deepseq-1.4.2 since it doesn't have NFData Ordering instance. But #34 could be merged if we really need GHC 8.0.

andreasabel commented 2 years ago

On master (c73d7fe6f29e178d3ea40160e904ab39236e3c9d), I did have success with deepseq-1.4.2:

$ cabal test -w ghc-8.0.2 --constraint='deepseq==1.4.2.*' --allow-older=text-icu:deepseq
$ cabal-3.6 test -w ghc-7.10.3 --constraint='deepseq==1.4.2.*' --allow-older=text-icu:deepseq

But not with -1.4.1:

$ cabal test -w ghc-7.10.3 --constraint='deepseq==1.4.1.*' --allow-older=text-icu:deepseq
...
[34 of 38] Compiling Data.Text.ICU.Calendar ( /Users/abel/bin/src/text-icu/dist-newstyle/build/x86_64-osx/ghc-7.10.3/text-icu-0.8.0/build/Data/Text/ICU/Calendar.hs, /Users/abel/bin/src/text-icu/dist-newstyle/build/x86_64-osx/ghc-7.10.3/text-icu-0.8.0/build/Data/Text/ICU/Calendar.o )

Data/Text/ICU/Calendar.hsc:546:14:
    No instance for (Show Clock.UTCTime) arising from a use of ‘show’
    In the expression: show (utcTime cal)
    In an equation for ‘show’: show cal = show (utcTime cal)
    In the instance declaration for ‘Show Calendar’

Also, if needed one could have a more restrictive bound for the testsuites.

Given the experiments, I propose to revise text-icu-0.8.0.1 on hackage to deepseq >= 1.4.2. What do you think @vshabanov ?

P.S.: If it makes you sleep better, just revise the bound for the library, but keep the stricter for the test-suite.

P.P.S.: The same experiments succeed on the released version (cabal get text-icu-0.8.0.1).

andreasabel commented 2 years ago

I made a revision of just the library deps, to allow deepseq >= 1.4.2: https://hackage.haskell.org/package/text-icu-0.8.0.1/revisions/ According to my experiments, it might also be sound for the test-suite, but I don't need the relaxation there.

vshabanov commented 2 years ago

Perhaps NFData Ordering instance is no longer necessary. I've relaxed lower bounds in https://github.com/haskell/text-icu/commit/e972f7c94c6569857da95be24825eb7b89f0fab4 (accidentally linked it to #71 instead of #72).

andreasabel commented 2 years ago

Ok, I made the lower bound of the test-suite conform to that of the library---by removing the bound. Published revision 2 on hackage.

vshabanov commented 2 years ago

Did the same in sources for future releases.