Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build streamly with ghc-9.6.1 #2415

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Build streamly with ghc-9.6.1 #2415

merged 2 commits into from
Jul 4, 2023

Conversation

rnjtranjan
Copy link
Collaborator

No description provided.

@rnjtranjan rnjtranjan force-pushed the build_961 branch 2 times, most recently from 0a6eadc to 6e7ab62 Compare June 6, 2023 12:14
@Bodigrim
Copy link
Contributor

Bodigrim commented Jun 6, 2023

newtype ParallelT m a = ParallelT {getParallelT :: K.StreamK m a}
instance MonadTrans ParallelT where
{-# INLINE lift #-}
lift = ParallelT . K.fromEffect

instance MonadAsync m => Monad (ParallelT m) where
return = pure
{-# INLINE (>>=) #-}
(>>=) = bind

The basic problem is that ParallelT is not an unconstrained monad transformer such that forall m. Monad m => Monad (ParallelT m), and thus does not match the definition of MonadTrans from transformers-0.6+.

@harendra-kumar
Copy link
Member

These types (ParallelT etc) were deprecated in the last release. We can either find a workaround to keep them working for some more time or just stop support for newer compilers if it is too hard or impossible.

Copy link
Member

@harendra-kumar harendra-kumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not comment out parts of the code. Either a whole module is in or out. In fact we should have the whole set of modules using IsStream out together. Otherwise it will be extremely confusing and messy for users.

Remove MonadTrans/MonadBase instances for:

* ParallelT
* AsyncT
* WAsyncT
* AheadT

To accommodate a breaking change in transformers-0.6 .
@rnjtranjan
Copy link
Collaborator Author

looks good.

@harendra-kumar harendra-kumar merged commit 4ccd51c into master Jul 4, 2023
13 of 15 checks passed
@harendra-kumar harendra-kumar deleted the build_961 branch January 25, 2024 14:39
@hasufell
Copy link
Contributor

hasufell commented Jan 27, 2024

Can this be backported to older streamly versions? (more specifically 0.8).

@harendra-kumar
Copy link
Member

Which version do you want it for? And what is the motivation? Can we use a newer streamly version instead?

The problem is that it is a breaking change which requires a major version bump. It will be confusing if things break just because a newer transformer version was chosen by cabal automatically.

@hasufell
Copy link
Contributor

Which version do you want it for?

0.8

Can we use a newer streamly version instead?

I'd have to migrate two packages, which I absolutely don't have bandwidth for since the API of streamly breaks too often and too severely with every major release:

@harendra-kumar
Copy link
Member

harendra-kumar commented Jan 27, 2024

0.9.0 was an exception due to some fundamental changes in concurrency API. let me check how easy or difficult it would be to migrate these. If we send a PR migrating these would it be fine?

@hasufell
Copy link
Contributor

hasufell commented Jan 27, 2024

If we send a PR migrating these would it be fine?

I don't want to risk regressions at the moment and my test suites are not good enough.

@harendra-kumar
Copy link
Member

How do we go about it then? The only option is to release a 0.8.x with the breaking change. Is there a better way?

@hasufell
Copy link
Contributor

Ah shoot... I missed that the fix itself involves a breaking change. I guess then it's not possible.

Going forward you should consider using the first version component more, so that such intermediate bumps are possible. Or just bump 2-3 increments of the second version component instead of the next.

@harendra-kumar
Copy link
Member

One possible option is to upload a new 0.8.x with a build flag ghc96 which you can enable in your build. Will that work?

@hasufell
Copy link
Contributor

One possible option is to upload a new 0.8.x with a build flag ghc96 which you can enable in your build. Will that work?

Well, flags really shouldn't affect exposed API.

The only option seems to be migration, so I'll be stuck with GHC < 9.6 for a while.

@harendra-kumar
Copy link
Member

Ok, let's try yaml-streamly first.

@hasufell
Copy link
Contributor

Hm... from what I understand, the patch here has nothing to do with GHC-9.6, but with transformers. But it's possible to depend on a different than "shipped-with-ghc" transformer package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants