haskellPackages: fix some builds#397451
Conversation
0bf59b9 to
716b4a4
Compare
maralorn
left a comment
There was a problem hiding this comment.
This is great. I have some general feedback on jailbreaks however.
I know we have regularly been sloppy about this, especially in configuration-common.nix but this is not good (TM).
wdyt?
There was a problem hiding this comment.
Okay, here my short guide to jailbreaking. Our goal has to be that jailbreaks are temporary and can easily be cleaned up. So some or all of the following things are desirable:
- Adding a date to the comment so that we can later quickly see how bitrotten it is.
- Adding a link to an upstream issue. In theory jailbreak is a last ressort, we should really nudge upstream to get there shit together.
- Adding an assert on the current version of the package with the comment, that the jailbreak can probably be dropped on the next release.
I know this increases friction but in the long term it increases maintainability.
There was a problem hiding this comment.
- Adding a date to the comment so that we can later quickly see how bitrotten it is.
My experience from #381006 and #381264 was, that the date was not really helpful. Much more helpful was the exact specification of which dependency exactly needed to be jailbroken.
At the same time, I found many packages, where the original bounds had been fixed, but new bounds were relaxed with the jailbreak in the meantime.. which meant, that I just updated the comments. Kinda pointless.
I did most of the jailbreaking here to get some numbers and my gut feeling was right: There are a lot of packages that just need to be jailbroken to build. I believe we can save some effort by just doing --allow-newer by default, as proposed elsewhere.
Nonetheless: The addition of the date is clearly the easiest thing to do, so no objection.
- Adding a link to an upstream issue. In theory jailbreak is a last ressort, we should really nudge upstream to get there shit together.
This would imply reporting the issue upstream first... which I have not done, so far, because... my theory is different, as written above. I think jailbreaking should not be a last ressort, it should be the default.
That is, if our interest here is the maintenance of the nixpkgs package set.
Checking those bounds and reporting them upstream is mostly a thing to help the greater haskell eco-system, not a nixpkgs-thing. That being said: Even if we jailbreak by default, we might still be able to generate some reports which list all kind of packages which built fine despite using newer packages than their bounds allow. We could still create issues upstream in the sense of "package X builds fine with dependency Y in version Z in nixpkgs, consider bumping".
- Adding an assert on the current version of the package with the comment, that the jailbreak can probably be dropped on the next release.
Ah, I had not seen that style before. Just looked it up, yeah seems sensible.
TLDR: I am not opposed to any of the 3, but I'd really first like to discuss the whole "allow newer by default" proposal. I think it could save us quite some effort.
There was a problem hiding this comment.
(added dates and asserts already, as that was fairly easy)
There was a problem hiding this comment.
Fair!
The problem with allow-newer is:
- technically we currently have no way to do this without also doing allow-older. jailbreak is a blunt hammer. I have been procrastinating finishing Move --allow-{newer,older} back to ./Setup.hs haskell/cabal#9016 for years at this point.
- it is just wrong … sometimes library authors really keep upper bounds on purpose. Libs can have breaking changes without breaking compilation. There have been rare cases of jailbreaks introducing runtime bugs in peoples production. Sometimes when we open an issue at upstream about a bound we get the feedback that it can’t be raised, which is another strong reason to actually open those issues. I totally understand the appeal and maybe this is hypocritical because we are never actually this thorough when we manually jailbreak, but this is and has been the reason until now.
There was a problem hiding this comment.
Okay, here my short guide to jailbreaking. Our goal has to be that jailbreaks are temporary and can easily be cleaned up. So some or all of the following things are desirable:
- Adding a date to the comment so that we can later quickly see how bitrotten it is.
- Adding a link to an upstream issue. In theory jailbreak is a last ressort, we should really nudge upstream to get there shit together.
- Adding an assert on the current version of the package with the comment, that the jailbreak can probably be dropped on the next release.
I know this increases friction but in the long term it increases maintainability.
While dates are better than nothing, there's the raised issue of them being a poor man's git blame as the override gets edited over time. On the other hand they survive unrelated formatting passes.
Personally I don't think they're worth the noise for the things which we can automatically detect, a-la #384591 but might be useful for more specific situations.
Even the assert I would only add if there was already an upstream PR open/merged that would remove the need for the override.
I do agree with the upstream issue point, and haskellPackages.krank can probably increase the payoff with some massaging. Running it on configuration-common.nix directly quickly starts hitting rate limits
jailbreak doesn't help, no patches available either.
First, they need some additional dependencies, but then some of them still fail in a weird way. Keeping the test dependencies in place, even if unused, in case somebody figures this out in the future.
No patches available.
Unmaintained since 2018 and failing.
Unmaintained since 2014, no repository.
716b4a4 to
00c178b
Compare
|
Thank you. We can continue having the general discussion. |
Fixes some failing haskell packages and a few as broken, where it doesn't make sense to look into again.
Things done
Add a 👍 reaction to pull requests you find important.