-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
[20.09] haskellPackages.hakyll: fix build #97563
Conversation
The PR has not been merged, so the referenced commits may disappear when the PR is rebased. |
Mh right, any idea how I could circumvent this? |
else overrideCabal super.hakyll { | ||
patches = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this only applies the patches if built on non-darwin? Is this logic correct?
patches = [ | ||
(pkgs.fetchpatch { | ||
url = "https://github.com/jaspervdj/hakyll/pull/787/commits/b71955f7db1fd6532d3988be671281d1f6fcdd1d.patch"; | ||
sha256 = "Q9dFJpd9vmV1StjBpscDymprdF0Cdfil/tHD6c/ejBs="; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These sha256 hashes look strange? Are they base64 encoded or something?
I didn't even realize this was legal.
@@ -144,7 +144,22 @@ self: super: { | |||
then dontCheck (overrideCabal super.hakyll (drv: { | |||
testToolDepends = []; | |||
})) | |||
else super.hakyll; | |||
else overrideCabal super.hakyll { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After fixing the above two problems, can you add a comment here that links to the PR containing these patches and explains why they are necessary?
Okay, let's stop this here. I'm not willing to put any more energy into pushing such a minor change forward. The amount of bikeshedding I've encountered in the Haskell part of Nixpkgs is just too high for me as a low-volume contributor. Somehow this is really haskellPackages specific for me. For the last few months I consistently regretted it to have opened a PR to haskellPackages. That is not the case for any other part of Nixpkgs. I'm not saying that the individual nitpicks in this PR and other PRs I opened aren't justified. It's just that I fail to see how meeting these high code standards stands in any relation to the utility of bringing these changes upstream. Most of the time when Hakyll broke in the last few years, there was no visible reaction in the NixOS community. This tells me, that only a few people use it. Why go through so many review iterations for this little gain? I'm perfectly fine with just rebasing my changes ontop of my own branch, without bringing them upstream. Considering that bringing my changes upstream usually takes an order of magnitude more time than just fixing it for myself, it just doesn't add up to me anymore. I think I'll still open PRs to haskellPackages, but close them immediately, so other people in need of a fix can at least find and cherry-pick the solution. |
@erictapen I assume you're talking about me, and I do appologize that you haven't had a good experience contributing to the Haskell part of Nixpkgs. Let me try to explain some of my thinking so you can see where we're coming from. The Haskell part of Nixpkgs has few regular contributors (only three of us). However, there is a relatively high volume of drive-by contributions. These are often one-off contributions by people that don't "follow-up" on stuff they get merged in. So if the stuff they submit becomes broken, it is up to the three regular maintainers to either fix it up or tear it out. Without ample comments (for example above, why were certain patches applied to hakyll, and why were they only applied when building with Linux), it is pretty challenging for us to continue to maintain all the overrides in A lot of the work of fixing up Haskell stuff falls on @peti (since he does the weekly updates), so my main goal over the past year has been to make sure new contributions are well documented, and fit the normal style for the Haskell stuff. This lets the other maintainers (as well as new users) easily figure out the reason for the various overrides we apply here in nixpkgs.
Thanks for doing this! I think a lot of people search for old PRs before creating new ones, so this will hopefully save some people from figuring out everything from scratch. |
Hakyll builds just fine in LTS-16.12, which Nixpkgs is based on. It seems to have been broken by 05bf532. I would expect that updates on top of stackage are opt-in. Doing it that way increases the chance that stackage builds successfully, which has most of the important packages. |
@cdepillabout Thank you for taking the time to elaborate on your perspective. First I'd like to make clear, that my rant wasn't about your reviews specifically, but more about a broader "the way thinks work" I noticed for quite some time in haskellPackages. Second I now understand better, that the high ratio of drive-by contributions vs. long-term maintainers is a situation with high incentives to avoid maintainenance debt. Maybe there really is no way around the high standards currently. I think I'll take a break to bringing my commits through review, until I have more time/energy to do it. |
Motivation for this change
Things done
I appended the commits from jaspervdj/hakyll#787.
Edit: This also fixes
hakyll-contrib-hyphenation
, so I marked it as unbroken.sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)