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

Update GHC and package dependency version pinning #1025

Merged
merged 12 commits into from
Feb 8, 2021
Merged

Conversation

atomb
Copy link
Contributor

@atomb atomb commented Jan 21, 2021

No description provided.

@RyanGlScott
Copy link
Contributor

As far as why this is failing on Windows in particular, see http://hackage.haskell.org/package/hsc2hs-0.68.7/hsc2hs.cabal. To work around a Windows-specific process bug, that version of hsc2hsrequires process >= 1.6.8, which is more recent that what is pinned in the .config file.

One possible way to fix this would be to re-freeze the files but with hsc2hs==0.68.6, which doesn't have quite as restrictive version bounds on process for Windows.

@atomb atomb force-pushed the at-update-freeze branch 2 times, most recently from 4705f2d to d836f04 Compare February 3, 2021 22:35
@atomb atomb marked this pull request as ready for review February 5, 2021 16:20
@atomb atomb requested a review from RyanGlScott February 5, 2021 16:21
@RyanGlScott
Copy link
Contributor

This looks sensible, although I worry that we'll have similar difficulties later when we try to update the .config files. Can you document somewhere the series of additional steps you needed to apply on top of cabal freeze to make things work? Better yet would be scripting these additional scripts, but perhaps that's not feasible.

@atomb
Copy link
Contributor Author

atomb commented Feb 5, 2021

I ultimately want this to be completely automated, and think it could be, but it'll take more work, and until we do at least part of this commit we can't build using GHC 8.10.3, which it's important for us to be able to do.

What I'd like in the long run would be to have some sort of automated process that would try running cabal freeze on each platform, try to build, and create a PR if the build succeeds. That way we could keep building against the latest libraries, but only through an easy, opt-in process.

There were some manual changes needed here because of some reorganization we've been doing. After this merge, I think the only manual changes would be to remove platform-specific entries. Alternatively, we could have separate freeze files for Windows (which we've done in the past), which should eliminate any need to do manual work.

@RyanGlScott
Copy link
Contributor

OK. I'm fine with merging this as-is, but I do think it's worth writing down what the platform-specific packages are somewhere, as that could save someone else some pain down the road.

@atomb
Copy link
Contributor Author

atomb commented Feb 5, 2021

Yes, that's a very good point. I just pushed a commit to the README outlining what packages to leave out of freeze files.

Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Now that we have Mergify enabled, I'm going to try to see if I can get this automatically merged once CI passes by applying the new "ready-to-merge" label.

@RyanGlScott RyanGlScott added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Feb 5, 2021
@atomb
Copy link
Contributor Author

atomb commented Feb 5, 2021

Whee! That'll be awesome. :) I'm so happy to have Mergify.

@brianhuffman
Copy link
Contributor

@atomb If you want to give your approval to #1059 then we can see have a race and see how mergify handles it

@brianhuffman
Copy link
Contributor

@atomb: So #1059 got merged first, and now this one's out of date with master. I think we need a "keep-updated" label if we want Mergify to automatically rebase and restart the CI when that happens. (That label doesn't exist yet; we should double check to make sure we name it the right thing.)

Also, I see some CI failures with the message "Error: retrieving gpg key timed out" which probably have nothing to do with this PR.

@atomb atomb merged commit 451538b into master Feb 8, 2021
@atomb atomb deleted the at-update-freeze branch February 8, 2021 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants