Compatibility with transformers-0.6 and mtl-2.3
sjakobi opened this issue · 12 comments
It seems that code changes are required. With --allow-newer
I get this:
src/Control/Lens/Wrapped.hs:91:1: error:
Could not load module ‘Control.Monad.Trans.Error’
It is a member of the hidden package ‘transformers-0.5.6.2’.
Perhaps you need to add ‘transformers’ to the build-depends in your .cabal file.
Use -v (or `:set -v` in ghci) to see a list of the files searched for.
|
91 | import Control.Monad.Trans.Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/Control/Lens/Wrapped.hs:96:1: error:
Could not load module ‘Control.Monad.Trans.List’
It is a member of the hidden package ‘transformers-0.5.6.2’.
Perhaps you need to add ‘transformers’ to the build-depends in your .cabal file.
It is a member of the hidden package ‘rerebase-1.14’.
Perhaps you need to add ‘rerebase’ to the build-depends in your .cabal file.
Use -v (or `:set -v` in ghci) to see a list of the files searched for.
|
96 | import Control.Monad.Trans.List
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build lens-5.1
If I build with mtl-2.3
and transformers < 0.6
, I get compiler errors too:
src/Control/Lens/Zoom.hs:177:10: error:
• Could not deduce (MonadState s (ListT m))
arising from the superclasses of an instance declaration
from the context: Zoom m n s t
bound by the instance declaration
at src/Control/Lens/Zoom.hs:177:10-53
• In the instance declaration for ‘Zoom (ListT m) (ListT n) s t’
|
177 | instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/Control/Lens/Zoom.hs:177:10: error:
• Could not deduce (MonadState t (ListT n))
arising from the superclasses of an instance declaration
from the context: Zoom m n s t
bound by the instance declaration
at src/Control/Lens/Zoom.hs:177:10-53
• In the instance declaration for ‘Zoom (ListT m) (ListT n) s t’
|
177 | instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/Control/Lens/Zoom.hs:185:10: error:
• Could not deduce (MonadState s (ErrorT e m))
arising from the superclasses of an instance declaration
from the context: (Error e, Zoom m n s t)
bound by the instance declaration
at src/Control/Lens/Zoom.hs:185:10-70
• In the instance declaration for
‘Zoom (ErrorT e m) (ErrorT e n) s t’
|
185 | instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
src/Control/Lens/Zoom.hs:185:10: error:
• Could not deduce (MonadState t (ErrorT e n))
arising from the superclasses of an instance declaration
from the context: (Error e, Zoom m n s t)
bound by the instance declaration
at src/Control/Lens/Zoom.hs:185:10-70
• In the instance declaration for
‘Zoom (ErrorT e m) (ErrorT e n) s t’
|
185 | instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build lens-5.1
I've prepared an mtl-2.3
branch that should (I believe) build with transformers-0.6.*
and mtl-2.3.*
. I'm currently holding off on merging this branch into master
since the benchmarks currently do not compile with these libraries due to simonmar/monad-par#74, which is a transitive dependency of criterion
.
Just sayin: there is no patch for monad-par
and I'm somewhat sure that Simon won't write it himself any time soon.
That said, statistics
depends on on monad-par
just to do parMap
in Statistics.Resample.Bootstrap
, which most people use parallel
package for (EDIT: monad-par
isn't used on GHCJS, see https://github.com/haskell/statistics/pull/153/files, and I tried to drop monad-par
completely in https://github.com/haskell/statistics/pull/154/files, but that was a major change, maybe change to use parallel
will be more welcome) . Yet, that package is also maintained by simonmar (but arguably is a simpler one, and lens
depends on parallel
directly as well, so maybe a better dependency overall - both parallel
and monad-par
is slightly silly).
If there's an easy way to trim down criterion
's dependencies, I'm all for it. Either way, we'll need some kind of action downstream.
I made a PR to statistics
, haskell/statistics#187
statistics-0.16.1.0
no longer depends on monad-par
, so that is no longer a blocker.
The next blocker is haskellari/microstache#30.
@RyanGlScott I looked at your branch, and I was curious why the transformers constraint is needed on the Zoom instances for ErrorT and ListT (src/Control/Lens/Zoom.hs line 196)? Those types should be defined in mtl (<2.3), not transformers. I tried removing the transformers constraint and was still able to build with
cabal v2-build --allow-newer=transformers
I wanted to know since I am doing the same migration for microlens and want to make sure I get it right.
Are you referring referring to this code?
#if !MIN_VERSION_transformers(0,6,0) && !MIN_VERSION_mtl(2,3,0)
instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
zoom l = ErrorT . liftM getErr . zoom (\afb -> unfocusingErr #. l (FocusingErr #. afb)) . liftM Err . runErrorT
{-# INLINE zoom #-}
instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
zoom l = ListT . zoom (\afb -> unfocusingOn . l (FocusingOn . afb)) . runListT
{-# INLINE zoom #-}
#endif
The reason those version bounds are in place is because:
ErrorT
andListT
are imported fromControl.Monad.Trans.Error
andControl.Monad.Trans.List
, which are defined intransformers
prior to version0.6.0.0
.Zoom
has aMonadState
superclass, so in order for these instances to typecheck, theMonadState
instances forErrorT
andListT
fromControl.Monad.State
(inmtl
) need to be in scope. These instances are defined inmtl
prior to version2.3
.
I'm not sure why you'd need to modify the bounds to get things building. Do you experience an error of some kind with the bounds as-is?
Yes that's the code. I changed the first line to
#if !MIN_VERSION_mtl(2,3,0)
and forced cabal to use transformers ==0.6 and was still able to build.
Thanks, you cleared up my confusion. I was confused because both mtl and transformers have definitions for ErrorT and ListT. In this case seems we are importing the types from transformers, not mtl as I thought.
I realized now that by forcing transformers ==0.6, it (transitively) forced mtl 2.3, because mtl 2.2.2 requires transformers <0.6, so the mtl version check is enough in that case. If you force transformers ==0.6 AND mtl ==2.2.2, then mtl will fail to build anyway (since it defines instances for ListT and ErrorT which don't exist in transformers 0.6).
So in summary, mtl >=2.3 already implies transformers >=0.6. For microlens, I will do the same version checks that you are doing to be explicit, and in case mtl lowers its transformers lower bound.
To answer your last question: I didn't need to modify any bounds, but I was just testing if the transformers bound was necessary so I pinned transformers ==0.6.
Thanks for the help!
So in summary, mtl >=2.3 already implies transformers >=0.6.
I don't think this is the case, since mtl-2.3
's lower bounds on transformers
are >= 0.5.6
. As a result, it's possible to import ErrorT
and ListT
from transformers-0.5.6
while still not having MonadState
instances defined for them in mtl-2.3
.
The opposite direction—i.e., transformers >= 0.6
implying mtl >= 2.3
—may very well be true. But I've been burned by having imprecise CPP before, so I've found it's best to be as explicit as possible.
I'll work on preparing a new release with these changes soon. To avoid having to make a new major release, I'll create a 5.1
branch and backport all of the non-breaking changes that have happened since the last Hackage release. By my count, that's everything except #994 (which is definitely a breaking change) and #998 (which likely isn't a breaking change, but TH codegen changes are tricky, so I'll hold off on that just to be on the safe side).
I've uploaded lens-5.1.1
to Hackage and applied a Hackage revision to lens-properties-4.11.1
.