Use cabal-doctest

[phadej]

ekmett/bytes#32

Tried locally with GHC HEAD and stack.yaml for ghc 8.0.1, seem to work.

[phadej]

cc @RyanGlScott @bgamari

[phadej]

some transient failure in GHC-8.0 job. doctests with GHC-7.0 seems to be genuinely broken.

[phadej]

This setup is tested also with bytes, http-api-data and lens. Travis' builds are all green. Also tested locally with stack and cabal new-build.

I'll wait for Ryan's and Ben's comments before merging them there.

[ekmett]

LGTM

[RyanGlScott]

A couple of inline comments, but otherwise this looks fantastic. Thank you so, so much for taking on this endeavor, @phadej. Setup scripts are something that quickly exhaust my patience, so I'm glad you're willing to wade through the muck to improve the state of things.

One thing I'm wondering: you mentioned that this works with stack and cabal new-build. Does it also work with cabal sandbox? I remember having some trouble getting that to work at one point. If it doesn't work with sandboxes, it's not a deal-breaker, but something worth considering.

Looking into the future, it looks like the Setup logic has become much more complicated than it used to be, so it might be wise to put as much of Setup into a library as possible. Obviously, the lack of custom-setup on older Cabals prevents us from putting everything into a library, but it would still be nice to be able to say:

main :: IO ()
#if MIN_VERSION_Cabal(1,24,0)
main = buildDoctestsModuleHook
#else
main = -- fallback case
#endif

Happily, the logic in the doctests driver itself is still relatively simple, so it might not need to be put into its own library. Or maybe it should anyway? Up to you.

Anyways, once my inline comments are addressed, feel free to merge.

[RyanGlScott]

Happily, you could remove the entirety of withUnicode if you're willing to depend on the code-page library and use withCP65001 instead. You also wouldn't need this file to be a .hsc file and could remove the use of hsc2hs.

Disclaimer: I wrote code-page ;)

[phadej]

Should we (you?) submit a patch to doctest? It will benefit everyone at once, if it would live there?

[RyanGlScott]

Good idea. I'll work on a doctest patch sometime today.

[RyanGlScott]

sol/doctest#149

[RyanGlScott]

Until that's merged, there's certainly no harm in keeping withUnicode (or withCP65001, if you want to get rid of this CPP cruft now). It should be the case that withUnicode . withUnicode = withUnicode.

[RyanGlScott]

Why do we need to define TRUSTWORTHY?

[phadej]

It's just left from lens. (IMHO it would be easier just to leave it, than to make a small forks of Setup.hs / doctests.hsc for each package).

[RyanGlScott]

Let me phrase my question this way: is there a reason why the Setup script can't figure out that TRUSTWORTHY is a flag, and have it pass that as an argument to doctest? That is, why wouldn't TRUSTWORTHY just be in the flags argument?

[phadej]

I see. I think I can grab cpp-options from lib's BuildInfo. I'll make the change.

[phadej]

See discussion at haskell/cabal#2327 Hopefully in the futurue we don't need setup-type: Custom, neither doctest.hs.

[RyanGlScott]

Hopefully in the futurue we don't need setup-type: Custom, neither doctest.hs.

Certainly. And a I owe you a big thank-you for pursuing that as well. This is a much-needed improvement to the Haskell ecosystem!

[phadej]

Ok. Now this seems to work. Next step: I'll make a library out of current Setup.hs, but still use current version for setup-depends unaware cabal-installs.

[RyanGlScott]

Huzzah!

Now comes the all-important question: do you want to release a new version of distributive now? I don't have time at the moment to go through every remaining Setup.hs in the Kmettiverse and upgrade it to this, but at the very least we could upload this to Hackage for now. That way we could more thoroughly battle-test it in the wild before migrating everything else over to it.

Also there's the question of Stackage (is cabal-doctest currently in Stackage nightly?).

[phadej]

I'll add cabal-doctest to Stackage, so it will enter there soonish.

About release: I'm neutral. If there are other reason to release also, then why not.