haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.63k stars 695 forks source link

Update/upgrade/switch away from our current doctest tool in CI #8504

Closed Mikolaj closed 1 year ago

Mikolaj commented 2 years ago

This emerged from https://github.com/haskell/cabal/pull/8503#issuecomment-1263470235. Citing the comment:

I think we should investigate whether we'd stick to the current doctest, change it (there are a couple of competing products), fork it, etc. I don't know which doctest we are using, but if it fails for GHC 9.2, it may not be worth sticking to. Or perhaps we somehow use it wrong or from a wrong branch. Outside the scope of this ticket, certainly.

ulysses4ever commented 2 years ago

Couple notes.

Mikolaj commented 2 years ago

@sol: hi! Would you have any quick hint how to improve our usage of your doctest tool? Thank you.

sol commented 2 years ago

Hi 👋

  1. Yes, you should use doctest via cabal repl as documented in the README and not rely on GHC environment files.
  2. As for the specific error you are seeing, the underlying issue is likely a GHC bug. Some versions of GHC unload modules in certain situations. I have reported this before and added a workaround to Doctest. Someone would need to figure out what exactly is happening here. We can then see how to address it, either in Doctest or your setup.
Mikolaj commented 2 years ago

@sol: thank you.

So the first task would be to switch to the cabal repl method of invoking doctest. Once a brave soul experiments with that, we can follow up.

ulysses4ever commented 2 years ago

@Mikolaj another idea would be to try cabal-docspec instead of doctest.

Mikolaj commented 2 years ago

Right, whatever the volunteer chooses and provides rationale for.

ulysses4ever commented 2 years ago

Okay, I tried the recommended method of calling doctest and got a number of similar funny errors where warnings make test fail, e.g., in Cabal-syntax:

src/Distribution/Compat/Newtype.hs:78: failure in expression `alaf Sum foldMap length ["cabal", "install"]'
expected: 12
 but got:
          ^
          <interactive>:509:18: warning: [-Wtype-defaults]
              • Defaulting the following constraint to type ‘[]’
                  Foldable t0 arising from a use of ‘length’
              • In the third argument of ‘alaf’, namely ‘length’
                In the expression: alaf Sum foldMap length ["cabal", "install"]
                In an equation for ‘it’:
                    it = alaf Sum foldMap length ["cabal", "install"]

          <interactive>:509:18: warning: [-Wtype-defaults]
              • Defaulting the following constraint to type ‘[]’
                  Foldable t0 arising from a use of ‘length’
              • In the third argument of ‘alaf’, namely ‘length’
                In the expression: alaf Sum foldMap length ["cabal", "install"]
                In an equation for ‘it’:
                    it = alaf Sum foldMap length ["cabal", "install"]
          12

(Sorry for the Unicode corruption.) @sol, any idea why I get those warnings when doing

cabal install doctest --overwrite-policy=always --ignore-project && cabal build && cabal repl --build-depends=QuickCheck --build-depends=template-haskell --with-ghc=doctest 

instead of a direct call to doctest?

ulysses4ever commented 2 years ago

OKay, I moved one notch further with this: adding --ghc-options="-Wno-type-defaults" to the cabal repl call shown above seems to do the trick.

Next challenge is the following. Cabal-syntax doctests just fine but Cabal fails mysteriously not able to find QuickCheck whereas it's in the build-depends clause, of course.

❯ cabal repl Cabal --build-depends=QuickCheck --build-depends=template-haskell --with-ghc=doctest --ghc-options="-Wno-type-defaults"
Resolving dependencies...
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - Cabal-syntax-3.9.0.0 (lib) (first run)
 - process-1.6.16.0 (lib:process) (requires build)
 - Cabal-3.9.0.0 (lib) (first run)
Preprocessing library for Cabal-syntax-3.9.0.0..
Starting     process-1.6.16.0 (all, legacy fallback)
Building library for Cabal-syntax-3.9.0.0..
[126 of 137] Compiling Distribution.Compat.Newtype ( src/Distribution/Compat/Newtype.hs, /data/artem/cabal/t8504-fix-doctest/dist-newstyle/build/x86_64-linux/ghc-8.10.7/Cabal-syntax-3.9.0.0/build/Distribution/Compat/Newtype.o, /data/artem/cabal/t8504-fix-doctest/dist-newstyle/build/x86_64-linux/ghc-8.10.7/Cabal-syntax-3.9.0.0/build/Distribution/Compat/Newtype.dyn_o )
Building     process-1.6.16.0 (all, legacy fallback)
Installing   process-1.6.16.0 (all, legacy fallback)
Completed    process-1.6.16.0 (all, legacy fallback)
Configuring library for Cabal-3.9.0.0..
Preprocessing library for Cabal-3.9.0.0..
src/Distribution/Verbosity.hs:359: failure in expression `import Test.QuickCheck (Arbitrary (..), arbitraryBoundedEnum)'
expected:
 but got:
          ^
          <no location info>: error:
              Could not load module ‘Test.QuickCheck’
              It is a member of the hidden package ‘QuickCheck-2.14.2’.
              Perhaps you need to add ‘QuickCheck’ to the build-depends in your .cabal file.

Examples: 26  Tried: 17  Errors: 0  Failures: 1
Error: cabal: repl failed for Cabal-3.9.0.0.
sol commented 2 years ago
ulysses4ever commented 2 years ago

Not sure what's the issue is with QuickCheck. Can you try to manually import Test.QuickCheck from a regular cabal repl session (cabal repl Cabal --build-depends=QuickCheck)? Does this work?

Indeed, it does not work. I think it's #6859 :disappointed: I'm surprised cabal repl-based approach works for significant portion of users. I guess, there're not that many people using both QuickCheck in their doctests and project files.

Mikolaj commented 2 years ago

using both QuickCheck in their doctests and project files.

I can't parse this. Is it "using both (QuickCheck in their doctests) and project file" or "using both QuickCheck (in their doctests and project file)"? Because if the former, I have a project with a project file and QuickCheck where doctests work, using, e.g., cabal repl --build-depends=QuickCheck --build-depends=template-haskell --with-ghc=doctest.

ulysses4ever commented 2 years ago

@Mikolaj to fail you need two conditions to apply: 1) your doctests use QuickCheck (NOT the same as your package depends on QuickCheck) 2) you have a project file.

Mikolaj commented 2 years ago

It works (or worked) for me in the other project (https://github.com/LambdaHack/LambdaHack/blob/master/README.md?plain=1#L222). Shall I test if that's still the case?

ulysses4ever commented 2 years ago

@Mikolaj could you rather show a doctest using QuickCheck?

Mikolaj commented 2 years ago

Here you go: https://github.com/LambdaHack/LambdaHack/blob/master/engine-src/Game/LambdaHack/Common/Point.hs#L111

Mikolaj commented 2 years ago

BTW, haskell-ci normally creates cabal.project file, so that most CI's would be affected.

ulysses4ever commented 2 years ago

All right, in that case I don't understand something.

andreasabel commented 1 year ago

@ulysses4ever : I ran into similar problems with doctest in my BNFC project:

The main lesson here was that --ghc-options does not work reliably, you need --repl-options:

cabal repl -w doctest --repl-options=-Wno-type-defaults

(--ghc-options works sometimes, this is the trap.)

ulysses4ever commented 1 year ago

@wismill could have just discovered the root of the mystery with the failure to find QuickCheck https://github.com/haskell/cabal/issues/6859#issuecomment-1415791508

ulysses4ever commented 1 year ago

Just checked that when I point cabal to a project file without allow-newer(I used cabal.project.libonly), the cabal-repl based method of calling doctest worked for Cabal (solving the issue discribed in https://github.com/haskell/cabal/issues/8504#issuecomment-1302498805).

This is great news! We can now switch to the officially recommended method for doctest, and ditch the cabal-env business from our doctest CI script. And we can suggest in the docs how to easily run doctest locally (solving #8147).