Support GHC 9.8.1 #1310

larskuhtz · 2023-10-16T23:14:15Z

Support building with 9.8.1
update pact-json pin (to match code version used in chainweb-node)
update sbv pin to support GHC 9.8.1 (turn ignore -Wx-partial warnings)

emilypi · 2023-10-18T19:27:21Z

CI seems to be broken with that last one @larskuhtz

larskuhtz · 2023-10-19T17:24:01Z

Yeah, not sure why the nix builds fail. The failure message is a bit cryptic. Some nix expert has to take a look.

This reverts commit a95e012.

larskuhtz · 2023-10-19T17:42:23Z

The nix m1 build now complains about x-partial (which comes as a surprise, because AFAIK this PR did not upgrade the nix build to use 9.8.). Also not sure about the other nix builds. The failure message are not meaningful (to me).

larskuhtz · 2023-10-19T17:43:08Z

I also reverted webauthn 0.8 support from the PR, since webauthn it is now vendored.

larskuhtz · 2023-10-19T20:00:29Z

@imalsogreg can you take another look at the nix builds? I don't know how to fix them.

0xd34df00d · 2023-10-19T23:06:08Z

src-tool/Pact/Analyze/Translate.hs

+              _:xs -> xs
+              [] -> error "Expected nonempty list"


I'm curious if this can be changed to avoid the preceding null protoSteps check, and returning an empty list in this branch instead of erroring out? That is, smth like

(case reverse protoSteps of [] -> [] _:xs -> xs)

0xd34df00d · 2023-10-19T23:07:27Z

src/Pact/Eval.hs

-    let i = if null vs then info else _tInfo $ view _1 $ head vs
+  CyclicSCC [] -> error "Expected nonempty list"
+  CyclicSCC vs@(v:_) -> do
+    let i = if null vs then info else _tInfo $ view _1 $ v


null vs shall always be False here, right? Also, I think you can drop this last $ before v.

Yeah, need to drop the if null if the above change is to make it in.

0xd34df00d · 2023-10-19T23:08:54Z

src/Pact/Eval.hs

@@ -659,8 +659,9 @@ enforceAcyclic
  -> Eval e [(Term (Either l r), key, [key])]
 enforceAcyclic info cs = forM cs $ \c -> case c of
  AcyclicSCC v -> return v
-  CyclicSCC vs -> do
-    let i = if null vs then info else _tInfo $ view _1 $ head vs
+  CyclicSCC [] -> error "Expected nonempty list"


The previous impl proceeded as usual binding i to info in this case if my brain is symbolic-executing the code right. Although I'm not sure if that makes sense given the semantics of this branch.

evalError or something else here, even if unreachable, just in case.

0xd34df00d · 2023-10-19T23:12:00Z

src/Pact/PersistPactDb/Regression.hs

-    [TxLog "USER_user1" "key1" row,
-     TxLog "USER_user1" "key1" row'] $
-    _getTxLog pactdb usert (head tids) v
+  case tids of


Shall we still check assertEquals "user txids" [1] tids?

0xd34df00d · 2023-10-19T23:13:47Z

src/Pact/Typechecker.hs

@@ -1204,7 +1205,10 @@ showFails = do

 -- | unsafe lens for using `typecheckBody` with const
 singLens :: Iso' a [a]
-singLens = iso pure head
+singLens = iso pure (\case


I'd add HasCallStack constraint to this function to get better traces where it happened for debuggability (unless it's a hot spot, though I doubt it is).

edmundnoble · 2023-10-20T19:11:33Z

cabal.project

+allow-newer: *:deepseq
+allow-newer: *:pretty
+allow-newer: *:text
+allow-newer: *:bytestring

 -- Patch merged into master (upcoming verison 10.0). We are currently using 9.2


s/verison/version

edmundnoble · 2023-10-20T19:12:07Z

flake.nix

@@ -25,16 +25,16 @@
          pact =
            final.haskell-nix.project' {
              src = ./.;
-              compiler-nix-name = "ghc962";
+              compiler-nix-name = "ghc981";


This seems to be why this PR is making nix use 9.8.

@larskuhtz Were we aiming to use ghc 9.8.1 in this PR? I thought so, but let me check my assumption.

On the cabal build side, with PR there are builds for both, ghc-9.6 and ghc-9.8.

Not sure what we want on the nix side. Personally, I'd say that in case of doubt we should prefer the latest.

jmcardon · 2023-11-06T17:31:25Z

src/Pact/Eval.hs

@@ -659,8 +659,9 @@ enforceAcyclic
  -> Eval e [(Term (Either l r), key, [key])]
 enforceAcyclic info cs = forM cs $ \c -> case c of
  AcyclicSCC v -> return v
-  CyclicSCC vs -> do
-    let i = if null vs then info else _tInfo $ view _1 $ head vs
+  CyclicSCC [] -> error "Expected nonempty list"


evalError or something else here, even if unreachable, just in case.

jmcardon · 2023-11-08T21:22:14Z

src/Pact/Eval.hs

-    let i = if null vs then info else _tInfo $ view _1 $ head vs
+  CyclicSCC [] -> error "Expected nonempty list"
+  CyclicSCC vs@(v:_) -> do
+    let i = if null vs then info else _tInfo $ view _1 $ v


Yeah, need to drop the if null if the above change is to make it in.

jmcardon · 2023-11-08T21:23:06Z

src/Pact/PersistPactDb/Regression.hs

+      [TxLog "USER_user1" "key1" row,
+      TxLog "USER_user1" "key1" row'] $
+      _getTxLog pactdb usert tid v
+   _ -> error "Expected nonempty list of tids"


Isn't there an assertFailure or something like this instead of error here?

larskuhtz added 2 commits October 16, 2023 16:01

support building with GHC-9.8.1 and aeson-2.2

fea7014

support webauthn 0.8

a95e012

larskuhtz requested review from jwiegley, emilypi and jmcardon as code owners October 16, 2023 23:14

larskuhtz changed the title ~~Lars/ghc 9.8.1~~ GHC 9.8.1 and webauth-0.8 Oct 16, 2023

larskuhtz added 2 commits October 16, 2023 18:14

add GHC-9.8 CI build

34d5b26

update github haskell setup action

17f5450

larskuhtz force-pushed the lars/ghc-9.8.1 branch from 1003ba8 to 17f5450 Compare October 17, 2023 01:14

Disable x-partial warning for GHC 9.8.1

0fb535e

bump nixpkgs to the version used by haskellNix

12ec9a2

larskuhtz added 2 commits October 19, 2023 10:25

Merge branch 'master' into lars/ghc-9.8.1

b7f91c0

Revert "support webauthn 0.8"

c048f90

This reverts commit a95e012.

larskuhtz changed the title ~~GHC 9.8.1 and webauth-0.8~~ GHC 9.8.1 Oct 19, 2023

larskuhtz changed the title ~~GHC 9.8.1~~ Support GHC 9.8.1 Oct 19, 2023

emilypi approved these changes Oct 19, 2023

View reviewed changes

imalsogreg and others added 3 commits October 19, 2023 13:56

Tell nix to ignore partial function warnings

99a3e06

fix cabal.project

7f26658

Remove partial functions

0dee20c

imalsogreg requested a review from sirlensalot as a code owner October 19, 2023 22:52

0xd34df00d reviewed Oct 19, 2023

View reviewed changes

whitespace to test CI rebuild time

b889e82

edmundnoble reviewed Oct 20, 2023

View reviewed changes

jmcardon reviewed Nov 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support GHC 9.8.1 #1310

Support GHC 9.8.1 #1310

larskuhtz commented Oct 16, 2023 •

edited

Loading

emilypi commented Oct 18, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

0xd34df00d Oct 19, 2023

0xd34df00d Oct 19, 2023

jmcardon Nov 8, 2023

0xd34df00d Oct 19, 2023

jmcardon Nov 6, 2023

0xd34df00d Oct 19, 2023

0xd34df00d Oct 19, 2023

edmundnoble Oct 20, 2023

edmundnoble Oct 20, 2023

imalsogreg Oct 20, 2023

larskuhtz Oct 20, 2023

jmcardon Nov 6, 2023

jmcardon Nov 8, 2023

jmcardon Nov 8, 2023

Support GHC 9.8.1 #1310

Are you sure you want to change the base?

Support GHC 9.8.1 #1310

Conversation

larskuhtz commented Oct 16, 2023 • edited Loading

emilypi commented Oct 18, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

larskuhtz commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larskuhtz commented Oct 16, 2023 •

edited

Loading