Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use a custom Setup.hs #53

Closed
wants to merge 1 commit into from
Closed

Don't use a custom Setup.hs #53

wants to merge 1 commit into from

Conversation

vmchale
Copy link

@vmchale vmchale commented Sep 9, 2019

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
Copy link
Collaborator

phadej commented Sep 9, 2019

Which bug?

@RyanGlScott
Copy link
Collaborator

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
Copy link
Author

vmchale commented Sep 9, 2019

@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: haskell/cabal#1493

@phadej
Copy link
Collaborator

phadej commented Dec 13, 2020

@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
Copy link
Collaborator

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
Copy link
Collaborator

phadej commented Dec 13, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator

phadej commented Dec 13, 2020

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
Copy link
Collaborator

phadej commented Dec 13, 2020

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

@RyanGlScott
Copy link
Collaborator

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?

@phadej phadej mentioned this pull request Dec 30, 2020
@RyanGlScott
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants