dhall-haskell icon indicating copy to clipboard operation
dhall-haskell copied to clipboard

Move library to `dhall-core` package

Open f-f opened this issue 6 years ago • 8 comments

As @Gabriel439 explained in https://github.com/dhall-lang/dhall-haskell/issues/1096#issuecomment-510963407:

the main issue we're running into is that the dhall package is actually doing two separate things:

  • Providing a native Haskell binding
  • Providing the dhall executable

If we could split out the native Haskell binding into a smaller separate package (i.e. dhall-core or dhall-api) and leave the dhall package for the executable (and re-exporting the Haskell API for backwards compatibility) then I'd have fewer reservations about adding extra dependencies to the dhall package. The reason why is that I'd like people using dhall as a native Haskell binding to have a smaller dependency footprint.

For example, if they were separate packages then somebody using the executable-free package to configure their Haskell program wouldn't need to depend on repline, aeson, aeson-pretty, Diff, dotgen, cborg-json, or ansi-terminal. Similarly, we could freely add dependencies like yaml to the executable package guilt-free.

I'll add that:

  • I agree that this separation would be greatly beneficial for downstream packages, e.g. in spago we have issues on some OSs because the repline package dynamically links to the native libterminfo (I guess we could statically compile releases, but I'd say just not depending on repline would be an easier option)
  • Should we also split off dhall-json-core from its executable package?

f-f avatar Jul 13 '19 12:07 f-f

Decoupling dependencies from the core would also make it a bit easier to do smoke testing and porting to new GHCs or particularly massive dependency bumps. aeson in particular would be nice to be able to selectively ignore, since it's a moderately complicated package that tends to need some porting work of its own.

quasicomputational avatar Jul 13 '19 15:07 quasicomputational

It would be great too for eta and dhall-eta. However i would keep the lib part of dhall-json and friends in its own repos/packages, with the core suffix as suggested by @f-f

jneira avatar Jul 13 '19 18:07 jneira

I've just made a pass over dhall's current direct dependencies. If we'd move the Dhall module and its module dependencies into a new library package, I expect that the new package wouldn't need to depend on the following non-boot packages:

  • aeson-pretty
  • ansi-terminal
  • cborg-json
  • dotgen
  • optparse-applicative
  • pretty-simple
  • prettyprinter-ansi-terminal
  • repline

All of these packages are pretty minimal though – limited gains IMHO.

To shed the aeson dependency (and transitive dependencies like dlist, strict, uuid-types) we'd also need to (re)move this FromJSON instance

https://github.com/dhall-lang/dhall-haskell/blob/6a1af03658d78165973358b9e91f1347e3ffaf4b/dhall/src/Dhall/Pretty/Internal.hs#L141-L145

, possibly to dhall-lsp-server where I believe it's being used.

I believe we'd need to keep the Diff dependency because it's being used to produce type error messages.

For additional dependency weight loss, we could try the following:

  • Replace the either dependency (used mostly for Validation in Dhall.Marshal.Decode) with something more light-weight
  • Remove the profunctors dependency. Basically its only current justification is to implement to :: (Profunctor p, Contravariant f) => (s -> a) -> Optic' p f s a (via dimap) which is used once: https://github.com/dhall-lang/dhall-haskell/blob/6a1af03658d78165973358b9e91f1347e3ffaf4b/dhall/src/Dhall/Pretty/Internal.hs#L150-L151 Note that either also has profunctors as a dependency.
  • Remove the parsers dependency, which pulls in attoparsec and charset

Lastly, I'd like to mention that users concerned about dhall's dependency footprint should check whether they require import resolution via HTTP, which can be disabled with -f-with-http.

sjakobi avatar Aug 11 '21 14:08 sjakobi

@sjakobi Thank you for your investigations on that topic!

  • IMHO we should move the FromJSON CharacterSet instance to the 'dhall-lsp-server' package provided we can get rid of the 'aeson' dependency that way.
  • Regarding 'profunctors': I'd say lets revive #998 and try to remove that dependency there. Maybe I'll get to it next week.
  • Regarding Data.Either.Validation.Validation: I would prefer a more lightweight solution; Either by replacing it completely or by trying to get some flags into upstream such that we can strip the dependencies using those.
  • Talking about flags: There is an 'attoparsec' flag in the parsers package. Similarly, configuring 'aeson-pretty' with 'lib-only=true' drops the dependency on 'cmdargs'.

mmhat avatar Aug 18 '21 12:08 mmhat

I'm somewhat skeptical that we can remove Dhall.Optics without increasing our dependence on other heavy-weight packages. https://hackage.haskell.org/package/prolens is lightweight and looks interesting though…

Regarding Cabal flags: The problem I see with these is that they have to be enabled by the end user. AFAIK there's no way in Cabal to say "dhall should preferably be built with aeson-pretty:-flib-only and parsers:-f-attoparsec". Nevertheless it might be useful to compile the flag settings that minimize dhall's dependency footprint, so users can refer to them.

sjakobi avatar Aug 19 '21 09:08 sjakobi

I'm somewhat skeptical that we can remove Dhall.Optics without increasing our dependence on other heavy-weight packages. https://hackage.haskell.org/package/prolens is lightweight and looks interesting though…

I share your concerns but I think it might be worth getting those functions in some lightweight upstream library and depend on that. Like the author of #998 I am not really familiar with 'lens-family' and their policy. Note that 'microlens' already has rewriteOf, to and transformOf. (Not sure if the to defined there suites our needs though.)

I didn't know about prolens which indeed looks interesting!

Nevertheless it might be useful to compile the flag settings that minimize dhall's dependency footprint, so users can refer to them.

Well, that's all I had in mind ;-)

mmhat avatar Aug 19 '21 11:08 mmhat

The recent discussion made me realize that there might be a more lightweight solution to this problem, which is to add a configure flag (e.g. -fno-cli or -f-cli) to disable the modules which are only used for the CLI (e.g. Dhall.Main). Then users who only want the Haskell API will have a lighter dependency footprint

Gabriella439 avatar Aug 19 '21 16:08 Gabriella439

@Gabriel439 I see that solving this by a Cabal Flag is tempting but I see some downsides of this approach as well:

  • If no-cli defaults to false it is inconvient for those who use Dhall as an library in their projects. In fact, we create a similar situation we are facing right now wrt. this library: We need to go through all the flags of the libraries we depend on (transitively) and figure out which combination suites us and we furthermore need to communicate these settings to the users of this library. And as far as it concerns me I still do prefer Dependency Hell over Flag Hell :smile:
  • If no-cli defaults to true it is inconvenient for those who just want install dhall-the-executable and use it as a part of some tooling - we do not want that either.

Personally I use Dhall mostly as a library and therefore I tend to prefer a split into two packages: dhall-core for the embedded usecase and dhall as a "frontend" of that library providing the executable.

mmhat avatar Aug 21 '21 16:08 mmhat