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

Add GHC 9.6 #286

Merged
merged 7 commits into from
Nov 18, 2023
Merged

Add GHC 9.6 #286

merged 7 commits into from
Nov 18, 2023

Conversation

epicallan
Copy link
Collaborator

@epicallan epicallan commented Nov 14, 2023

  • Stack fails to execute plugins with GHC 9.6 and for that reason, SourceConstraints is disabled on GHC 9.6 for now.

@epicallan epicallan self-assigned this Nov 14, 2023
@epicallan epicallan marked this pull request as ready for review November 14, 2023 21:41
@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch 10 times, most recently from 44c0cd4 to 8918e12 Compare November 15, 2023 20:33
Copy link
Collaborator Author

@epicallan epicallan Nov 15, 2023

Choose a reason for hiding this comment

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

Comment: This golden test expectation file is suspiciously empty and I can't figure out why. I was expecting to see base, unliftio-core and text dependencies

Copy link
Owner

Choose a reason for hiding this comment

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

@epicallan mprelude is "wierd" as its a deep local dependency and is not registered to the stack.yaml like the others, you'll see this in extra-deps NOT packages.

You can try moving it to packages now as maybe stack is now smarter for "dependency trees" in packages.

Copy link
Collaborator Author

@epicallan epicallan Nov 15, 2023

Choose a reason for hiding this comment

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

I moved it to packages and stack and seemed to have no trouble with its dependency tree resolutions. I will update PR with this change.

Update:

Further testing shows that dependency tree resolution is not yet smarter.

@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch 2 times, most recently from 47f47e3 to f6eca7e Compare November 15, 2023 20:45
@epicallan epicallan changed the title Change to upgrade to GHC 9.6 Add GHC 9.6 Nov 15, 2023
@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch from 18a78ae to e244db1 Compare November 15, 2023 21:07
@epicallan epicallan requested review from mbj, dkubb and snusnu November 15, 2023 21:10
@@ -174,7 +174,6 @@ test-suite test
, mtl
, scientific
, should-not-typecheck
, source-constraints
Copy link
Owner

Choose a reason for hiding this comment

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

@epicallan I do not see a common code change assocaited with this, commit scheduling error?

Copy link
Collaborator Author

@epicallan epicallan Nov 15, 2023

Choose a reason for hiding this comment

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

So common/package.yaml already conditionally adds source-constraints as a dependence. This commit removes the duplicate addition of source-constraints via individual packages.yaml configs.

Maybe I should reword the commit to: Remove duplicate source-constraints dependency

Copy link
Owner

Choose a reason for hiding this comment

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

yes but the change to common/package.yaml is not on that commit?

Copy link
Collaborator Author

@epicallan epicallan Nov 16, 2023

Choose a reason for hiding this comment

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

common/package.yaml already had source-constraints as a conditional dependency based on the development flag so there was no need to change it before this change.

The other change I make to common/package.yaml is to make source-constraints dependent on the GHC version. I don't believe that change needs to be part of this commit since it's a new change altogether.

@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch from e244db1 to 683666a Compare November 15, 2023 21:52
@dkubb
Copy link
Collaborator

dkubb commented Nov 16, 2023

@epicallan the git commit message uses Mprelude when it should use MPrelude.

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch 2 times, most recently from 092dbfd to 5659c5d Compare November 16, 2023 19:58
else
ghc-options: -Wwarn
if impl(ghc < 9.6)
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Is there a ghc 9.5? If so, would we want to run the code on it? Or should/can this be <= 9.4?

Copy link
Collaborator Author

@epicallan epicallan Nov 16, 2023

Choose a reason for hiding this comment

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

Good catch. I have changed it to < 9.5 since <= 9.4 excludes 9.4 minor releases such as 9.4.7

* So common/package.yaml already conditionally adds source-constraints as a dependence.
  This commit removes the duplicate addition of source-constraints via individual packages.yaml configs.
@epicallan epicallan force-pushed the change/upgrade-to-ghc-9.6 branch from 6dfd1be to 3d135db Compare November 16, 2023 22:42
@mbj mbj self-requested a review November 18, 2023 00:44
@mbj mbj merged commit 2d16113 into main Nov 18, 2023
49 checks passed
@mbj mbj deleted the change/upgrade-to-ghc-9.6 branch November 18, 2023 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants