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

Add Dhall.Deriving module for deriving-via helpers #1700

Merged
merged 10 commits into from
Mar 21, 2020

Conversation

akrmn
Copy link
Collaborator

@akrmn akrmn commented Mar 9, 2020

I've been working on adapting @parsonsmatt's blog post on reflection and encoding via applied to Aeson to work on Dhall. I'm happy with my current implementation, though I'd still like to add a few tests and possibly improve documentation with a guide or something similar. I'd also like to gather some feedback before merging.

  • Write a guide?

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 14, 2020

Could you add the new module to dhall/doctest/Main.hs so the doctests are actually run?

@akrmn
Copy link
Collaborator Author

akrmn commented Mar 15, 2020

Hi @sjakobi, I tried adding the Deriving module to dhall/doctest/Main.hs, but I wanted to test locally first. I've been struggling to get the development environment working on my machine (macOS 10.15.3). cabal worked fine for writing the module Dhall.Deriving, but when trying to run cabal new-test dhall:doctest I got a few errors like this:

dhall-haskell/dhall/ghc-src/Dhall/Import/HTTP.hs:38:5: error:
     error: function-like macro 'MIN_VERSION_http_client' is not defined
   |
38 | #if MIN_VERSION_http_client(0,5,0)
   |     ^
#if MIN_VERSION_http_client(0,5,0)
    ^

I tried building with stack too, but stack test dhall:doctest gave me the same errors. Nix is out of the question on my version of macOS, I believe (happy to be corrected, though!), so I also tried building inside an Ubuntu VM. In the VM, running nix-shell didn't provide the executables for cabal or ghc, so I couldn't do anything inside the nix shell.

Finally, nix-build did run, but I got some errors in the doctests, the biggest of which is that DerivingVia wasn't available before ghc-8.6.1, while the default compiler is set to ghc843 in nix/shared.nix, so I'll probably need a check on the ghc version (which I'm not sure how to write).

Whew. I think I've exhausted my options here, so I'd really appreciate your help, if possible. Thanks!

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 15, 2020

Well, while working doctests are totally neat, the setup and ergonomics do leave a lot to be desired! :/

I always use stack to run doctests, which with this patch

--- a/dhall/doctest/Main.hs
+++ b/dhall/doctest/Main.hs
@@ -38,6 +38,7 @@ main = do
 
             -- , prefix </> "src"
             , "-i" <> (prefix </> "src")
+            , prefix </> "src/Dhall/Deriving.hs"
             , prefix </> "src/Dhall/Tags.hs"
             , prefix </> "src/Dhall/Tutorial.hs"
             ]

…gives me:

$ stack test dhall:doctest
dhall-1.30.0: unregistering (local file changes: doctest/Main.hs)
dhall> build (lib + test)
Preprocessing test suite 'doctest' for dhall-1.30.0..
Building test suite 'doctest' for dhall-1.30.0..
[1 of 1] Compiling Main
Linking .stack-work/dist/x86_64-linux/Cabal-2.4.0.1/build/doctest/doctest ...
Preprocessing library for dhall-1.30.0..
Building library for dhall-1.30.0..
dhall> copy/register
Installing library in /home/simon/src/dhall-haskell/.stack-work/install/x86_64-linux/2cd7226215d912742b9bc19685d6004f3f9e5cacfed4b485b2ba1dd5d309906e/8.6.5/lib/x86_64-linux-ghc-8.6.5/dhall-1.30.0-KKKBcbpJFlQ70XgsDWAseO
Installing executable dhall in /home/simon/src/dhall-haskell/.stack-work/install/x86_64-linux/2cd7226215d912742b9bc19685d6004f3f9e5cacfed4b485b2ba1dd5d309906e/8.6.5/bin
Registering library for dhall-1.30.0..
dhall> test (suite: doctest)
                   
Progress 1/2: dhallLoaded package environment from /tmp/stack27734/test-ghc-env
Loaded package environment from /tmp/stack27734/test-ghc-env
Loaded package environment from /tmp/stack27734/test-ghc-env
Loaded package environment from /tmp/stack27734/test-ghc-env
Loaded package environment from /tmp/stack27734/test-ghc-env

when making flags consistent: warning:
    -O conflicts with --interactive; -O ignored.
/home/simon/src/dhall-haskell/dhall/src/Dhall/Deriving.hs:241: failure in expression `:{
newtype Name = Name { getName :: Text }
<BLANKLINE>
data Font = Arial | ComicSans | Helvetica | TimesNewRoman
<BLANKLINE>
data Person = Person
  { personName :: Name
  , personFavoriteFont :: Font
  }
:}'
expected: 
 but got: 
          <interactive>:23:1: error: parse error on input ‘<’

/home/simon/src/dhall-haskell/dhall/src/Dhall/Deriving.hs:298: failure in expression `:{
deriving stock instance Generic Name
deriving stock instance Generic Font
deriving stock instance Generic Person
<BLANKLINE>
deriving anyclass instance FromDhall Name
deriving anyclass instance FromDhall Font
deriving anyclass instance FromDhall Person
:}'
expected: 
 but got: 
          <interactive>:38:1: error: parse error on input ‘<’

/home/simon/src/dhall-haskell/dhall/src/Dhall/Deriving.hs:399: failure in expression `:{
deriving via Codec (Field (CamelCase <<< DropPrefix "person")) Person instance FromDhall Person
deriving via Codec (Constructor TitleCase) Font instance FromDhall Font
deriving via Codec (SetSingletonConstructors Bare) Name instance FromDhall Name
:}'
expected: 
 but got: 
          <interactive>:49:1: error:
              Illegal standalone deriving declaration
                Use StandaloneDeriving to enable this extension
          
          <interactive>:49:1: error:
              Illegal deriving strategy: via
              Use DerivingVia to enable this extension

Examples: 220  Tried: 207  Errors: 0  Failures: 3
                   
dhall> Test suite doctest failed
Completed 2 action(s).
Test suite failure for package dhall-1.30.0
    doctest:  exited with: ExitFailure 1
Logs printed to console

I use Ubuntu BTW.

With cabal you're probably running into this issue: sol/doctest#245

You may want to try some of the workarounds described there.

Also, l think most people eventually managed to get the doctests working with stack, so there are a few things you could try:

  1. Check that you have the most recent stack version installed.
  2. Try running stack build dhall before stack test dhall:doctest to ensure the GHC environment files are written.

Regarding GHC compatibility: The first thing to try would be to move the problematic doctests behind CPP. I usually refer to https://guide.aelve.com/haskell/cpp-vww0qd72#item-fkuvztqe for that.

If that doesn't work out (I'm not sure how well doctest interacts with CPP), we can conditionally disable the doctest suite in dhall.cabal.

This is all a bit annoying and tricky, so please do let me know in case you get stuck! :)

@akrmn akrmn force-pushed the deriving-dhall branch 2 times, most recently from d928d34 to 3c8bbf4 Compare March 15, 2020 19:47
@akrmn
Copy link
Collaborator Author

akrmn commented Mar 15, 2020

Hey again @sjakobi! Thanks for your tips! Eventually I got stack working on macOS, though I had to fiddle with stack.yaml (almost matching your #1695 PR, I just noticed). It took me a few tries but it seems to be working now, the only odd thing is setting a couple of extensions here and there.
Regarding compatibility, I think CPP might be overkill for this module since it's not really useful before ghc-8.6.5. I learned about Cabal conditionals by looking at dhall.cabal and they seem like a good fit for this case.
Again, thanks a lot for the help!

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 15, 2020

I'm glad you got it working! :)

I haven't yet reviewed Dhall.Deriving properly, but I wanted to ask you to add a reference to the new module from the main Dhall module, to make this stuff more easy to discover.

@akrmn
Copy link
Collaborator Author

akrmn commented Mar 15, 2020

Gladly!

@akrmn
Copy link
Collaborator Author

akrmn commented Mar 17, 2020

Hi again @sjakobi, is there anything I need to do to get the hydra check to run?

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 17, 2020

Hydra does run (and fail), but the GitHub interface doesn't show it. It would be great if we could get that fixed properly! :/

For now you can find the Hydra results for this PR at https://hydra.dhall-lang.org/jobset/dhall-haskell/1700.

@akrmn
Copy link
Collaborator Author

akrmn commented Mar 17, 2020

I see! Thanks!

@akrmn akrmn force-pushed the deriving-dhall branch 2 times, most recently from 8677872 to 16d8a98 Compare March 17, 2020 16:03
@akrmn
Copy link
Collaborator Author

akrmn commented Mar 17, 2020

Finally got it to build! Is it okay if I merge it?

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 17, 2020

Well done! Sorry for the delay with my review! I'll try to get it done until tomorrow evening at the latest.

@akrmn
Copy link
Collaborator Author

akrmn commented Mar 17, 2020

Oh don’t worry at all. Thanks for the help!

Copy link
Collaborator

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Sorry again for the late review! This machinery looks super slick! 👍

Nevertheless I have a few comments! ;)

dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall.hs Outdated Show resolved Hide resolved
dhall/src/Dhall/Deriving.hs Show resolved Hide resolved
dhall/src/Dhall/Deriving.hs Show resolved Hide resolved
dhall/doctest/Main.hs Outdated Show resolved Hide resolved
Copy link
Collaborator

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Cheers! :)

dhall/src/Dhall/Deriving.hs Outdated Show resolved Hide resolved
dhall/src/Dhall/Deriving.hs Show resolved Hide resolved
@akrmn akrmn merged commit bf6af02 into dhall-lang:master Mar 21, 2020
@akrmn akrmn deleted the deriving-dhall branch March 21, 2020 13:51
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