Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for Aeson-2.2.3 #252

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

tonyalaribe
Copy link

No description provided.

stack.yaml Outdated Show resolved Hide resolved
@tonyalaribe
Copy link
Author

tonyalaribe commented Aug 1, 2024 via email

@j6carey
Copy link
Collaborator

j6carey commented Aug 1, 2024

Yeah, exactly. I disabled this just so I could get the branch to compile. I will delete it from the PR now.

There's some hope that as the toolchain evolves we will be able to support dhall once again for GHC 9.8.

@j6carey
Copy link
Collaborator

j6carey commented Aug 1, 2024

@tonyalaribe , we are investigating the mysterious CI failure.

@@ -103,6 +103,7 @@ library
build-depends: aeson >= 1.1.1.0 && < 2.2,
aeson-pretty,
attoparsec >= 0.13.0.1,
attoparsec-aeson,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is attoparsec-aeson used for? If we absolutely must depend on attoparsec-aeson then can we have bounds added?

TristanCacqueray added a commit to TristanCacqueray/proto3-suite that referenced this pull request Jan 7, 2025
@j6carey
Copy link
Collaborator

j6carey commented Jan 17, 2025

@tonyalaribe , CI seems to be working again.

Copy link
Collaborator

@riz0id riz0id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add bounds on the attoparsec-aeson package and we can merge this.

@tonyalaribe
Copy link
Author

Add bounds on the attoparsec-aeson package and we can merge this.

Thanks! I've pushed an update with the bounds

@tonyalaribe tonyalaribe marked this pull request as ready for review January 17, 2025 17:00
@j6carey
Copy link
Collaborator

j6carey commented Jan 18, 2025

From the CI failures, my guess is that we will have to make the mention of attoparsec-aeson conditional.

@j6carey
Copy link
Collaborator

j6carey commented Jan 20, 2025

@tonyalaribe , could you merge branch "master" into your PR? There have been some recent changes to the Nix environment that might be relevant.

@tonyalaribe
Copy link
Author

@tonyalaribe , could you merge branch "master" into your PR? There have been some recent changes to the Nix environment that might be relevant.

@j6carey The branch is up to date with master

@tonyalaribe
Copy link
Author

I'm not so familiar with nix, so i'm having trouble debugging the issue with the CI

@j6carey
Copy link
Collaborator

j6carey commented Jan 20, 2025

@tonyalaribe , could you merge branch "master" into your PR? There have been some recent changes to the Nix environment that might be relevant.

@j6carey The branch is up to date with master

My mistake; when I checked for that I was looking at the wrong PR. Sorry for the confusion. I'll look into the build issues.

@j6carey
Copy link
Collaborator

j6carey commented Jan 22, 2025

@tonyalaribe , could you try the following changes to avoid the build failures for older package sets that lack attoparsec-aeson, and to start using the aeson version chosen by nixpkgs?:

git rm these files:

	deleted:    nix/packages/aeson.nix
	deleted:    nix/packages/attoparsec-aeson.nix
	deleted:    nix/patches/aeson-2.1.2.1.patch

Apply this patch:

diff --git a/nix/overlays/haskell-packages.nix b/nix/overlays/haskell-packages.nix
index 621b842..d33862a 100644
--- a/nix/overlays/haskell-packages.nix
+++ b/nix/overlays/haskell-packages.nix
@@ -34,9 +34,6 @@ in {
 
           # With nixpkgs-23.11 and ghc981, aeson-2.1.2.1 thinks that th-abstraction is out of bounds.
           #
-          # Also, in order to avoid the breaking change to package structure in aeson-2.2.0.0,
-          # we patch the import list of aeson-2.1.2.1.
-          #
           # And we disable tests because explicitly specifying aeson-2.1.2.1
           # seems to trigger a test failure, at least on GHC 9.4.8 and 9.8.1;
           # perhaps somewhere in nixpkgs the test is suppressed and
@@ -52,9 +49,7 @@ in {
           #
           aeson =
             pkgsNew.haskell.lib.doJailbreak
-              ( pkgsNew.haskell.lib.dontCheck
-                  ( pkgsNew.haskell.lib.appendPatches haskellPackagesOld.aeson
-                      [ ../patches/aeson-2.1.2.1.patch ] ) );
+              (pkgsNew.haskell.lib.dontCheck haskellPackagesOld.aeson);
 
           # With nixpkgs-23.11 and ghc981, atomic-write wants hspec for testing,
           # which causes problems.
diff --git a/proto3-suite.cabal b/proto3-suite.cabal
index 3e750c3..4a9651d 100644
--- a/proto3-suite.cabal
+++ b/proto3-suite.cabal
@@ -43,6 +43,12 @@ flag large-records
   Default:       True
   Manual:        True
 
+flag attoparsec-aeson
+  Description:   Depend upon the Haskell package "attoparsec-aeson".
+  Default:       True
+  Manual:        False
+    -- "Manual: False" to automatically skip "attoparsec-aeson" if it is not available
+
 source-repository head
   type:     git
   location: https://github.com/awakesecurity/proto3-suite
@@ -100,10 +106,9 @@ library
 
   other-modules:       Turtle.Compat
 
-  build-depends:       aeson >= 1.1.1.0 && < 2.2,
+  build-depends:       aeson >= 1.1.1.0,
                        aeson-pretty,
                        attoparsec >= 0.13.0.1,
-                       attoparsec-aeson >= 2.2.0.0 ,
                        base >=4.15 && <5.0,
                        base64-bytestring >= 1.0.0.1 && < 1.3,
                        binary >=0.8.3,
@@ -136,6 +141,8 @@ library
                        transformers >=0.4 && <0.7,
                        turtle < 1.6.0 || >= 1.6.1 && < 1.7.0,
                        vector >=0.11 &&  <0.14
+  if flag(attoparsec-aeson)
+    build-depends:     attoparsec-aeson >= 2.2.0.0
   hs-source-dirs:      src
   default-language:    Haskell2010
   ghc-options:         -O2 -Wall -Werror
@@ -195,7 +202,6 @@ test-suite tests
   build-depends:
       aeson >= 1.1.1.0
     , attoparsec >= 0.13.0.1
-    , attoparsec-aeson
     , base >=4.15 && <5.0
     , base64-bytestring >= 1.0.0.1 && < 1.3
     , bytestring >=0.10.6.0 && <0.13
@@ -223,6 +229,8 @@ test-suite tests
     , transformers >=0.4 && <0.7
     , turtle
     , vector >=0.11 && < 0.14
+  if flag(attoparsec-aeson)
+    build-depends:     attoparsec-aeson
 
   ghc-options:         -O2 -Wall
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants