ekmett / distributive

Dual Traversable
http://hackage.haskell.org/package/distributive
Other
41 stars 25 forks source link

Don't use a custom Setup.hs #53

Closed vmchale closed 3 years ago

vmchale commented 5 years ago

Due to a bug in cabal-install, cabal will try to cross-compile any custom Setup.hss. So anything depending on distributive cannot be cross-compiled.

This would change that by simply removing the Setup.hs stanza, which would have no effect on users.

phadej commented 5 years ago

Which bug?

RyanGlScott commented 5 years ago

I'm extremely reluctant to remove the use of cabal-doctest, as it is nigh impossible to make the doctests work correctly without it. I'd be more willing to entertain the idea if cabal doctest were fully supported. (AFAIU, this is not the case currently... right, @phadej?)

In the meantime, I think a serviceable workaround might be to use a Hackage overlay that patches libraries to be suitable for cross-compilation. This overlay looks promising, especially since it has patches for distributive, comonad, semigroupoids, and several other things with Custom setup scripts. @vmchale, can you see if this works for your use case?

vmchale commented 5 years ago

@phadej the bug in question: when cross-compiling, cabal-install tries to build Setup.hs using the cross-compiler and then tries to run it (which fails).

See: https://github.com/haskell/cabal/issues/1493

phadej commented 3 years ago

@RyanGlScott the haskell-ci can run doctest with GHC-8.2+ (relies on .ghc.environment.* files). This is why I don't use cabal-doctest myself anymore. (Check e.g. splitmix)

You loose testing with older GHCs, but if there something to actually test (and there is a risk of different GHCs behaving differently) then maybe a separate test-suite is justified.

RyanGlScott commented 3 years ago

Hm. Having to drop the doctest test suite entirely (in favor of only running doctests on CI) would be a bit unfortunate, in my opinion. Even though doctests tend to test relatively simple code paths, they have still caught numerous bugs that I would not have been aware of if other people hadn't run the test suite. (See here for one example.) On the other hand, it's clear that they cause quite a bit of pain for certain toolchains.

I wonder if there's a middle ground here: could we put the doctest test suite in a separate .cabal file, restricting the use of Custom setups to that test suite? That way, we could still bundle the test suite without forcing those who only care about distributive-the-library (and not distributive's test suite) to use a Custom setup script. I doubt that cabal-doctest's current implementation would support something like this, but I wonder if it could be tweaked to do so.

phadej commented 3 years ago

I wonder if there's a middle ground here: could we put the doctest test suite in a separate .cabal file,

I don't understand this.

You mean having distributive-doctests package? I doubt anyone would run that.

ad example is on the edge, IMHO. Yes, people run tests on different architectures, but there they didn't discover any bug in the implementation, only bug in test itself! I do however expect ARM specific bugs to pop-up soon (whether they are in library, tests, or GHC). Whether comonad (and distributive) would be affected by them. Hard to say.

build-type: Custom in semigroupoids dependency closure is a bit painful now. If you want Foldable1 (or Apply), you have to deal with build-type: Custom mess. For ad and lens on the other hand build-type: Custom haven't caused me any problems.

TL;DR my hard line is compatibility packages. E.g. bifunctors is build-type: Simple and that is very good. If Comonad or Distributive class would migrate to base, then that is the point where I'd require build type to change. Until then it is all just nice to have. (If e.g. distributive would just become a boot library, then I still think it would be easier for make and hadrian if it's build-type: Simple).

RyanGlScott commented 3 years ago

You mean having distributive-doctests package? I doubt anyone would run that.

It's less likely, sure, but the chances of it being run are still higher. IIRC, the CI for Stackage builds other .cabal projects that are bundled with the sdists on Hackage. (Nix might do something similar, but I'm not sure off the top of my head.)

build-type: Custom in semigroupoids dependency closure is a bit painful now.

Definitely. I do recognize that cabal-doctest causes a disproportionate amount pain compared to the code coverage it increases. My philosophy is that we should either use cabal-doctest in all doctest-equipped libraries, or don't use cabal-doctest in any of them. Sadly, given that a working version of cabal v2-doctest doesn't appear to be on the horizon, this leads me to lean towards the latter rather than the former.

Although it would make me somewhat sad to do so, I think I could live with only running doctests on CI. My question is: does haskell-ci's support for running doctests match the capabilities of cabal-doctest itself? For example, I wonder how well it would fare for .cabal files that currently use x-doctest-options or other esoteric flags.

phadej commented 3 years ago

For example, I wonder how well it would fare

Reasonably.

-- Enable Doctest job
doctest: False

-- Additional Doctest options
doctest-options:

-- Doctest version
doctest-version: ^>=0.17

-- Filter packages from .ghc.environment file
-- (this is for filtering out `base-compat` when there is `base-compat-batteries` in the closure)
doctest-filter-packages:

-- Skip doctests for these packages
doctest-skip:
phadej commented 3 years ago

Anyway, I don't feel very strongly about this. I was just looking through issues / pull requests in comonad.

RyanGlScott commented 3 years ago

Ah, I had overlooked the fact that haskell-ci supports fine-grained options for doctest as well. In that case, I would be OK with transitioning from cabal-doctest to only running doctests on CI with GHC 8.2 or later. What are your thoughts on this, @ekmett?

RyanGlScott commented 3 years ago

This has been superseded by #60, which removes the custom Setup script in favor of a cabal-docspec–based approach to running doctests. See also ekmett/lens#959, which tracks the larger effort to migrate other Kmettiverse libraries away from custom Setup scripts.