mrkkrp / modern-uri

Modern library for working with URIs
Other
68 stars 18 forks source link

Fixing compilation with ghc-9.0-rc1 #35

Closed recursion-ninja closed 3 years ago

recursion-ninja commented 3 years ago

Fixes #34 .

mrkkrp commented 3 years ago

I'm going to keep this in pending state till 9.0 is officially released.

mithrandi commented 3 years ago

Ping! :smile:

sjakobi commented 3 years ago

Ping @mrkkrp! :)

sjakobi commented 3 years ago

Also, @recursion-ninja, please note the merge conflict.

recursion-ninja commented 3 years ago

@sjakobi The CI build logs have expired, so I'm not aware what the "merge conflict" (CI issue?) was. I have a fleeting recollection from many months past that it was not related to the code changes, but a spurious tooling issue. Perhaps @mrkkrp could restart the CI so we can identify and address any potential issues.

sjakobi commented 3 years ago

@recursion-ninja Thanks for the quick response. I was simply referring to GitHub listing modern-uri.cabal as a "conflicting file". A rebase should help.

recursion-ninja commented 3 years ago

@sjakobi, Ah I see. I didn't read the merge status closely enough.

mrkkrp commented 3 years ago

Hey @sjakobi and @recursion-ninja I'm not sure the ecosystem is ready for 9.0. Last time I tried there were dependencies that wouldn't build even for simpler packages like megaparsec, notoriously criterion's dependencies had problems. I do not know if the situation has changed. If it has, I'll go through all my packages to make sure they work, if not, I'll wait more.

sjakobi commented 3 years ago

Yeah, https://github.com/haskell-hvr/cassava/issues/191 is still a blocker for the benchmarks. Since users mostly rely on the library component, I think you could still make a compatible release.

Or you could consider switching to tasty-bench for the benchmarks…

recursion-ninja commented 3 years ago

@mrkkrp , @sjakobi While I agree that the Haskell ecosystem isn't holistically prepared for ghc-9.0.* (despite having a release candidate available for 6 months prior to release), I do think it's important for each package to push for compatibility as soon as possible, lest the ecosystem stagnate.

The following cabal invocation successfully builds, leading me to believe (perhaps wrongly) that this patch is future proofed for when the dependencies are updated to be compatible with ghc-9.0.*

cabal build --enable-benchmarks --enable-tests --with-compiler=ghc-9.0.1 --allow-newer=base,template-haskell
recursion-ninja commented 3 years ago

@mrkkrp , @sjakobi this PR looks mostly good to me.

I didn't add GHC-9.0.1 to the CI build matrix in this PR as it's not supported via GitHub actions yet. I was told that GHC-8.10.4, GHC-9.0.1, and Cabal-3.4.0.0 should be added to GitHub actions by the end of next week. It may be worth delaying merging the PR until next week to see if we can wrap in the expanded CI builds in the same commit.

sjakobi commented 3 years ago

Yeah, haskell-hvr/cassava#191 is still a blocker for the benchmarks.

cassava was made compatible with a Hackage revision. The benchmarks build fine now.

recursion-ninja commented 3 years ago

@mrkkrp I updated the CI as described above. I feel this is ready to merge.

mrkkrp commented 3 years ago

Will merge as soon as I get to it, it is in my immediate TODO list.

mrkkrp commented 3 years ago

Released 0.3.4.1.