-
Notifications
You must be signed in to change notification settings - Fork 273
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
Merge newest trunk in "Upgrade to LTS 22.26 GHC 9.6.5" #5224
Conversation
This `only-tools-nixpkgs` devShell generally paralleled the `cabal-only-tools` devShell, but avoiding haskell.nix. While I’m not a huge fan of haskell.nix, this just created duplication and gave us a shell with a somewhat different environment than the one used by `nix build`, etc. It also didn’t work for everyone. In removing that shell, it also sets the default devShell to be `cabal-only-tools`, which some people were already using to work around issues with the previous default.
There are various benefits to using a Nixpkgs release - more likely that things are cached - easier to update the input without breaking everything This also renames a lot of things in the flake: - `nixpkgs-unstable` to `nixpkgs-release` – partially because it’s not unstable any more, but also because both it and the nixpkgs from haskell.nix unstable, so it didn’t really clarify anything - `nixpkgs` to `nixpkgs-haskellNix` – to make it clear where it comes from - `unstable` to `release-pkgs` – the convention is to use `pkgs` for derivation attrsets, and the source switched from unstable to release - `nixpkgs-packages` to `tool-pkgs` – this holds our build tools, so that seemed clearer than “nixpkgs”
Overlays are for derivations, and this isn’t one. Putting it in an overlay also just gives us more levels of indirection to dig through to figure out where things are coming from.
`cabal-local` no longer triggers rebuilds of GHC, so now we can use the devShell that provides the same environment as our build.
This means those environments will also be cached in CI.
Bumped some versions around, but they’re negotiable. - ormolu 0.5.2.0 → 0.7.2.0 - hls 2.9.0.0 → 2.8.0.0 - stack 2.15.5 → 2.15.7
cc891c8
to
456b8e6
Compare
I swapped the merge target to trunk which makes it much easier to review, I think if we just merge this one it should automatically close #5142 and sort everything out 👍🏼 View just the new changes here: https://github.com/unisonweb/unison/pull/5224/files/ecf5fe1df7e16c17f065fdf8544427ed56d69449..4e44b94ad63496657300d05a37a784445adb1094 |
We're only supposed to use cached jit results when they would be the same anyway, but it's been hard to get right, so perhaps there's something wrong with the caching. Do you think something had passed when it should have failed, or do you think it's currently passing when it should be failing? |
This passes CI, so I suppose it’s ok, but I think the solver pulled a new version of cmark, which is now used for transcript parsing/printing. cmark-hs 0.6.1 (released July 8 and called 0.7 in the changelog?) upgrades from libcmark 0.29.0 to 0.30.3 and libcmark 0.30.0 (within that range) now outputs all code blocks as fenced. I’m surprised the Stack grabbed the newer release – I thought we would have had to upgrade to LTS 22.27 (or something) to get cmark-hs 0.6.1. But I guess not. I suppose Stackage bumps minor releases in its snapshots? |
It's currently passing when it should be failing. I've raised #5236 to highlight the issue & offer a potential fix/workaround but I'm not sure how good of a fix it is. Let me know your thoughts! |
Overview
Due to how github displays changes, this PR shows like it contains more changes that it actually has. It shows all the new
trunk
changes since we're merging into an older branch. The actual changes are tiny (and you can see at the end of the commit list):Functor
derives 456b8e6Note: We can also to merge this directly into trunk if people are happy with it.
Loose ends
As mentioned, not sure why the transcripts had to be updated
Although the CI tests pass, I had a previous PR in which CI was failing after the above changes: https://github.com/neduard/unison/actions/runs/9918850847/job/27404234514
As such, I had to make two additional changes:
I think the tests now pass because we're caching the jit tests?