Compile with ghc-8.1.2017-01-07

[phadej]

Follow up to #31

[phadej]

Note: we need https://github.com/haskell/cabal/issues/4224 to make this setup reliable when there are more than one test-suite. (Having doctests last might help until than).

[phadej]

I asked @ezyang to review this

[ezyang]

A few questions:

• In formatOne you state "The problem is how different cabal executables handle package databases when doctests depend on the library". I still don't understand what this means, or why it implies we have to do -package in that one particular case.
• Looking at the new doctests code, it looks like you are snarfing up package databases based on whether or not they exist at certain paths. I think this can cause very strange, hard to understand errors if both paths exist (which could happen if someone is built with new-build, and then switched to an old-style build) and also means that the code doesn't work if someone is building in a sandbox, etc. Maybe it's good enough for now though! (Certainly the old code hardcoded dist so...)

[phadej]

2: if I could get PackageDBStack used to compile library from all Cabal's back to 1.14 then I could render it

1: for some reason with some old cabal I got the bytes-1.15.2-inplace dependency for doctests (sinsible), but it was registered just as bytes-1.15.2 in inplace db. I don't bother to investigate, whether that was an actual bug. (I don't want to use cabal-install-1.16 locally)

[ezyang]

2: Yeah, you're going to need cooperation from a Setup script to do get this info, but assuming you have a LocalBuildInfo in hand it is not too hard :/

1: Well, if the Cabal is old enough you could rule it out of the support window. I don't know how old it was :/

Anyway, it looks fine for now.

[phadej]

hmm. LocalBuildInfo has PackageDBStack directly. Let's see if it contains exactly what I want. Thanks @ezyang .

[phadej]

... and it doesn't seem to include the inplace db.

[phadej]

@ezyang now it's much clearer, thanks for the "idea" to extract as much as possible from what Cabal gives us.

[ezyang]

Yes, that is because the inplace database is configured by hand by build. But the location is deterministic assuming you know the dist directory, which I see you've added!

[phadej]

Ok, this is no green and works without hacks. Merging. Will update lens and distributive.

ping @RyanGlScott if you want to push this to other repositories too.

[RyanGlScott]

@phadej: Overall, I like this, but I'll echo my sentiments in https://github.com/ekmett/distributive/pull/27#pullrequestreview-16574512: this custom-setup stuff really deserves to be in its own library. I'm worried that we're going to copy over this complicated script into every single one of @ekmett's libraries that uses doctests, find a bug in it a couple of days later, then have to re-update every Setup.hs script once again. At least compartmentalizing it into a library will minimize the change we would have to percolate through the library ecosystem.

[phadej]

@RyanGlScott, yes, I understand your point, copy&paste is usually bad.

1. But at this point (with experience only over 5 packages), I'm only confident to say it works for them and maybe few others. And have no idea what kinds of hooks to create.

2. To get things working with cabal-installs which don't know about setup-depends, external library would be PITA. I'm pretty sure we cannot afford that yet. Currently this script work with pretty old cabal/Cabals.

[RyanGlScott]

@RyanGlScott, yes, I understand your point, copy&paste is usually bad.

I wouldn't be bothered that much if I only needed to copy-and-paste a few lines, but this Setup.hs logic has grown ~160 SLoC and counting! Given the amount of Cabal API surface area that this covers, it's inevitable that future Cabal releases are going to break this script. And when that time comes, I (as a co-maintainer of many of these packages) don't want to have to lose my sanity re-updating Every. Single. Setup.hs. Script. for the umpteenth time.

But at this point (with experience only over 5 packages), I'm only confident to say it works for them and maybe few others.

I think you're underestimating the robustness of your own code! I've seen most of these Setup.hs scripts firsthand, and I can say that 95% of them do the exact same thing. In face, bytes was one of the trickiest ones given its use of c-sources, so the fact that you got it right in bytes means that it's highly likely that you got it right for them all.

If there really is some exceptional case that we discover later which we can't account for, we can reevaluate. But having only 95% of these scripts using a library is far better than the status quo.

And I'm not suggesting that this library needs to work with all doctest test suites in existence. It's quite possible that this will only work with libraries in the Kmettiverse, and I'm OK with that. We need not drown ourselves in our own ambition.

And have no idea what kinds of hooks to create.

That's easy, because you're already creating UserHooks here!

defaultMainWithHooks simpleUserHooks
  { buildHook = \pkg lbi hooks flags -> do
     generateBuildModule flags pkg lbi
     buildHook simpleUserHooks pkg lbi hooks flags
  }

All you'd need to do it turn this into a UserHooks value (I'll call it doctestUserHooks, but feel free to think of a better name) and export it from a library. Then using it is as simple as:

main :: IO ()
#if MIN_VERSION_Cabal(1,24,0)
main = defaultMainWithHooks doctestUserHooks
#else
main = -- what we do currently in Setup.hs
#endif

To get things working with cabal-installs which don't know about setup-depends, external library would be PITA. I'm pretty sure we cannot afford that yet. Currently this script work with pretty old cabal/Cabals.

I disagree! custom-setup is ignored on older versions of cabal-install, so if you're using a version before 1.24, you can just do this in Setup.hs:

#ifndef MIN_VERSION_Cabal
#define MIN_VERSION_Cabal(x,y,z) 0
#endif

(In fact, now that I look, you already do this!)

So with newer Cabals, it'll use the library that compartmentalizes this script. With older Cabals, we can fall back gracefully to what we do now. Having to include an entire copy of this script is unfortunate, but necessary, and besides, the vast majority of the updates we'll perform to this library will be for forwards compatibility with future versions of Cabal. So as long as we get it right for the older Cabal versions (which we've done already), we won't have to update that part much (if any).

So as a co-maintainer, I beg of you: please consider releasing this as a library. It's tempting to just plop a huge blob of code like this into every one of your projects and call it a day, but in the long run it's going to be an enormous pain point. I'd certainly volunteer to help you out if you need any assistance.

[phadej]

Ok, you convinced me. I'll try to conjure the lib.