search

tweag / asterius

DEPRECATED in favor of ghc wasm backend, see https://www.tweag.io/blog/2022-11-22-wasm-backend-merged-in-ghc
1.98k stars 58 forks source link

Enforce Haskell formatting in CI #334

Open curiousleo opened 5 years ago

curiousleo commented 5 years ago

As of https://github.com/tweag/asterius/pull/325, ormolu is the Haskell formatter of the asterius project. That PR formatted all Haskell code with ormolu. To make sure that future Haskell code is formatted correctly, we should add a CI step that fails if there is a Haskell file that has a non-empty diff with its ormolu-formatted version.

It would be nice to have a convenient way of formatting Haskell files locally. If that is hard to do, we can create a separate issue for that.

ormolu itself is formatted with ormolu - this commit could be a good starting point for enforcing formatting in asterius too: https://github.com/tweag/ormolu/commit/480d6edfb36e6818a5de2c5095cbabce4b97d53a

The relevant CircleCI build step is here: https://github.com/tweag/ormolu/blob/480d6edfb36e6818a5de2c5095cbabce4b97d53a/.circleci/config.yml#L32-L36

gabrielelana commented 5 years ago

It seems like ormolu doesn't support CPP, if we try to run it

ormolu --tolerate-cpp --debug --mode inplace asterius/src/Asterius/Backends/Binaryen.hs
Parsing of source code failed:
  asterius/src/Asterius/Backends/Binaryen.hs:28:1-6
  parse error on input ‘import’

We saw that you were able to run it, how did you do it?

curiousleo commented 5 years ago

Interesting! The PR to format the code base using ormolu came from @TerrorJack originally (https://github.com/tweag/asterius/pull/325). It looks like the exact file you're having trouble with, asterius/src/Asterius/Backends/Binaryen.hs, was formatted successfully in that PR: https://github.com/tweag/asterius/pull/325/files#diff-7ab79c4f28925fd66555fef95bb369ca. I'm not sure how.

Regarding CPP support more generally, there is an ongoing discussion on the ormolu issue tracker here: https://github.com/tweag/ormolu/issues/415.

I'm not sure what the best path forward is here. AFAICT this file is the only not-copied-over Haskell source file in the asterius subdirectory which uses CPP.

One option could be to try to remove the use of CPP here. Perhaps the conditional compilation based on whether BINARYEN is set can be moved to package.yaml entirely? https://github.com/tweag/asterius/blob/33f6eafe5bf6aeeaf9a2ba31f8f14df60618d85d/asterius/package.yaml#L80-L84

Blacklisting could be a (temporary) option too.

TerrorJack commented 5 years ago

When I was formatting Asterius.Backends.Binaryen, I didn't apply ormolu module-wise; I simply selected and formatted the sections within the CPP pragmas, which by themselves are valid "module"s and can be handled by ormolu.

IIRC the binaryen Cabal flag to disable the binaryen backend existed to make it easier for @hamishmack to test his nix build. I don't use this flag myself and it's not used on CI either. If the binaryen package works fine there then probably best thing is to remove this flag and all CPP altogether.