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

Update to more recent GHC #130

Open
kozross opened this issue Oct 27, 2019 · 22 comments
Open

Update to more recent GHC #130

kozross opened this issue Oct 27, 2019 · 22 comments

Comments

@kozross
Copy link

kozross commented Oct 27, 2019

Is it possible to have an update of the version bounds to allow this to build on GHC 8.8?

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

The currently-published version of lvish is lvish-1.1.4, which currently requires base >=4.6 && <4.9, so ghc-7.6 to ghc-7.10.

The github version of lvish, however, is much higher, lvish-2.0.2.1 requiring base >= 4.6, so no upper bound, great!

The next step would be to double-check that the code indeed builds on recent versions of ghc (as of this writing, the most recent version is ghc-9.4).

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

The project has multiple packages which depend on each other, and there is a stack.yaml file but no cabal.project file, so clearly stack build is the intended way to build this project. Unfortunately, that doesn't work:

$ stack build
Could not parse '/[...]/lvars/stack.yaml':
Aeson exception:
Error in $.packages[8]: failed to parse field 'packages': parsing Text failed, expected String, but encountered Object
See http://docs.haskellstack.org/en/stable/yaml_configuration/

As explained at that URL, the problem is that lvish is currently using stack's legacy syntax for specifying git dependencies. It should be straightforward to update stack.yaml to the more modern format.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

Having now fixed the legacy syntax in #131, the next step is to bump the lts from lts-6.13 (ghc-7.10.3) to something more recent. The most recent at the time of writing is lts-19.29 (ghc-9.0.2). Switching to that resolver and attempting to run stack build reveals which bounds need to be bumped:

$ stack build

Error: While constructing the build plan, the following exceptions were encountered:

In the dependencies for atomic-primops-0.8.0.4:
    base-4.15.1.0 from stack configuration does not match >=4.6.0.0 && <4.10  (latest matching version is 4.9.1.0)
needed due to ctrie-0.1.0.3 -> atomic-primops-0.8.0.4

In the dependencies for concurrent-skiplist-0.1.0.1:
    random-1.2.1.1 from stack configuration does not match >=1.0 && <1.2  (latest matching version is 1.1)
needed since concurrent-skiplist is a build target.

In the dependencies for par-collections-1.2:
    primitive-0.7.3.0 from stack configuration does not match >=0.6 && <0.7  (latest matching version is 0.6.4.0)
    vector-0.12.3.1 from stack configuration does not match >=0.10 && <0.12  (latest matching version is 0.11.0.0)
needed since par-collections is a build target.

In the dependencies for par-mergesort-1.0:
    vector-0.12.3.1 from stack configuration does not match >=0.10 && <0.12  (latest matching version is 0.11.0.0)
needed since par-mergesort is a build target.

In the dependencies for pcg-random-0.1.3.4:
    primitive-0.7.3.0 from stack configuration does not match >=0.4 && <0.7  (latest matching version is 0.6.4.0)
needed due to lvish-2.0.2.1 -> pcg-random-0.1.3.4

Some different approaches to resolving this:

  * Set 'allow-newer: true' in /home/gelisam/.stack/config.yaml to ignore all version constraints and build anyway.

  * Build requires unattainable version of base. Since base is a part of GHC, you most likely need to use a different
    GHC version with the matching base.

Plan construction failed.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

par-collections and par-mergesort are local packages, it's easy to bump their bounds.

concurrent-skiplist is a git submodule, so although the code lives in a different repo and thus fixing its bounds would require a separate PR, it's easy to test locally by bumping the local copy's bounds.

atomic-primops-0.8.0.4 and pcg-random-0.1.3.4 are package versions are added to stack.yaml's extra-deps section, either because those packages were not part of lts-6.13 or because lvish requires a different version from the one provided by that resolver. It is easier to pin a different version than to locally bump their bounds, so the next step is to check if more recent versions exist and if they were since added to stackage.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

lts-6.13 pins atomic-primops-0.8.0.4, so it was completely redundant for it to be listed in extra-deps in the first place. Let's remove it.

lts-6.13 did not include pcg-random, but more recent lts resolvers do include it, so let's also remove it.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

While we're at it, let's also remove thread-local-storage-0.1.0.3, which is also included in recent lts resolvers.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

Having done all that, stack build is now willing to try building the project. Alas, the chaselev-deque dependency doesn't build with this resolver:

$ stack build chaselev-deque
chaselev-deque> configure
chaselev-deque> Configuring chaselev-deque-0.5.0.5...
chaselev-deque> build
chaselev-deque> Preprocessing library for chaselev-deque-0.5.0.5..
chaselev-deque> Building library for chaselev-deque-0.5.0.5..
chaselev-deque> [1 of 3] Compiling Data.Concurrent.Deque.ChaseLev
chaselev-deque> 
chaselev-deque> /tmp/stack-5cd9b66364f46807/chaselev-deque-0.5.0.5/Data/Concurrent/Deque/ChaseLev.hs:43:44: error:
chaselev-deque>     Module ‘GHC.Prim’ does not export ‘unsafeCoerce#’
chaselev-deque>    |
chaselev-deque> 43 | import GHC.Prim (reallyUnsafePtrEquality#, unsafeCoerce#)
chaselev-deque>    |                                            ^^^^^^^^^^^^^

--  While building package chaselev-deque-0.5.0.5 (scroll up to its section to see the error) using:
      ~.stack/setup-exe-cache/x86_64-linux-tinfo6/Cabal-simple_mPHDZzAJ_3.4.1.0_ghc-9.0.2 --builddir=.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.4.1.0 build --ghc-options " -fdiagnostics-color=always"
    Process exited with code: ExitFailure 1

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

chaselev-deque is the git dependency which I moved from the packages section to the extra-deps section in #131, so it is pinned to a particular commit. This is not the most recent commit, so the next thing to try would be to bump the pin to a more recent commit.

edit: my mistake, the git dependency was tslogger, not chaselev-deque.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

Alas, same error with the latest commit.

edit: because I bumped tslogger instead of chaselev-deque.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

Strange, the resolver I am using pins ghc-prim-0.7.0, and ghc-prim-0.7.0 does export unsafeCoerce#! So why can't chaselev-deque find it?

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

I think unsafeCoerce# might be even more magic than MagicHash? The following standalone module also fails to import unsafeCoerce#:

{-# LANGUAGE MagicHash #-}
module Main where
import GHC.Prim (reallyUnsafePtrEquality#, unsafeCoerce#)

main
  :: IO ()
main = putStrLn "typechecks."

Since reallyUnsafePtrEquality# imports just fine, we know that the # is not the problem.

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

The source for GHC.Prim explains that the code from which is used in hackage's documentation is not the real GHC.Prim. So I guess the problem is that the documentation for GHC.Prim includes unsafeCoerce#, while the real GHC.Prim doesn't?

@gelisam
Copy link
Contributor

gelisam commented Oct 21, 2022

haskell/primitive#258 explains that unsafeCoerce# was moved out of GHC.Prim. It can now be imported from either GHC.Exts or Unsafe.Coerce. So supporting a new version of ghc needs more than a version bump, this is a real breaking change which requires a code change to chaselev-deque, to import unsafeCoerce# from a different module.

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

I made a mistake a few comments above (which I have now crossed out), but it doesn't affect the conclusion. chaselev-deque-0.5.0.5 is a hackage dependency, not a git dependency. chaselev-deque-0.5.0.5 is the most recent version as the time of writing, so it is still the case that supporting a new version of ghc requires a code change to chaselev-deque.

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

The required code change has already been merged in 2020. The changelog implies that this fix will be released in the version which follows chaselev-deq-0.5.0.4... but we're using chaselev-deq-0.5.0.5! What's going on?

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

I filed an issue requesting a new hackage release for a chaselev-deq-0.5.0.6 which does include the 2020 changes. Until then, we can pin a git commit instead of a hackage version.

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

Having done all this, I can finally build all the dependencies with lts-19.29 (ghc-9.0.2)... but lvish itself fails to build.

$ stack build
lvish        > build (lib)
lvish        > Preprocessing library for lvish-2.0.2.1..
lvish        > Building library for lvish-2.0.2.1..
lvish        > [ 1 of 28] Compiling Control.LVish.DeepFrz.Internal
lvish        > 
lvish        > /[...]/src/lvish/Control/LVish/DeepFrz/Internal.hs:18:19: error:
lvish        >     Module ‘GHC.Prim’ does not export ‘unsafeCoerce#’
lvish        >    |
lvish        > 18 | import GHC.Prim  (unsafeCoerce#)
lvish        >    |                   ^^^^^^^^^^^^^
[...]

Luckily, we are now well-acquainted with this error and its fix!

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

Alas, it is not the only build error:

$ stack build
lvish        > build (lib)
lvish        > Preprocessing library for lvish-2.0.2.1..
lvish        > Building library for lvish-2.0.2.1..
lvish        > [ 1 of 28] Compiling Control.LVish.DeepFrz.Internal
lvish        > 
lvish        > /[...]/src/lvish/Control/LVish/DeepFrz/Internal.hs:38:11: error:
lvish        >     • The default type signature for frz: a -> a
lvish        >       does not match its corresponding non-default type signature
lvish        >     • When checking the class method:
lvish        >         frz :: forall a. DeepFrz a => a -> FrzType a
lvish        >       In the class declaration for ‘DeepFrz’
lvish        >    |
lvish        > 38 |   default frz :: a -> a
lvish        >    |           ^^^
[...]

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

Luckily, the comment above the problematic line explains the intent well:

-- | While `frz` is not exported, users may opt-in to the `DeepFrz`
-- class for their datatypes and take advantage of the default instance.
-- Doing so REQUIRES that `type FrzType a = a`.
default frz :: a -> a
frz a = a

Apparently in ghc-7.10, defining an instance

class DeepFrz MyType1 where
  type FrzType a = MyType1

Would succeed because MyType1 -> FrzType MyType1 simplifies to MyType1 -> MyType1, whereas

class DeepFrz MyType2 where
  type FrzType a = String

Would fail because MyType2 -> FrzType MyType1 simplifies to MyType2 -> String, not MyType2 -> MyType2. In more recent versions of ghc, this no longer works. I am guessing that ghc used to checks that the default type signature matches the method type in the instance declarations, so the check was delayed until the definition of FrzType a was known, whereas ghc now performs the check in the class declaration. This catches mistakes earlier but breaks the lvish implementation.

One simple fix is to use this default type signature instead:

default frz :: DeepFrz a ~ a => a -> DeepFrz a

This time, the default type signature matches the method type, so this is accepted. Furthermore, at every instance where the default implementation is used, ghc checks that the extra constraints are satisfied, so default instances are only allowed if DeepFrz MyType1 is indeed defined to be MyType1.

@gelisam
Copy link
Contributor

gelisam commented Oct 22, 2022

After fixing a lot more unsafeCoerce# imports (why does lvish need so much unsafeness under the hood??), I now encounter yet another error:

lvish        > /[...]/src/lvish/Data/LVar/Internal/Pure.hs:124:21: error:
lvish        >     Not in scope: type constructor or class ‘JoinSemiLattice’

I am guessing that this comes from a library, whose version changed when I bumped the lts, and this new version has renamed JoinSemiLattice to something else.

I have already spent quite some time on this, so I now plan to take a break for at least a few days. If somebody wants to take over, I have pushed my changes so far to https://github.com/gelisam/lvars/tree/bump-lts. Note that this branch does not include the changes to the git submodules, so after fetching the git submodules using git submodule init && git submodule update, you'll have to apply the following patch:

diff --git a/concurrent-skiplist.cabal b/concurrent-skiplist.cabal
index 57df5b0..9cefd2f 100644
--- a/deps/concurrent-skiplist/concurrent-skiplist.cabal
+++ b/deps/concurrent-skiplist/concurrent-skiplist.cabal
@@ -32,7 +32,7 @@ library
   other-extensions:  NamedFieldPuns, BangPatterns, RankNTypes, ExistentialQuantification, GADTs,
                      ScopedTypeVariables, ParallelListComp, TypeSynonymInstances, FlexibleInstances
   build-depends:  base   >=4.6,
-                  random >=1.0 && <1.2,
+                  random >=1.0 && <1.3,
                   ghc-prim >= 0.3,
                   transformers >=0.3,
                   atomic-primops >=0.7 && < 0.9,

@rrnewton
Copy link
Member

Hello there, thanks for taking the lead on all this. I aim to catch up and cut a release next weekend (10/28-10/29).

@rrnewton
Copy link
Member

so much unsafeness under the hood

Heh. Some may be performance hacks. But some of it is intrinsic to the idea that you define a high-level / monotonic deterministic programming model, but it's implemented with imperative state changes, and we need to tell GHC to trust us that it's actually pure.

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

No branches or pull requests

3 participants