Minor refactoring to add compatability with GHC 9.10 #319
Conversation
recursion-ninja
requested review from
dcoutts,
jorisdral and
mheinzel
as code owners
jorisdral
left a comment
jorisdral
left a comment
There was a problem hiding this comment.
First contribution! Nice. Some comments:
I'd expect that fewer allow-newer stanzas are required if we consider more recent versions of packages on Hackage and CHaP (a Cardano-specific Hackage instance fyi). There are probably already a few releases that support ghc-9.10.1 among them.
diff --git a/cabal.project b/cabal.project
index cf07834..e265d56 100644
--- a/cabal.project
+++ b/cabal.project
@@ -12,11 +12,11 @@ repository cardano-haskell-packages
index-state:
-- Bump this if you need newer packages from Hackage
- -- current date: crc32c-0.2.2
- , hackage.haskell.org 2024-06-05T04:25:30Z
+ -- current date: support ghc-9.10.1
+ , hackage.haskell.org 2024-08-01T00:00:00Z
-- Bump this if you need newer packages from CHaP
- -- current date: fs-api-0.2.0.1, fs-sim-0.2.1.1
- , cardano-haskell-packages 2023-11-30T09:59:24Z
+ -- current date: support ghc-9.10.1
+ , cardano-haskell-packages 2024-08-01T00:00:00Z
packages: .I'd also suggest this change to .github/workflows/haskell.yml to ensure our CI also builds/tests with ghc-9.10.1
diff --git a/.github/workflows/haskell.yml b/.github/workflows/haskell.yml
index f7f1045..41d94fd 100644
--- a/.github/workflows/haskell.yml
+++ b/.github/workflows/haskell.yml
@@ -22,7 +22,7 @@ jobs:
strategy:
fail-fast: false
matrix:
- ghc: ["9.2.8", "9.4.8", "9.6.4", "9.8.2"]
+ ghc: ["9.2.8", "9.4.8", "9.6.4", "9.8.2", "9.10.1"]
cabal: ["3.10.2.1"]
os: [ubuntu-latest, windows-latest, macOS-latest]
cabal-flags: [""]
@@ -36,6 +36,10 @@ jobs:
os: windows-latest
- ghc: "9.8.2"
os: macOS-latest
+ - ghc: "9.10.1"
+ os: windows-latest
+ - ghc: "9.10.1"
+ os: macOS-latest
include:
- ghc: "9.6.4"
cabal: "3.10.2.1"
phadej
left a comment
phadej
left a comment
There was a problem hiding this comment.
The comment applies to other occurrences of the import too
recursion-ninja
force-pushed
the
ghc-9.10
branch
from
98f196f to
8864224
Compare
recursion-ninja
force-pushed
the
ghc-9.10
branch
2 times, most recently
from
db6e3a7 to
ffc1777
Compare
There was a problem hiding this comment.
This is required after changing the pinned dependency hash of fs-api and fs-sim as there was a name change of the function in the upstream API.
There was a problem hiding this comment.
An explicit import list was required to avoid a name clash with locally defined writeTMVar.
There was a problem hiding this comment.
You can probably just remove the local one. It was only there because writeTMVar was missing from io-classes
recursion-ninja
left a comment
recursion-ninja
left a comment
There was a problem hiding this comment.
I think the PR looks ready to merge into main.
jorisdral
left a comment
jorisdral
left a comment
There was a problem hiding this comment.
LGTM. Let's address the two new comments I inculded, and then we can merge
There was a problem hiding this comment.
You can probably just remove the local one. It was only there because writeTMVar was missing from io-classes
recursion-ninja
force-pushed
the
ghc-9.10
branch
8 times, most recently
from
93ac1ef to
e0a238a
Compare
dcoutts
left a comment
dcoutts
left a comment
There was a problem hiding this comment.
LGTM. Make sure the "merge branch main" patch gets squashed into the main patch, before the PR is merged.
recursion-ninja
force-pushed
the
ghc-9.10
branch
4 times, most recently
from
a027326 to
b0fda3d
Compare
recursion-ninja
force-pushed
the
ghc-9.10
branch
from
b0fda3d to
84e5303
Compare
5 participants
On my machine,
ghc-9.10is the default compiler. Hence my first attempt at build the project failed... but barely. I notice that the project could easily build with the newest version of GHC with very minor changes to disambiguate some calls tofoldl'. This PR adds future-proofing compatibility with GHC 9.10.