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

[Mikolaj]

This emerged from #8503 (comment). 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]

Couple notes.

• doctest that we are using seems to be reasonably actively maintained (last commit at the moment is from 5 days ago).

• The failures that we got when upgraded from GHC 8.10 to 9.2 are not entirely trivial (not a base bounds problem, etc.). At least, doctest itself installs just fine. It's the tests that are failing, and by the look of it, it seems something wrong with the environment. Here's a full run on GHC 9.2 and here's an example of failed test:

   Cabal-syntax/src/Distribution/Types/VersionRange/Internal.hs:286: failure in expression `simpleParsec "^>= 3.4" :: Maybe VersionRange'
   expected: Just (MajorBoundVersion (mkVersion [3,4]))
    but got: 
             ^
             <interactive>:161:33: error:
                 Not in scope: type constructor or class ‘VersionRange’
  

  It's almost like doctest doesn't see Cabal-syntax contents somehow.

• One significant downside of the doctest tool we're using is dependence on environment files rather than normal cabal packages. At least, if you believe this comment:

  cabal/Makefile

  Line 74 in a5106be

  # doctests (relies on .ghc.environment files)

  I thought maybe the code setting up these environments should be updated somehow, but I don't see how:
  

  cabal/.github/workflows/quick-jobs.yml

  Lines 78 to 81 in a5106be

  	- name: Install libraries
  	run: |
  	cabal-env --transitive QuickCheck
  	cabal-env array bytestring containers deepseq directory filepath pretty process time binary unix text parsec mtl

• Note that the upstream suggests using another method than what we do: cabal repl --with-ghc=doctest --build-depends.... I doubt that's the root of our problem though (but that's just a hunch).

[Mikolaj]

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

[sol]

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]

@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]

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

[Mikolaj]

Right, whatever the volunteer chooses and provides rationale for.

[ulysses4ever]

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]

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]

• You probably want to use -w instead of -Wno-type-defaults. We may want to document this in the README of Doctest. PRs welcome.
• 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? If not, then this is some thing we would need to figure out first.

[ulysses4ever]

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 😞 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]

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]

@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]

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]

@Mikolaj could you rather show a doctest using QuickCheck?

[Mikolaj]

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

[Mikolaj]

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

[ulysses4ever]

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

[andreasabel]

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

• Extra type-defaults warnings (from OverloadedStrings) make doctests fail sol/doctest#390

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]

@wismill could have just discovered the root of the mystery with the failure to find QuickCheck #6859 (comment)

[ulysses4ever]

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 #8504 (comment)).

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).