Gabriella439 / Haskell-MMorph-Library

Monad morphisms
BSD 3-Clause "New" or "Revised" License
47 stars 26 forks source link

mmorph-1.0.7 introduces breaking changes in a minor version bump #31

Closed snoyberg closed 7 years ago

snoyberg commented 7 years ago

See bug report at: https://github.com/snoyberg/conduit/issues/288

hvr commented 7 years ago

FYI: To reduce the impact on Hackage, I've soft-blacklisted 1.0.7 via http://hackage.haskell.org/package/mmorph/preferred until this has been sorted out.

This should discourage the cabal solver from picking up the 1.0.7 release for install-plans unless forced to.

hvr commented 7 years ago

The problematic change may be even more problematic:

 class MFunctor t where
     {-| Lift a monad morphism from @m@ to @n@ into a monad morphism from
         @(t m)@ to @(t n)@
     -}
-    hoist :: (Monad m) => (forall a . m a -> n a) -> t m b -> t n b
+#if MIN_VERSION_base(4,8,0)
+    hoist :: Functor m => (forall a . m a -> n a) -> t m b -> t n b
+#else
+    hoist :: Monad m => (forall a . m a -> n a) -> t m b -> t n b
+#endif

This results in a conditional API where API consumers may need to mirror the same kind of CPP conditionality, and the Haddocks shown on Hackage will be misleading as only show the type-signatures for a single CPP branch. It may be better to just drop support for pre-AMP (i.e. base < 4.8) starting with . mmorph-1.1 and avoid the potential mess this could result in.

hvr commented 7 years ago

Fwiw, here's another build-failure I stumbled over (it's safe to say that this will cause major headaches for Hackage trustees and lots of .cabal editing if not resolved in mmorph):

Preprocessing library pipes-4.1.9...
[1 of 6] Compiling Pipes.Internal   ( src/Pipes/Internal.hs, /tmp/pipes-4.1.9/dist-newstyle/build/x86_64-linux/ghc-8.0.1/pipes-4.1.9/build/Pipes/Internal.o ) [Control.Monad.Morph changed]

src/Pipes/Internal.hs:147:24: error:
    • Could not deduce (Monad m) arising from a use of ‘observe’
      from the context: Functor m
        bound by the type signature for:
                   hoist :: Functor m =>
                            (forall a1. m a1 -> n a1)
                            -> Proxy a' a b' b m b1 -> Proxy a' a b' b n b1
        at src/Pipes/Internal.hs:147:5-9
      Possible fix:
        add (Monad m) to the context of
          the type signature for:
            hoist :: Functor m =>
                     (forall a1. m a1 -> n a1)
                     -> Proxy a' a b' b m b1 -> Proxy a' a b' b n b1
    • In the first argument of ‘go’, namely ‘(observe p0)’
      In the expression: go (observe p0)
      In an equation for ‘hoist’:
          hoist nat p0
            = go (observe p0)
            where
                go p
                  = case p of {
                      Request a' fa -> Request a' (\ a -> ...)
                      Respond b fb' -> Respond b (\ b' -> ...)
                      M m -> M (nat (m >>= ...))
                      Pure r -> Pure r }

src/Pipes/Internal.hs:152:39: error:
    • Could not deduce (Monad m) arising from a use of ‘>>=’
      from the context: Functor m
        bound by the type signature for:
                   hoist :: Functor m =>
                            (forall a1. m a1 -> n a1)
                            -> Proxy a' a b' b m b1 -> Proxy a' a b' b n b1
        at src/Pipes/Internal.hs:147:5-9
      Possible fix:
        add (Monad m) to the context of
          the inferred type of
          go :: Proxy a'1 a1 b'1 b2 m r -> Proxy a'1 a1 b'1 b2 n r
          or the type signature for:
               hoist :: Functor m =>
                        (forall a1. m a1 -> n a1)
                        -> Proxy a' a b' b m b1 -> Proxy a' a b' b n b1
    • In the first argument of ‘nat’, namely
        ‘(m >>= \ p' -> return (go p'))’
      In the first argument of ‘M’, namely
        ‘(nat (m >>= \ p' -> return (go p')))’
      In the expression: M (nat (m >>= \ p' -> return (go p')))
Gabriella439 commented 7 years ago

Sorry, this was my mistake: I didn't think this through since I should have realized it was not backwards-compatible. I will revert the change for now, but is the best policy to release the reverted change as mmorph-1.0.8 or mmorph-1.1.0?

snoyberg commented 7 years ago

I'd recommend 1.0.8 with the intention of "1.0.7 was just a faulty release." I don't know that there's an official policy.

Gabriella439 commented 7 years ago

Yeah, I'm fine with blacklisting mmorph-1.0.7

snoyberg commented 7 years ago

I'm not sure if you've seen the trick for this, but the most reliable way to do it I'm aware of is editing the metadata on Hackage to set a bound of, e.g. base < 0 && > 0

Gabriella439 commented 7 years ago

I didn't know about that trick. Thanks!

Gabriella439 commented 7 years ago

Done. mmorph-1.0.8 is up on Hackage with the reverted change and mmorph-1.0.7 uses the trick you mentioned to blacklist the library

snoyberg commented 7 years ago

Awesome, thank you!

On Sun, Nov 20, 2016 at 6:59 PM, Gabriel Gonzalez notifications@github.com wrote:

Done. mmorph-1.0.8 is up on Hackage with the reverted change and mmorph-1.0.7 uses the trick you mentioned to blacklist the library

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Gabriel439/Haskell-MMorph-Library/issues/31#issuecomment-261790119, or mute the thread https://github.com/notifications/unsubscribe-auth/AADBB-Npv-dcsSCKR0Z970eLN_XaPmhvks5rAHyGgaJpZM4K3cjh .

snoyberg commented 7 years ago

Also, just confirming: do you intend to still make that change from Monad to Functor? If so, I'd like to give a soft 👎 on it, since it actually does prevent some useful logic in conduit around merging together sequential actions. I think the same applies to pipes.

Gabriella439 commented 7 years ago

I don't plan to restore the change. My mind was out-to-lunch when I made it and I didn't realize it would make it harder to write MFunctor instances

snoyberg commented 7 years ago

Awesome, thanks!

Gabriella439 commented 7 years ago

You're welcome!

hvr commented 7 years ago

@snoyberg @Gabriel439 just for the record, the trick/convention is actually just base<0 as there's an implicit base>=0 in place since version numbers can never be negative (Hackage will simplify base<0 to base <1 && >1 though, so don't be suprised). Also, I tend to add a comment line above any build-depends: base<0 I edit in to explain (or link to a github issue) why I hard-blacklisted a release

In future Cabal version we may get a new keyword taking a string to help the solver give a more descriptive error message.