test: `cabal test -w ghc-9.4 --constraint="mtl == 2.3.1"`

[ncaq]

There were a few places where import availability depended on the mtl version, but was determined by referring to the transformers version, so the CPP macro has been modified to refer to the mtl version. I really wanted to get rid of the direct dependence on transformers, but since mtl does not export MaybeT, etc., I decided it would be difficult to do so.

The way I verified this was by running cabal test --constraint="mtl == 2.3.1" This would not pass before the fix, but it will if you adapt this commit.

[ncaq]

I stopped changing transformers import to mtl. It seems that old GHC can only read from mtl, and if the direct dependence on transformers cannot be removed, there is not much point in changing it.

In the end, all I may have done was make the test code pass through. It's just a modest but necessary for safe version checking.

[RyanGlScott]

Thanks for the patch! That being said, I'm a bit confused by the motivation here. You say:

The way I verified this was by running cabal test --constraint="mtl == 2.3.1" This would not pass before the fix, but it will if you adapt this commit.

Am I missing something? I tried running this command, and it succeeds:

$ cabal test --constraint="mtl == 2.3.1"
Resolving dependencies...
Build profile: -w ghc-9.6.2 -O1
In order, the following will be built (use -v for more details):
 - exceptions-0.10.7 (lib) (first run)
 - exceptions-0.10.7 (test:exceptions-tests) (first run)
Configuring library for exceptions-0.10.7..
Preprocessing library for exceptions-0.10.7..
Building library for exceptions-0.10.7..
[1 of 2] Compiling Control.Monad.Catch ( src/Control/Monad/Catch.hs, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/build/Control/Monad/Catch.o, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/build/Control/Monad/Catch.dyn_o )
[2 of 2] Compiling Control.Monad.Catch.Pure ( src/Control/Monad/Catch/Pure.hs, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/build/Control/Monad/Catch/Pure.o, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/build/Control/Monad/Catch/Pure.dyn_o )
Configuring test suite 'exceptions-tests' for exceptions-0.10.7..
Preprocessing test suite 'exceptions-tests' for exceptions-0.10.7..
Building test suite 'exceptions-tests' for exceptions-0.10.7..
[1 of 2] Compiling Control.Monad.Catch.Tests ( tests/Control/Monad/Catch/Tests.hs, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/t/exceptions-tests/build/exceptions-tests/exceptions-tests-tmp/Control/Monad/Catch/Tests.o )
[2 of 2] Compiling Main             ( tests/Tests.hs, /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/t/exceptions-tests/build/exceptions-tests/exceptions-tests-tmp/Main.o )
[3 of 3] Linking /home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/t/exceptions-tests/build/exceptions-tests/exceptions-tests
Running 1 test suites...
Test suite exceptions-tests: RUNNING...
Test suite exceptions-tests: PASS
Test suite logged to:
/home/ryanglscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/t/exceptions-tests/test/exceptions-0.10.7-exceptions-tests.log
1 of 1 test suites (1 of 1 test cases) passed.

I'm also unclear on what the benefit of removing the transformers dependency is. exceptions still depends on mtl, which itself depends on transformers. Either way, there is a dependency on transformers.

[ncaq]

Am I missing something? I tried running this command, and it succeeds:

I tried it on hand. I found that with GHC-9.6.2, cabal test succeeds even with master, and with GHC-9.4.5 with ghcup, cabal test fails with master status.

The current latest version of the LTS for stackage, lts-21.6, which is used by a good number of people, uses ghc-9.4.5, so it is a version that is worth supporting.

I'm also unclear on what the benefit of removing the transformers dependency is. exceptions still depends on mtl, which itself depends on transformers. Either way, there is a dependency on transformers.

I am only stating that I tried to remove the direct dependence of transformers and was unable to do so, but did not actually do so.

The reason we wanted to remove the direct dependencies is that mtl forms a dependency on transformers, and it was complicated and confusing to manage the mtl version and the transformers version as the build went through. I thought it would be easier to just manage mtl and the transformers dependencies would be automatically resolved.

The reason I am paying attention to dependencies is that the reason I opened this PR is because for some reason doctest did not adopt mtl-2.3.1 as a dependency, and when I forced myself to add it, the build failed, but the scope supports mtl-2.3.1, so I did not understand the meaning. I followed the dependencies using cabal-plan and found exceptions. I fixed it to mtl-2.3.1 and ran cabal test and it failed to compile, so I fixed it anyway.

[RyanGlScott]

I found that with GHC-9.6.2, cabal test succeeds even with master, and with GHC-9.4.5 with ghcup, cabal test fails with master status.

Ah, thanks for the clarification. Indeed, you can also reproduce this with GHC 9.6 by pinning the transformers version to an old one:

$ cabal test -w ghc-9.6 --constraint="transformers==0.5.6.2"
Warning: The package list for 'hackage.haskell.org' is 25 days old.
Run 'cabal update' to get the latest list of available packages.
Resolving dependencies...
Build profile: -w ghc-9.6.2 -O1
In order, the following will be built (use -v for more details):
 - exceptions-0.10.7 (test:exceptions-tests) (first run)
Preprocessing test suite 'exceptions-tests' for exceptions-0.10.7..
Building test suite 'exceptions-tests' for exceptions-0.10.7..
[1 of 2] Compiling Control.Monad.Catch.Tests ( tests/Control/Monad/Catch/Tests.hs, /home/rscott/Documents/Hacking/Haskell/exceptions/dist-newstyle/build/x86_64-linux/ghc-9.6.2/exceptions-0.10.7/t/exceptions-tests/build/exceptions-tests/exceptions-tests-tmp/Control/Monad/Catch/Tests.o )

tests/Control/Monad/Catch/Tests.hs:43:1: error:
    Could not find module ‘Control.Monad.Error’
    Perhaps you meant
      Control.Monad.Co (needs flag -package-id kan-extensions-5.2.5)
      Control.Monad.Co (needs flag -package-id kan-extensions-5.2.5)
      Control.Monad.Co (needs flag -package-id kan-extensions-5.2.5)
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
43 | import Control.Monad.Error (ErrorT(..))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

tests/Control/Monad/Catch/Tests.hs:44:1: error:
    Could not find module ‘Control.Monad.List’
    Perhaps you meant
      Control.Monad.Fix (from base-4.18.0.0)
      Control.Monad.Zip (from base-4.18.0.0)
      Control.Monad.Base (needs flag -package-id transformers-base-0.4.6)
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
44 | import Control.Monad.List (ListT(..))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I agree that we should address this. However, I will note that this PR, while fixing the issue above, does change the API in a rather subtle way. Currently, if you compile exceptions with these constraints:

cabal build -w ghc-9.6 --constraint="transformers==0.5.6.2" --constraint="mtl==2.3.1"

Then exceptions will provide MonadCatch instances for ErrorT and ListT. After this PR, however, that will no longer be the case, since the CPP is now predicated on mtl rather than transformers.

In the spirit of avoiding API changes, I wonder if it would be better to instead change the imports in the exceptions test suite to use transformers imports (e.g., Control.Monad.Trans.Error) rather than mtl imports (e.g., Control.Monad.Error). That way, all of the MIN_VERSION_transformers CPP will accurately reflect the actual library that they are guarding.

What do you think?

[ncaq]

Well, it turns out that I was mistaken. Since ListT in transformers is broken as is well known, mtl-2.3.1 no longer exports instances of ListT etc. My mistake was to think that mtl-2.3.1 would be incompatible with the version of transformers that supports ListT if that situation existed. But in fact, mtl-2.3.1 has a support range of transformers (>=0.5.6 && <0.7), so the old transformers-0.5.6.2 and the new mtl-2.3.1 can be used together. With a policy that emphasizes not making changes to the API, I certainly felt that your proposed method was more correct.

[RyanGlScott]

OK, I think we are in agreement about a plan to fix the test suite, then. Do you plan to submit a revised version of this PR which implements this new plan?

[ncaq]

I commited and pushed.

I tested it command.

cabal test && cabal test -w ghc-9.4 --constraint="mtl == 2.3.1" && cabal test -w ghc-9.6 --constraint="transformers==0.5.6.2" && cabal test -w ghc-9.6 --constraint="transformers==0.5.6.2" --constraint="mtl==2.3.1"