unisonweb
Upgrade to LTS 22.26 GHC 9.6.5
Overview
Upgrades to LTS 22.26; GHC 9.6.5
Recommended you update your local GHCup to: HLS 2.9.0.0 and Stack 2.15.5
Does someone who knows nix want to update the flake as well? It seems ghc 965 isn't being found, I'm sure it's an easy fix for someone who knows how it works 🙃
Implementation notes
- Updates the LTS
- Removes redundant applicative updates
- Replaces use of NanoID lib with secure crypto instead, just cuz the NanoID lib has bitrotted
- Move from
x509-*tocrypton-x509-*libs since HVR quit Haskell - Bump fuzzyfind
- Removes a few custom pinnings now that they've been added to the snapshot
Interesting/controversial decisions
Not really
Test coverage
Existing tests
Loose ends
- [ ] Should I fix the JWT lib deprecations now?
Does someone who knows nix want to update the flake as well? It seems ghc 965 isn't being found, I'm sure it's an easy fix for someone who knows how it works 🙃
cc @sellout
Nice!
On testing, could you smoke test that the new file watch code works locally
Does someone who knows nix want to update the flake as well? It seems ghc 965 isn't being found, I'm sure it's an easy fix for someone who knows how it works 🙃
I would normally agree with you, but haskell.nix (which is still used a lot by our flake) doesn’t support much in the way of GHC. But I’ll give it a shot.
Okay I think this is ready to go now except for the Nix stuff which I think @sellout is going to take a gander at 😎
Okay I think this is ready to go now except for the Nix stuff which I think @sellout is going to take a gander at 😎
I have a draft up at #5155, which builds on Nix simplification (against trunk) in a draft at #5154.
Recommended you update your local GHCup to:
HLS 2.9.0.0andStack 2.15.5
Why these specifically?
@aryairani
For stack, The current stack is too old to support this GHC, and that version is just the one recommended by GHCup, I tested it and it works, no other reason.
For HLS, that's the latest version available on GHCup, I tested it out and it seems to work.
I see Greg swapped some versions in the Nix PR, if those work too that's totally fine by me :)
@ChrisPenner Ok excellent, thanks. In that case, I'm just holding to sort out the Nix questions, which we'll try to do later this afternoon. I'll start a Discord thread to answer the question of which versions we should be pinning.
@sellout "No space left on device" for nix-dev-cache.yaml on Ubuntu 😬
According to https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories, the disk size is 14GB.
One can pay for bigger runners (debatable), but not on the free/oss plan.
"No space left on device" for
nix-dev-cache.yamlon Ubuntu 😬
A re-run succeeded. I grabbed the log archive before and after. I’m guessing it needed to build something the first time that it didn’t the second[^1], and that pulled in enough transitive deps to push us over the limit.
[^1]: I checked – it didn’t try to build any GHCs 😅
Since this job builds the devShells as well as the Cabal packages, it should contain all the dependencies (tooling & Haskell libs) needed by Unison no matter which Unison packages need to be rebuilt in a given PR, but we could just be very close to the limit. We could probably break this into multiple jobs, so one makes sure all the dependencies are built, and a second only ever builds Unison packages.
On the plus side, even if this job fails, it will at least push what it’s finished to the cache, so a re-run is more likely to succeed (as it just did) … but we definitely don’t want to be manually re-running the job on every PR. And pinning certain packages (GHC, HLS, etc.) in Cachix can make sure they don’t get GCed.
So we'll plan to merge this after #5041; I'm going to clear the approval so I don't merge it by accident.
@aryairani I checked out the conflicts with #5041 and I'm not too concerned, nothing major; that is, assuming Ormolu has already been re-run on this branch right?
assuming Ormolu has already been re-run on this branch right?
It hasn’t – it appears that in general CI only checks Ormolu against files that were modified in the PR (which is good), and at least some things had slipped through in the period that CI wasn’t running Ormolu[^1]. But I do think it’s good practice to run Ormolu all at once when we upgrade it.
[^1]: I’ve noticed things like reordered imports that I didn’t touch when I format some files locally, etc.
It’s just unfortunate that it’s difficult to separate upgrading Ormolu from the rest of this PR. I can run Ormolu as another commit on this PR, or we can do a separate PR immediately after this. I also figured since we were waiting on #5041, that running Ormolu after we merge that would be prudent.
And since I couldn’t figure out where I linked this before, here is the effect of running Ormolu on top of this PR: https://github.com/sellout/unison/commit/694b065b8ea3e6520c42f3db71adb1cd0d8e62a8
@ChrisPenner You can be in charge of merging this PR when you are ready.
There is also an ormolu Github action set up you can run when ready, which hopefully does the right thing; cc @sellout
Ah, thanks Greg, yeah we can wait then. I'm getting pretty close with project roots now anyways :)
Hey @ChrisPenner , I was trying out this branch and I needed some changes from trunk so I had a go at merging it in & fixing the conflicts: https://github.com/neduard/unison/pull/3 - figured you'll probably encounter the same so it might be worth sharing in case it saves you a bit of time. As before feel free to either cherrypick or simply use as a reference.
Something worth pointing out is that I had to revert #5192 since numerals requires text (>=0.11 && <1.3) but LTS 22.26 has text 2.0.2 ( cc @aryairani ), but other than that the changes were minimal 🚀
Hey @ChrisPenner , I was trying out this branch and I needed some changes from trunk so I had a go at merging it in & fixing the conflicts: neduard#3 - figured you'll probably encounter the same so it might be worth sharing in case it saves you a bit of time. As before feel free to either cherrypick or simply use as a reference.
Something worth pointing out is that I had to revert #5192 since numerals requires
text (>=0.11 && <1.3)but LTS 22.26 hastext 2.0.2( cc @aryairani ), but other than that the changes were minimal 🚀
@neduard I used allow-newer-deps: [numerals] for our stack build, because https://github.com/roelvandijk/numerals wanted containers (<6), so it probably accepted text-2.0.2 due to that flag, before the error was even on my radar. The author of numerals invites a PR with updated bounds, but I didn't have the time/knowledge to do it; but maybe you'd want to take a stab at it since you're more familiar with cabal than I am.
Otherwise, is there a similar directive to cabal to ignore the bounds on numerals (only)? I don't really want to revert #5192.
That's a much better shout @aryairani , thanks! I was so focused on getting it to build that I didn't think of other ways of solving it. I've raised the PR upstream and in the meantime worked around it by adding the following to contrib/cabal.project
allow-newer:
numerals:text,
numerals:containers
That's a much better shout @aryairani , thanks! I was so focused on getting it to build that I didn't think of other ways of solving it. I've raised the PR upstream and in the meantime worked around it by adding the following to
contrib/cabal.projectallow-newer: numerals:text, numerals:containers
Okay great — https://github.com/neduard/unison/pull/3 is merged though, is that still where to look?
worked around it by adding the following to
contrib/cabal.project
Thanks, @neduard. We really need to get a Cabal GitHub check into CI (even if it’s not required). It should generally be trivial to keep it in sync, but easy to overlook without a reminder.
worked around it by adding the following to
contrib/cabal.projectThanks, @neduard. We really need to get a Cabal GitHub check into CI (even if it’s not required). It should generally be trivial to keep it in sync, but easy to overlook without a reminder.
Yeah, the Cabal Github check is missing a) because we didn't have anyone to maintain the .cabal file (no longer true) and b) because it would have doubled the resources needed to run CI (not fully true anymore, but not a non-issue either). I guess we could run cabal build on linux only; wouldn't have run tests. We should have some sort of build cache.
Yeah, the Cabal Github check is missing a) because we didn't have anyone to maintain the .cabal file (no longer true) and b) because it would have doubled the resources needed to run CI (not fully true anymore, but not a non-issue either). I guess we could run cabal build on linux only; wouldn't have run tests. We should have some sort of build cache.
I opened #5225 so we could avoid conflating this discussion (my apologies for starting it here), and so we have somewhere to point when it comes up again.
@ChrisPenner @aryairani https://github.com/unisonweb/unison/pull/5224 ready for merge into this one (or directly into trunk if you so wish) 😊 - please have a look at the description as there's a couple of things I'm unsure regarding updating a few transcripts and the jit tests potentially being cached but still requiring a couple of small updates - hoping you'll know more!