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

Build issues with ghc 8.0.0 RC3 #56

Closed
DougBurke opened this issue Apr 12, 2016 · 12 comments
Closed

Build issues with ghc 8.0.0 RC3 #56

DougBurke opened this issue Apr 12, 2016 · 12 comments

Comments

@DougBurke
Copy link
Contributor

I bumped the groundhog transformers constraint from < 0.5 to < 0.6 in groundhog.cabal (as well as bumping the version to 0.7.0.4) and tried to build 6d2f681 with 8.0.0 RC3 - i.e.

% ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.0.0.20160411

and got the following errors:

% cabal install groundhog
Resolving dependencies...
Notice: installing into a sandbox located at
/home/djburke/chandraobs/groundhog/.cabal-sandbox
Configuring groundhog-0.7.0.4...
Building groundhog-0.7.0.4...
Failed to install groundhog-0.7.0.4
Build log ( /home/djburke/chandraobs/groundhog/.cabal-sandbox/logs/groundhog-0.7.0.4.log ):
cabal: Entering directory '/home/djburke/chandraobs/groundhog/groundhog'
Configuring groundhog-0.7.0.4...
Building groundhog-0.7.0.4...
Preprocessing library groundhog-0.7.0.4...

Database/Groundhog/Expression.hs:1:134: warning:
    -XOverlappingInstances is deprecated: instead use per-instance pragmas OVERLAPPING/OVERLAPPABLE/OVERLAPS

Database/Groundhog/Instances.hs:1:57: warning:
    -XOverlappingInstances is deprecated: instead use per-instance pragmas OVERLAPPING/OVERLAPPABLE/OVERLAPS
[1 of 9] Compiling Database.Groundhog.Core ( Database/Groundhog/Core.hs, dist/dist-sandbox-f9efc5f2/build/Database/Groundhog/Core.o )

Database/Groundhog/Core.hs:178:1: error:
    • Potential superclass cycle for ‘Projection'’
        one of whose superclass constraints is headed by a type family:
          ‘ProjectionDb p db’
      Use UndecidableSuperClasses to accept this
    • In the class declaration for ‘Projection'’
cabal: Leaving directory '/home/djburke/chandraobs/groundhog/groundhog'
cabal: Error: some packages failed to install:
groundhog-0.7.0.4 failed during the building phase. The exception was:
ExitFailure 1
@lykahb
Copy link
Owner

lykahb commented Apr 13, 2016

Thank you for the report. It looks like there were quite a few breaking changes. I will look at this.

@DougBurke
Copy link
Contributor Author

Thanks. It is not urgent for me (I'm using heroku so stuck with ghc 7.8 for
the time being).

On Wed, Apr 13, 2016, 12:50 Boris Lykah [email protected] wrote:

Thank you for the report. It looks like there were quite a few breaking
changes. I will look at this.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#56 (comment)

@mightybyte
Copy link
Contributor

Bump

@lykahb
Copy link
Owner

lykahb commented Oct 25, 2016

There is a branch https://github.com/lykahb/groundhog/commits/ghc8. The Template Haskell breaking changes were easy to fix. The hard part is to fix the type inference for the expressions. You can see the errors if you remove the GHC pragmas in Expression.hs and try running the tests.

@andrewthad
Copy link
Contributor

andrewthad commented Nov 2, 2016

I have some improvements to that branch here: https://github.com/andrewthad/groundhog/tree/correct_overlapping_instances

Most of the type errors in the test suite were only caused because this branch has OverlappingInstances disabled in Expression.hs. I've added all of the appropriate OVERLAPPING and OVERLAPPABLE pragmas, and almost all of the type errors are gone. I would appreciate if someone would look over this.

There were two places where this did not clear things up:

  1. There was one test that used pi as a constant to lookup a value. GHC's type inference was unable to unify it with anything, so I added an annotation to force it to be Double.

  2. There's a test that looks like this:

    -- This test must just compile
    testKeys :: PersistBackend m => m ()
    testKeys = do
      migr (undefined :: Keys)
      k <- insert $ Single ""
      select $ RefDirectField ==. k ||. RefKeyField ==. k ||. RefDirectMaybeField ==. Just k ||. RefKeyMaybeField ==. Just k
      select $ AutoKeyField ==. k -- type error on this line
      return ()
    

I'm new to groundhog, so I don't fully what the problem here is or if it's likely to be an issue in practice. I suspect that polymorphic keys are probably uncommon and that this test can be disabled. If so, it would be nice to have a GHC8 release, even if it has to come with these small type inference regressions.

@andrewthad
Copy link
Contributor

Another side note, I just realized that running cabal new-build in groundhog-test is non-deterministic. If I run it once, I get two type errors. If I run it a second time, I get a hundred type errors. For some reason, the second time, it doesn't see the overlap pragmas on the instances. Weird.

@andrewthad
Copy link
Contributor

Nevermind the non-determinism. I think that cabal-install is picking up the wrong version of the module, and I cannot reproduce the issue with stack, so it might just be an issue with new-build. Anyway, I've disabled the failing test for now, so anyone should be able to use this branch to try it out with GHC8. cc @mckeankylej

@mightybyte
Copy link
Contributor

The non-determinism is probably related to this issue. #51

@andrewthad
Copy link
Contributor

@mightybyte It's not that because the errors I get are about the instances I added overlapping pragmas to. The errors are missing the [overlappable] and [overlapping] notes they usually have, so it's like it doesn't see the pragmas at all. I suspect that it's pulling in an earlier build of groundhog when I try to build the tests the second time.

@mightybyte
Copy link
Contributor

Ahh ok. I just wanted to make sure you knew about that one.

@lykahb
Copy link
Owner

lykahb commented Nov 2, 2016

I planned to make a new release after finishing pipes/conduit integration and upgrading to ghc 8. However, recently it's been harder to find time for the open source.

@andrewthad Disabling testKeys is fine, that case should not be common. Can you open a PR? Thank you for your work!

@andrewthad
Copy link
Contributor

I just put up a PR, which my company has been using internally. It has been working for us. There is another module that uses the deprecated OverlappingInstances pragma. I've not looked into updates that module to get rid of the deprecation warning. Someone should probably do that. #62

@lykahb lykahb closed this as completed in #62 Nov 8, 2016
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

4 participants