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.

Sign up for GitHub

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

Jump to bottom

Add GHC 8.10.5 support #1899

Merged
merged 10 commits into from
Jun 10, 2021
Merged

Add GHC 8.10.5 support #1899

merged 10 commits into from
Jun 10, 2021

Conversation

Ailrun
Copy link
Member

@Ailrun Ailrun commented Jun 6, 2021 •
edited
Loading

This PR

  • drops 8.10.2 support (We need to wait until 8.10.6)
  • adds 8.10.5 support

Resolves Related to #1898, probably resolves related to #1465

CI fails until https://github.com/hsyl20/ghc-api-compat/pull/11 is resolved (If it takes too long, I will redirect the dependency to PR head) I redirected the dependency

Ailrun
Ailrun commented Jun 6, 2021
View reviewed changes
.github/workflows/build.yml Outdated Show resolved Hide resolved
@Ailrun Ailrun force-pushed the move-ghc-8.10.5 branch 6 times, most recently from ca4e047 to 9de7468 Compare June 7, 2021 01:41
@Ailrun
Copy link
Member Author

Ailrun commented Jun 7, 2021

Maybe blocked by upstream? https://gitlab.haskell.org/ghc/ghc/-/issues/19763#note_357220

@jneira
Copy link
Member

jneira commented Jun 7, 2021

We should estimate the impact of the bug, but i suppose we will have to add support for 8.10.5, as people is gonna use it anyways 🤕
It would suppose mark tests as broken for ghc 8.10.5, it will let us make progress and help us to estimate the impact at same time.

@jneira
Copy link
Member

jneira commented Jun 7, 2021

To try to avoid this scenario i think we should build and test hls against ghc branches that will serve as base for minor releases (8.10 and 9.0). This will help ghc developers to catch them earlier too.

@Ailrun Ailrun force-pushed the move-ghc-8.10.5 branch 4 times, most recently from de2dc2a to 7dab1ee Compare June 7, 2021 23:21
@Ailrun
Copy link
Member Author

Ailrun commented Jun 8, 2021 •
edited
Loading

@jneira It looks like that GHC plugin is the only case that causes the problem in our test set, so I think we are not in the worst situation.

@Ailrun
Copy link
Member Author

Ailrun commented Jun 8, 2021 •
edited
Loading

I don't know why but MacOS build stucks after completing Brittany installation... Anyone with MacOS machine?

@konn
Copy link
Collaborator

konn commented Jun 8, 2021 •
edited
Loading

GHC 8.10.5 + Stack gets stuck also on my laptop (MacBook Pro), preprocessing the network package forever...

@Ailrun
Copy link
Member Author

Ailrun commented Jun 8, 2021

@konn so GHC 8.10.5 on Mac is kinda... broken? (as it does not work with both Cabal and Stack)

@konn
Copy link
Collaborator

konn commented Jun 8, 2021

@Ailrun I haven't done any deep investigation, but it seems there is a problem, I'm afraid...

@Ailrun
Copy link
Member Author

Ailrun commented Jun 9, 2021

@konn Could you check whether the network package alone causes the same issue? If so we can report a more specific issue on GHC

@konn
Copy link
Collaborator

konn commented Jun 9, 2021

@Ailrun Ah, I've just confirmed that building the network package alone causes the same issue, and reported it to GHC GitLab: https://gitlab.haskell.org/ghc/ghc/-/issues/19968.

@Ailrun
Copy link
Member Author

Ailrun commented Jun 9, 2021

Awesome, thank you for the fast process!

@Ailrun
Copy link
Member Author

Ailrun commented Jun 9, 2021 •
edited
Loading

Should we leave 8.10.2 until GHC 8.10.6 is out? Especially for MacOS... I disabled MacOS CI and build for 8.10.5, and I don't know whether it's possible to generate a binary in MacOS with GHC 8.10.5

@jneira
Copy link
Member

jneira commented Jun 9, 2021

Should we leave 8.10.2 until GHC 8.10.6 is out

I think we should be cautious and keep it some time more

@Ailrun Ailrun changed the title Move ghc 8.10.2 to 8.10.5 Add GHC 8.10.5 support Jun 10, 2021
@Ailrun Ailrun mentioned this pull request Jun 10, 2021
Closed
jneira
jneira approved these changes Jun 10, 2021
View reviewed changes
Copy link
Member

@jneira jneira left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think we could merge this to make it available to user, which are being used it already.
Great work as usual, many thanks 🙂

@jneira
Copy link
Member

jneira commented Jun 10, 2021

test (windows-latest, 8.10.4, true) is required but it would be Testing / test (windows-latest, 8.10.4) (pull_request)?

@Ailrun
Copy link
Member Author

Ailrun commented Jun 10, 2021

@jneira Yes, now we should change the required test to Windows 8.10.5 with test and Windows 8.10.4 without test. I will change them.

@jneira jneira added the merge me Label to trigger pull request merge label Jun 10, 2021
@Ailrun Ailrun merged commit 7ba6279 into master Jun 10, 2021
@Ailrun Ailrun deleted the move-ghc-8.10.5 branch June 10, 2021 20:22
@Ailrun Ailrun mentioned this pull request Jun 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jneira jneira jneira approved these changes

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ailrun @jneira @konn