Skip to content

haskell: Add hoogle option to developPackage #103061

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.

Sign up for GitHub

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
Merged
merged 3 commits into from
Nov 13, 2020
Merged

haskell: Add hoogle option to developPackage #103061

merged 3 commits into from
Nov 13, 2020
+51 −5

Conversation

expipiplus1
Copy link
Contributor

Also add composeExtensionss to lib.

This seems to be a sensible way of getting ghcWithHoogle into the mix.

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.

Sorry, something went wrong.

@ofborg ofborg bot added 6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 7, 2020
@expipiplus1
Copy link
Contributor Author

name hoogle -> withHoogle for consistency with other makeShell

cdepillabout
cdepillabout requested changes Nov 10, 2020
View reviewed changes
Copy link
Member

@cdepillabout cdepillabout left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left a few comments.

Also, it would be nice if you wrote some documentation for end users who want to use Hoogle. Could you add a sentence or two for what an end user would be able to do by enabling the withHoogle option? I'm guessing the idea is that they now get a hoogle in their path with a database of all dependencies?

It'd also be nice to have a test for developPackage (similar to the test for shellFor), but that's more of a wish-list type thing. Not having a test shouldn't block this from being merged in.

@expipiplus1
Copy link
Contributor Author

It'd also be nice to have a test for developPackage (similar to the test for shellFor), but that's more of a wish-list type thing. Not having a test shouldn't block this from being merged in.

Such a test would involve import-from-derivation, is that permissible in nixpkgs?

@cdepillabout
Copy link
Member

Such a test would involve import-from-derivation, is that permissible in nixpkgs?

Ah, good call.

I guess you could always just mark it __noChroot and we could use it as a test that we run manually, but it would be unfortunate that we couldn't have it run by Hydra.

@expipiplus1
Copy link
Contributor Author

Such a test would involve import-from-derivation, is that permissible in nixpkgs?

Ah, good call.

I guess you could always just mark it __noChroot and we could use it as a test that we run manually, but it would be unfortunate that we couldn't have it run by Hydra.

Right, perhaps best for another PR

cdepillabout
cdepillabout approved these changes Nov 11, 2020
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.

Aside from the one remaining comment, I think this looks really good.

I'm going to leave this open for a few days, so that any else that got notified has a chance to comment on the change in lib/.

@expipiplus1 If there are no additional comments to this in a few days, please feel free to ping me and I will merge it in (assuming the test thing has been fixed).

@expipiplus1
Copy link
Contributor Author

Will do, thanks

@expipiplus1
Copy link
Contributor Author

@cdepillabout ping!

@expipiplus1
lib: Add composeManyExtensions

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
29dc127
@expipiplus1
haskell: improve documentation on ghcWithHoogle and developPackage

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
be91d93
@expipiplus1
haskell: add withHoogle option to developPackage

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
6586a9c
@expipiplus1
Copy link
Contributor Author

I don't think that @ofborg's failure is to do with this PR

@cdepillabout cdepillabout merged commit 57c9485 into NixOS:haskell-updates Nov 13, 2020
@expipiplus1
Copy link
Contributor Author

Thanks!

@expipiplus1 expipiplus1 deleted the joe-hoogle branch November 13, 2020 04:40
@FRidh FRidh mentioned this pull request Dec 19, 2020
Closed
@FRidh
Copy link
Member

FRidh commented Dec 19, 2020

composeManyExtensions was proposed in #33258.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout

@edolstra edolstra

@infinisil infinisil

@nbp nbp

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@expipiplus1 @cdepillabout @FRidh