dhall-lang / dhall-to-cabal

Compile Dhall expressions to Cabal files
MIT License
100 stars 19 forks source link

Generated GenericPackageDescription is not strictly equal to cabal one #130

Open jneira opened 6 years ago

jneira commented 6 years ago

Hi, in the process of integrate dhall with etlas (a cabal fork suited for eta), we've found that the GenericPackageDescription generated by dhall-to-cabal is not equal to the generated by cabal itself using parseGenericPackageDescription. Although it is similar enough to generate the same output using showGenericPackageDescription (to be fair, the main goal of the package), i am afraid that you cant use it directly against the cabal api. I've setup a branch adding a strict comparation to analyze the changes between them:

I think some sort of strict comparation can be useful to detect and analyze possible bugs. I will open a pr to discuss the relevance of and possible changes over the strict tests.

quasicomputational commented 6 years ago

RE (3), I independently discovered that omitting condTreeConstraints isn't a good idea if the GenericPackageDescription is being passed into the solver. Fortunately, the value of condTreeConstraints is completely determined by the contents of the corresponding node, so it can be done as a post-processing step. Here's some code that does just that:

fixGPDConstraints
  :: GenericPackageDescription
  -> GenericPackageDescription
fixGPDConstraints
  = over ( Lenses.condBenchmarks . traverse . _2 ) fixCondTreeConstraints
  . over ( Lenses.condExecutables . traverse . _2 ) fixCondTreeConstraints
  . over ( Lenses.condForeignLibs . traverse . _2 ) fixCondTreeConstraints
  . over ( Lenses.condLibrary . traverse ) fixCondTreeConstraints
  . over ( Lenses.condSubLibraries . traverse . _2 ) fixCondTreeConstraints
  . over ( Lenses.condTestSuites . traverse . _2 ) fixCondTreeConstraints

fixCondTreeConstraints
  :: ( Lenses.HasBuildInfo a )
  => CondTree v cs a
  -> CondTree v [Dependency] a
fixCondTreeConstraints ( CondNode a _ branches ) =
  CondNode a deps ( fixCondBranchConstraints <$> branches )
  where
  deps = view ( Lenses.buildInfo . Lenses.targetBuildDepends ) a

fixCondBranchConstraints
  :: ( Lenses.HasBuildInfo a )
  => CondBranch v cs a
  -> CondBranch v [Dependency] a
fixCondBranchConstraints ( CondBranch cond true falseMay ) =
  CondBranch cond
    ( fixCondTreeConstraints true )
    ( fixCondTreeConstraints <$> falseMay )
jneira commented 6 years ago

Wow, thanks for that elegant code. However, do you think that inform that field shouldn't be in cabal-to-dhall? could be done in parsing, at same time that other condTree* fields?

quasicomputational commented 6 years ago

Yeah, makes sense to do it there. I had it in my head that we were doing something with the conditionals that meant that it'd have to be a separate step, but I think I was getting that mixed up with the cabal-to-dhall code or something.