-
-
Notifications
You must be signed in to change notification settings - Fork 412
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
Add GHC 9.2 support #1482
Add GHC 9.2 support #1482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution,
Here some additional tasks that mustn't be forgotten :
- Be sure that the Nix can handle GHC 9.2
- Add the parameter in the nix/README
- Add a check with Cabal/GHC 9.2 on the CI (Only the stack check have been updated on the CI) - see https://github.com/haskell-servant/servant/blob/master/.github/workflows/master.yml
- If needed, exclude GHC version of a Cookbook (and not commenting it please)
Are you okay with doing that?
Note : The PR (#1475) must be merged and this PR rebased before integrating it to the code base.
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ I rebased and updated nix/README + github actions. I don't use Nix, so I won't be able to help with the Nix point, and I don't know what you mean by "Cookbook" |
servant/src/Servant/Links.hs
Outdated
type MkLink (arr :> sub) _ = TypeError (PartialApplication HasLink arr) | ||
type MkLink (arr :> sub) _ = TypeError (PartialApplication (HasLink :: * -> Constraint) arr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aab7e0d added another instance of this problem:
type ServerT (arr :> sub) _ = TypeError (PartialApplication HasServer arr) |
HasServer
needs to become (HasServer :: * -> [*] -> Constraint)
(and Constraint
needs to be added to the import list).
@brandon-leapyear Looks like the 9.2 build is failing because of an old cabal-install version. |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ @ysangkok I changed GHC 9.2 to run with Cabal 3.6, but I'm still getting errors. I don't use Cabal myself, can you help fix that? |
Okay I will try on my side and keep you posted. |
I am not entirely making sense of the Cabal failure ; it seems to me that Cabal commits to a bound-to-fail plan because the constraints it tries to satisfy do not match pre-installed versions, but I am not entirely sure. I am slightly concerned with the amount of
I am guessing we don't really have a choice because these libraries do not have 9.2-compatible releases yet ? |
I have managed to get past Cabal configure, but |
Yes, the Hackage & Stackage folks have had some issues with basement and foundation, looks like the 9.2 support for these libraries will come one day™ |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ @gdeest in the future, can you let me know before you push changes to my fork? Thanks! |
@@ -821,7 +823,7 @@ instance (HasContextEntry context (NamedContext name subContext), HasServer subA | |||
-- Erroring instance for 'HasServer' when a combinator is not fully applied | |||
instance TypeError (PartialApplication HasServer arr) => HasServer ((arr :: a -> b) :> sub) context | |||
where | |||
type ServerT (arr :> sub) _ = TypeError (PartialApplication HasServer arr) | |||
type ServerT (arr :> sub) _ = TypeError (PartialApplication (HasServer :: Type -> Constraint) arr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be Type -> [Type] -> Constraint
?
I think it is a bad idea to make the default |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ I intended for the PR to be merged now. I wasn't sure how Stack is used in this project, e.g. if the standard build workflow is via Cabal, and the stack config is just there for reference. I've moved the Stack stuff into a new I would prefer to not have a PR that I maintain open for an indefinite period of time. If you'd prefer to not merge any aspect of this PR at this time, feel free to close. |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨ @ysangkok can we get this merged anytime soon? I'd prefer to not be maintaining this long-term |
@brandon-leapyear I am not a project maintainer, so it is not my decision to make. But I have submitted #1555 which achieves GHC 9.2 compatibility for a subset of packages that work on GHC 9.2, and marks the others as unbuildable. I prefer this approach due to how it doesn't legitimize unofficial forks of dependencies (as previously mentioned). |
✨ _**This is an old work account. Please reference @brandonchinn178 for all future communication**_ ✨
<!-- updated by mention_personal_account_in_comments.py -->
…---
Fine by me. Can you link the same issue in your PR?
On Wed, Mar 9, 2022, 11:06 AM Janus Troelsen ***@***.***> wrote:
@brandon-leapyear <https://github.com/brandon-leapyear> I am not a
project maintainer, so it is not my decision to make. But I have submitted
#1555 <#1555> which
achieves GHC 9.2 compatibility for a subset of packages that work on GHC
9.2, and marks the others as unbuildable. I prefer this approach due to how
it doesn't legitimize unofficial forks of dependencies (as previously
mentioned
<#1482 (comment)>
).
—
Reply to this email directly, view it on GitHub
<#1482 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGUC75NJBNCEESHLHBBDVUTU7DZENANCNFSM5ICWIIJQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ysangkok @brandon-leapyear please check if this PR is still relevant after merge of #1555. And thanks for your work! |
✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨
Related: #1475