Skip to content
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

haskellPackages.binary-search: unbreak and patch #104426

Closed
wants to merge 1 commit into from

Conversation

LaloHao
Copy link

@LaloHao LaloHao commented Nov 20, 2020

Motivation for this change

Fix broken haskell package binary-search

This was wrongly managed in another PR i closed for being badly formatted and merging wrong updates: #98196

I rebased from NixOS/nixpkgs#haskell-updates and created a new/clean branch for better organization

I just have one question, is this the right branch to merge? It is a haskell package

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@@ -261,7 +261,7 @@ self: super: {
});
aws-kinesis = dontCheck super.aws-kinesis; # needs aws credentials for testing
binary-protocol = dontCheck super.binary-protocol; # http://hydra.cryp.to/build/499749/log/raw
binary-search = dontCheck super.binary-search;
binary-search = dontCheck (appendPatch super.binary-search ./patches/binary-search-fix-for-ghc-8.patch);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we try not to carry around patches here in nixpkgs. Would it be possible to pull this patch from upstream using fetchpatch? There should be examples of using fetchpatch in this file.

And of course reporting this problem upstream would help the rest of the Haskell community as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, i will do that, thanks for the suggestion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any progress?

@peti peti force-pushed the haskell-updates branch 3 times, most recently from bb792f3 to 855c7fb Compare November 27, 2020 20:16
@peti
Copy link
Member

peti commented Dec 4, 2020

Ping? Will you address cdepillabout's comments?

@peti peti force-pushed the haskell-updates branch 3 times, most recently from ed51033 to 8e2abd0 Compare December 4, 2020 20:20
@LaloHao
Copy link
Author

LaloHao commented Dec 7, 2020

I ended up adding compiler flags instead of overriding the whole main, it seems so keep working for old ghc versions + the new ones who used to break
https://github.com/LaloHao/binary-search/tree/fix/ghc882

But there seems to be a a request already on the original repo nushio3/binary-search#3, from September 29

Should we wait?

@peti peti force-pushed the haskell-updates branch 3 times, most recently from 2c7ad11 to 0e5f7e8 Compare December 18, 2020 19:28
@peti peti force-pushed the haskell-updates branch 3 times, most recently from ba32886 to c9e5a3f Compare January 2, 2021 18:58
@peti peti force-pushed the haskell-updates branch 2 times, most recently from a0a57d7 to 952ebcf Compare January 22, 2021 19:34
@jappeace
Copy link
Contributor

@LaloHao I'm trying to take over maintainership on hackage.
I think I'll contact the maintainers of hackage within 2 weeks because we've waited long enough.

@peti peti force-pushed the haskell-updates branch 2 times, most recently from 56fa6fc to f3cb253 Compare February 19, 2021 19:56
@peti
Copy link
Member

peti commented Feb 19, 2021

Any news?

@jappeace
Copy link
Contributor

yes, I just became maintainer yesterday, i'll do a release on monday

@jappeace
Copy link
Contributor

I uploaded a new realase, I think this should be fixed next Friday.

@sternenseemann
Copy link
Member

No longer an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants