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

Jump to bottom

Add a callHackage variant that doesn't require all-cabal-hashes #52848

Merged
merged 2 commits into from Jan 27, 2019
Merged

Add a callHackage variant that doesn't require all-cabal-hashes #52848

merged 2 commits into from Jan 27, 2019

Conversation

mightybyte
Copy link
Contributor

Motivation for this change

Because callHackage uses all-cabal-hashes to avoid requiring the user to specify a hash, it does not work for any package versions that have been uploaded after the version of all-cabal-hashes that you happen to have. In my experience this means that probably greater than half the time callHackage doesn't work for me, and I have to fall back to fetchFromGitHub. This is a pretty poor user experience and is also less reliable because github packages can be deleted, but hackage releases cannot.

This function works for any package that has been released on hackage. I have been using it recently in some of my production projects and it has been working fine. It would be really nice to have it available in nixpkgs.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cdepillabout
cdepillabout approved these changes Dec 26, 2018
View reviewed changes
Copy link
Member

@cdepillabout cdepillabout left a comment

Choose a reason for hiding this comment

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

This looks good to me.

I've wanted a function like this before. It'll be great to have it in nixpkgs.

shlevy
shlevy approved these changes Dec 26, 2018
View reviewed changes
Copy link
Member

@shlevy shlevy left a comment

Choose a reason for hiding this comment

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

LGTM other than @puffnfresh's comment.

@mightybyte
Copy link
Contributor Author

I responded to @puffnfresh above. Also, regarding the name...I'm definitely not stuck on this name. I'm not crazy about callHackage' though, because I tend to think primes aren't visually distinct enough. Open to better suggestions here.

@mightybyte
Copy link
Contributor Author

I've tested this and verified that it works with a real external package. Should be ready to merge now.

@shlevy shlevy merged commit 15e8d1d into NixOS:master Jan 27, 2019
@ElvishJerricco
Copy link
Contributor

This should probably have some kind of support for revisions, like a revision number arg and a cabal file sha arg.

@ElvishJerricco
Copy link
Contributor

It'd also be really cool to have some way to use this from packageSourceOverrides

@mightybyte
Copy link
Contributor Author

@ElvishJerricco Agreed, although that requires a bit more work because you have to download the tarball and replace the cabal file inside it with the one for the revision. In practice, the lack of revision support hasn't bothered me too much yet because revisions can only make changes to metadata and that can usually be accomplished with doJailbreak. There may be places where this isn't sufficient and proper revision support is needed, but it's been good enough for me thus far.

@ElvishJerricco ElvishJerricco mentioned this pull request Jan 31, 2019
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@puffnfresh puffnfresh puffnfresh left review comments

@rimmington rimmington rimmington left review comments

@shlevy shlevy shlevy approved these changes

@cdepillabout cdepillabout cdepillabout approved these changes

@basvandijk basvandijk basvandijk approved these changes

@peti peti Awaiting requested review from peti

@ryantm ryantm Awaiting requested review from ryantm

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@mightybyte @ElvishJerricco @puffnfresh @cdepillabout @rimmington @shlevy @basvandijk @GrahamcOfBorg