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

Support ghc-9.10 #19

Merged
merged 6 commits into from
Aug 5, 2024
Merged

Support ghc-9.10 #19

merged 6 commits into from
Aug 5, 2024

Conversation

erikd
Copy link
Collaborator

@erikd erikd commented Jul 16, 2024

The unordered-containers and vector packages have been updated. Still waiting for containers (see haskell/containers#1012 ).

When containers is added, still need to add ghc-9.10 to the CI build matrix.

@erikd erikd marked this pull request as draft July 16, 2024 04:53
@erikd
Copy link
Collaborator Author

erikd commented Jul 29, 2024

@infinity0 The tag has been provided for containers and I have tried regenerating but ran into some issues building the tests which seem to be beyond my level of expertise.

@infinity0
Copy link
Member

Sorry, I don't have time to look very closely at the moment. The errors look related to missing instances, e.g. for vector:

tests/Tests/Move.hs:43:6: error: [GHC-39999]
    • No instance for ‘Arbitrary (V.Vector Int)’
        arising from a use of ‘testProperty’

I notice that the autoregen script changed the QuickCheck bound for these vector tests from >2.16 to >2.15, that will be because vector upstream made this change but it still seems weird, do you know why?

@infinity0
Copy link
Member

If vector moved their instances into a new file that is indirectly imported somehow, then it's possible ./regen.sh would have missed this file, without causing an obvious import error during the build. If this is correct, then ./regen.sh needs to be edited manually to include this new file here too.

@Benjamin-McRae-Tracsis
Copy link

Sorry if this isn't relevant, but in an attempt to help out I forked, cloned, and ran cabal test all -w ghc-9.10, and all the tests passed for me locally. I also ran cabal test all -w ghc-9.8.2 and I got green across the board. Am I doing something incorrectly?

@erikd
Copy link
Collaborator Author

erikd commented Aug 1, 2024

Sorry if this isn't relevant, but in an attempt to help out I forked, cloned, and ran cabal test all -w ghc-9.10

For me that does not work on the master branch, nor on my erikd/ghc-9.10 branch.

With ghc-9.10 on master you should at least be getting errors like:

src/Data/Strict/Map/Autogen/Internal.hs:4254:13: error: [GHC-87543]
    Ambiguous occurrence ‘foldl'’.
    It could refer to
       either ‘Prelude.foldl'’,
              imported from ‘Prelude’ at src/Data/Strict/Map/Autogen/Internal.hs:386:1-84
              (and originally defined in ‘ghc-internal-9.1001.0:GHC.Internal.Data.Foldable’),
           or ‘Data.Strict.Map.Autogen.Internal.foldl'’,
              defined at src/Data/Strict/Map/Autogen/Internal.hs:3247:1.
     |
4254 |   product = foldl' (*) 1
     |             ^^^^^^

which are fatal errors.

On my branch there are errors like:

    • Could not deduce ‘Model (Data.Strict.Vector.Vector (Int, a))
                        ~ [(Int, a)]’
        arising from a use of ‘testTuplyFunctions’
      from the context: (CommonContext a Data.Strict.Vector.Vector,
                         Ord a, Data a, Model (Data.Strict.Vector.Vector Int) ~ [Int],
                         EqTest (Data.Strict.Vector.Vector Int) ~ Property,
                         Model (Data.Strict.Vector.Vector (a, a)) ~ [(a, a)],
                         Model (Data.Strict.Vector.Vector (a, a, a)) ~ [(a, a, a)])
        bound by the type signature for:
                   testGeneralBoxedVector :: forall a.
                                             (CommonContext a Data.Strict.Vector.Vector, Ord a,
                                              Data a, Model (Data.Strict.Vector.Vector Int) ~ [Int],
                                              EqTest (Data.Strict.Vector.Vector Int) ~ Property,
                                              Model (Data.Strict.Vector.Vector (a, a)) ~ [(a, a)],
                                              Model (Data.Strict.Vector.Vector (a, a, a))
                                              ~ [(a, a, a)]) =>
                                             Data.Strict.Vector.Vector a -> [TestTree]
        at tests/Tests/Vector/Boxed.hs:(13,1)-(20,46)
    • In the expression: testTuplyFunctions
      In the second argument of ‘concatMap’, namely
        ‘[testSanity, inline testPolymorphicFunctions, testOrdFunctions,
          testTuplyFunctions, ....]’
      In the expression:
        concatMap
          ($ dummy)
          [testSanity, inline testPolymorphicFunctions, testOrdFunctions,
           testTuplyFunctions, ....]
    • Relevant bindings include
        dummy :: Data.Strict.Vector.Vector a
          (bound at tests/Tests/Vector/Boxed.hs:21:24)
        testGeneralBoxedVector :: Data.Strict.Vector.Vector a -> [TestTree]
          (bound at tests/Tests/Vector/Boxed.hs:21:1)
   |
26 |   , testTuplyFunctions

which is also a fatal error.

@erikd
Copy link
Collaborator Author

erikd commented Aug 1, 2024

If vector moved their instances into a new file that is indirectly imported somehow, then it's possible ./regen.sh would have missed this file, without causing an obvious import error during the build. If this is correct, then ./regen.sh needs to be edited manually to include this new file here too.

All the Arbitrary instances for vector seem to be in the file contrib/vector/vector/tests/Utilities.hs and that file gets regenerated to strict-containers/tests/Utilities.hs. These two files are identical and the Tests.Vector.Boxed module does import Utilities. This has got me stumped.

@erikd
Copy link
Collaborator Author

erikd commented Aug 1, 2024

Ok, I manually added the following to the Utilities module:

instance Arbitrary (DSV.Vector Int) where
    arbitrary = fmap DSV.fromList arbitrary

and some of the errors went away. I also added a CoArbitrary instance, but there are still errors.

@erikd
Copy link
Collaborator Author

erikd commented Aug 1, 2024

Ok, finally have this building and passing all tests locally).

@erikd
Copy link
Collaborator Author

erikd commented Aug 1, 2024

@infinity0 All the tests have passed. Is this ok to merge? Also, I can bump the cabal version numbers and upload to Hackage if you want me to (I am a Hackage Trustee).

@erikd erikd marked this pull request as ready for review August 1, 2024 05:57
@erikd erikd self-assigned this Aug 1, 2024
@Benjamin-McRae-Tracsis
Copy link

Benjamin-McRae-Tracsis commented Aug 1, 2024

EDIT: I realised it probably wasn't running all the tests so I tried running cabal-3.10.3.0 test strict-containers -w ghc-9.10 and it told me to add tests:true to my cabal.project.local. Upon doing so and recompiling, I get the missing instances errors.

Here's my example output, not that it matters if you've fixed things. I was running on this branch. I just get RUNNING.../PASS when using cabal-3.10.3.0.

$ git checkout
Your branch is up to date with 'upstream/erikd/ghc-9.10'.
$ cabal test all -w ghc-9.10
Warning: this is a debug build of cabal-install with assertions enabled.
Build profile: -w ghc-9.10.1 -O1
In order, the following will be built (use -v for more details):
 - strict-containers-tests-0.2 (test:strictness-tests) (first run)
Warning: this is a debug build of cabal-install with assertions enabled.
Preprocessing test suite 'strictness-tests' for strict-containers-tests-0.2...
Building test suite 'strictness-tests' for strict-containers-tests-0.2...
Warning: this is a debug build of cabal-install with assertions enabled.
Running 1 test suites...
Test suite strictness-tests: RUNNING...
toplevel
  HashMap singleton:  OK
  HashMap insert:     OK
  HashMap insert 2:   OK
  HashMap ix:         OK
  HashMap binary:     OK
  HashMap serialise:  OK
  IntMap singleton:   OK
  IntMap insert:      OK
  IntMap insert 2:    OK
  IntMap ix:          OK
  IntMap binary:      OK
  IntMap serialise:   OK
  Map singleton:      OK
  Map insert:         OK
  Map insert 2:       OK
  Map ix:             OK
  Map binary:         OK
  Map serialise:      OK
  Sequence singleton: OK
  Sequence insert:    OK
  Sequence insert 2:  OK
  Sequence ix:        OK
  Sequence binary:    OK
  Sequence serialise: OK
  Vector singleton:   OK
  Vector insert:      OK
  Vector insert 2:    OK
  Vector ix:          OK
  Vector binary:      OK
  Vector serialise:   OK

All 30 tests passed (0.00s)
Test suite strictness-tests: PASS
Test suite logged to:
/home/snip/strict-containers/dist-newstyle/build/x86_64-linux/ghc-9.10.1/strict-containers-tests-0.2/t/strictness-tests/test/strict-containers-tests-0.2-strictness-tests.log
1 of 1 test suites (1 of 1 test cases) passed.

@infinity0
Copy link
Member

@infinity0 All the tests have passed. Is this ok to merge? Also, I can bump the cabal version numbers and upload to Hackage if you want me to (I am a Hackage Trustee).

If you had to edit strict-containers/tests/Utilities.hs (or anything else) after running regen.sh, please make sure your edit also exists in the patch strict-containers/patches/tests.patch so that you don't have to do anything after running regen.sh.

As long as running regen.sh is the last step before building and testing, then it's OK to merge. Otherwise your changes will get lost the next time someone runs regen.sh.

@infinity0
Copy link
Member

infinity0 commented Aug 2, 2024

An easy way to figure out what to add to the patch files, is to re-run regen.sh then run git diff -R (reverse diff). If this is non-empty then those changes should be added to the patch files. After committing, run regen.sh again and then git diff -R should output empty with exit code 0.

@erikd
Copy link
Collaborator Author

erikd commented Aug 4, 2024

Wow, fiddling with these patches REALLY hurts my brain.

@erikd
Copy link
Collaborator Author

erikd commented Aug 4, 2024

FInally have it working! 🎉

Running regen.sh now produces a zero length diff.

@erikd erikd merged commit bf19956 into master Aug 5, 2024
11 checks passed
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