Fix import and non-canonical instance warnings #382
Conversation
| import Prettyprinter | ||
|
|
There was a problem hiding this comment.
I think, so far, we used the old style import because @pepeiborra supports, unofficially, HLS for a GHC 8.8.4 stackage snapshot, which only has the old prettyprinter version and doesn't have the new imports. Updating this breaks HLS on a GHC 8.8.4. @pepeiborra Is that still relevant?
|
No worries! |
|
@fendor switching to different versions of GHC to test that the version bounds work correctly would probably go faster with nix so I'm going to set that up to save myself some time. I see that an issue #25 is asking for Nix support and a Asking ahead of time because I know not everyone is alright with Nix coinfecting repositories so if thats the case here no worries. |
While I am a nix-user myself, I am a fair bit scared off from maintaining a nix CI. For example, in HLS, nix CI is constantly outdated, which is why we decided to make it a non-blocking CI job... Rendering it relatively useless for nix developers again. So, if you want to "commit" to maintaining the nix CI, I am happy about the contribution, but I don't want to maintain it, unless it is really trivial. |
|
Ah, in that case I won't worry about adding Nix in a separate PR (I don't have space to commit to maintaining anything at the moment). Out of curiosity though, what do you mean by Nix CI is constantly out of date? I'm not sure I've run into that problem myself. If you could elaborate or possibly link me to an example of an issue that demonstrates the problem I would really appreciate it. I maintain some packages that use Nix CI so knowing about potential pitfalls would be very helpful. |
|
Nix is a time sink. Cache misses that lead to compile failures are not uncommon, despite the claim that it's reproducible. The ecosystem constantly changes its ways on how to do things, so when you update your recipes, you're likely investing a lot of time again. |
|
I'm not sure how a cache miss could lead to a compile failure unless the Haskell package was just not portable? Do you have a specific example of this happening in mind? |
Because if you don't hit the cache, you'll rebuild the entire universe. I've seen build failure in CI numerous times then. I'm not going to dig those out to make a point though. |
|
Thats fine I just wanted to make sure that your point was not related to Nix. |
It duplicates the information from .cabal files since nix can't nixify a cabal project automatically, so we have lots of logic for it: https://github.com/haskell/haskell-language-server/blob/master/flake.nix Also, since it doesn't use the build-plan cabal found, but rather follows a stackage snapshot, you have to customarily override versions, jailbreak, not test, etc... None of this applies to hie-bios directly, it is likely just 20 lines of nix, but imo nix expressions need to be maintained either way, just as nix CI, upgrading nixpkgs pins, etc... |
|
If you rebase, this can be merged. My apologies, I forgot about the PR 😅 |
|
Closing as all of these fixes were done as part of #429 |
This PR has a few changes that fix GHC warnings when building
lib:hie-biosandexe:hie-bios:Removes
Data.Text.Prettyprint.Docimports in favor of thePrettyprintermodule. TheData.Text.Prettyprint.Docmodule has been deprecated sinceprettyprinter-1.7.0.Removes any redundant imports reported by
-Wunused-imports.Encloses the non-canonical
returndefinition specified forCradleLoadResultTinMIN_VERSION_base(4,8,0), fixing the-Wnoncanonical-monad-instanceswarning.I expect some of the
prettyprinterand redundant imports are actually necessary for backward compatibility. I'm happy to add the correct#ifdirectives to silence these warning if that is the case.