Working with Dhall 1.20

[mstksg]

Hi! Thanks for the great package :)

While I was submitting this, I just noticed also #143, which is still open and which might be a bit more thorough than this PR! So feel free to close this one if you want; I can start pulling in changes from #143 once it is merged :) Just leaving this here in case you might want to do something with both.

[jneira]

@mstksg thanks for the step forward! Some suggestions:

• I would use dhall-1.20.1 version as lower bound, cause 1.20.0 had a serious bug and was updated in hackage to make it cabal impossible to use it. I think we should double checking here to make it explicit
• The nix derivation should be updated to make hydra ci pass the build, see f.e. 5bf23ca and 78a76b1
  • I used cabal2nix to automatically generating it, feel free to ask if you have any question about its use

What do you think about apply the significant changes of this (that is, from dhall-1.19.1to dhall-1.20.1) in #143 and make the next cabal-to-dhall version jump directly to 1.20.1?

[mstksg]

@jneira Thanks for the review and suggestions. It makes a lot of sense!

As for applying changes to other PR's, I'm not quite sure how the logistics of that work on github, since it's the first time I'd be working on an unmerged PR. Should I check out the branch that #143 is on and start working from there, and then submit that as a new PR once #143 is merged?

[jneira]

As for applying changes to other PR's, I'm not quite sure how the logistics of that work on github, since it's the first time I'd be working on an unmerged PR. Should I check out the branch that #143 is on and start working from there, and then submit that as a new PR once #143 is merged?

That would be an option, another one would be make a pr against my branch so #143 already would have all changes.

[quasicomputational]

Oh, and concerning the overlapping pattern warnings: it's a GHC bug. See #136 for the history there.

[ocharles]

I'm happy to merge this once @quasicomputational's comments are addressed. I agree the smaller PR is easier to merge and make progress with. I would like to bring in some of @jneira's changes from his PR but that can happen after

[jneira]

Usually smaller is better so agree in focus in this one:

• Closed #143
• Just in case can be used, the commits are saved in https://github.com/dhall-lang/dhall-to-cabal/tree/dhall-1.19.1,

[jneira]

@mstksg maybe it is needless to say that you can cherry-pick, copy o whatever you consider useful the code in https://github.com/dhall-lang/dhall-to-cabal/tree/dhall-1.19.1 to address the requested changes and merge the pr. Also let me know if i can help with them in any way.

@ocharles

I would like to bring in some of @jneira's changes from his PR but that can happen after

If you think that the rest of changes after merge this can be useful i can do a new pr (or several prs to make them smaller and easier to review). Let me know if you think the changes can/need to be splitted.

[ocharles]

@jneira Certainly the simplification to the Dhall files needs to happen, so that's one separate PR that we can have after this.

[mstksg]

This seem to be running for me; I've applied @quasicomputational 's suggestions, and also rebased against HEAD.

One thing RE: @jneira 's #143 branch: I noticed a lot of refactoring code in both lib/DhallToCabal.hs and lib/CabalToDhall.hs. From what I can infer, it seems to mostly be just reducing code duplication and improving readability by pulling out common code into helper functions. Is this the case, or does it cover functionality that is necessary for being up to date with new cabal or dhall? If it isn't related to compatibility, I might suggest we leave it out for now and try to pull it in as a separate PR. If not, I'll try to pull in the changes.

[jneira]

@mstksg Yeah, the required changes was mainly in Let expressions and they are done. The rest of them in dhall-1.19.1 are pure refactoring. The all green test suite confirm it.

I had to change stack.yaml to run the tests wit stack:

resolver: lts-13.5
packages:
  - .
extra-deps:
  - dhall-1.20.1
  - Cabal-2.4.0.0
  - tasty-1.1.0.4

Otoh it would be great that hydra ci would be ok too, just in case. For everything else LGTM.

[jneira]

To make hydra work some of *.nix should be changed. I used cabal2nix to generate the files.

For dhall-to-cabal.nix executing cabal2nix . in the project dir it generates:

mkDerivation {
  pname = "dhall-to-cabal";
  version = "1.3.1.0";
  src = ./.;
  isLibrary = true;
  isExecutable = true;
  libraryHaskellDepends = [
    base bytestring Cabal containers contravariant dhall filepath
    hashable text transformers vector
  ];
  executableHaskellDepends = [
    base bytestring Cabal dhall directory filepath microlens
    optparse-applicative prettyprinter text transformers
  ];
  testHaskellDepends = [
    base bytestring Cabal dhall Diff filepath microlens prettyprinter
    tasty tasty-golden tasty-hunit text
  ];
  homepage = "https://github.com/ocharles/dhall-to-cabal";
  description = "Compile Dhall expressions to Cabal files";
  license = stdenv.lib.licenses.mit;
}

For dhall.nix (with cabal2nix cabal://dhall-1.20.1) the output is

{ mkDerivation, aeson, aeson-pretty, ansi-terminal, base
, bytestring, case-insensitive, cborg, cborg-json, containers
, contravariant, criterion, cryptonite, deepseq, Diff, directory
, doctest, dotgen, exceptions, filepath, haskeline, http-client
, http-client-tls, http-types, lens-family-core, megaparsec, memory
, mockery, mtl, optparse-applicative, parsers, prettyprinter
, prettyprinter-ansi-terminal, QuickCheck, quickcheck-instances
, repline, scientific, serialise, stdenv, tasty, tasty-hunit
, tasty-quickcheck, template-haskell, text, transformers
, unordered-containers, uri-encode, vector
}:
mkDerivation {
  pname = "dhall";
  version = "1.20.1";
  sha256 = "e077e8f4945484db4e35e8ae422e9eabac1b155972602f0404d818e3e185
  revision = "1";
  editedCabalFile = "1km0zbbahhq24s84s9gcck1javhplqjg51q4qf8i19iahnxkl3r
  isLibrary = true;
  isExecutable = true;
  libraryHaskellDepends = [
    aeson aeson-pretty ansi-terminal base bytestring case-insensitive
    cborg cborg-json containers contravariant cryptonite Diff directory
    dotgen exceptions filepath haskeline http-client http-client-tls
    http-types lens-family-core megaparsec memory mtl
    optparse-applicative parsers prettyprinter
    prettyprinter-ansi-terminal repline scientific serialise
    template-haskell text transformers unordered-containers uri-encode
    vector
  ];
  executableHaskellDepends = [ base ];
  testHaskellDepends = [
    base containers deepseq directory doctest filepath mockery
    prettyprinter QuickCheck quickcheck-instances serialise tasty
    tasty-hunit tasty-quickcheck text transformers vector
  ];
  benchmarkHaskellDepends = [
    base bytestring containers criterion directory serialise text
  ];
  description = "A configuration language guaranteed to terminate";
  license = stdenv.lib.licenses.bsd3;
}

You can install cabal2nix using cabal or stack.

[mstksg]

I've pulled in those adjustments :)

[quasicomputational]

LGTM!

[ocharles]

Thanks all! I'll try and cut a release soon, my day is just finishing here so it will probably be tomorrow.