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

haskell: Add hoogle option to developPackage #103061

Merged
merged 3 commits into from Nov 13, 2020

Conversation

@expipiplus1
Copy link
Contributor

@expipiplus1 expipiplus1 commented Nov 7, 2020

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.
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 10, 2020

name hoogle -> withHoogle for consistency with other makeShell

Copy link
Member

@cdepillabout cdepillabout left a comment

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.

lib/fixed-points.nix Outdated Show resolved Hide resolved
lib/tests/misc.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/make-package-set.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/make-package-set.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/make-package-set.nix Outdated Show resolved Hide resolved
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 11, 2020

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

@cdepillabout cdepillabout commented Nov 11, 2020

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 expipiplus1 force-pushed the expipiplus1:joe-hoogle branch from 138bf04 to f9883ba Nov 11, 2020
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 11, 2020

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

Copy link
Member

@cdepillabout cdepillabout left a comment

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).

lib/tests/misc.nix Outdated Show resolved Hide resolved
@expipiplus1 expipiplus1 force-pushed the expipiplus1:joe-hoogle branch from f9883ba to d76509f Nov 11, 2020
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 11, 2020

Will do, thanks

@expipiplus1 expipiplus1 force-pushed the expipiplus1:joe-hoogle branch from d76509f to e6af99a Nov 12, 2020
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 13, 2020

@expipiplus1 expipiplus1 force-pushed the expipiplus1:joe-hoogle branch from e6af99a to 6586a9c Nov 13, 2020
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 13, 2020

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
2 of 4 checks passed
2 of 4 checks passed
tests
Details
action
Details
Wait for ofborg This failed status will be cleared when ofborg finishes eval.
Details
grahamcofborg-eval The branch this PR will merge in to does not cleanly evaluate, and so this PR cannot be checked.
Details
@expipiplus1
Copy link
Contributor Author

@expipiplus1 expipiplus1 commented Nov 13, 2020

Thanks!

@expipiplus1 expipiplus1 deleted the expipiplus1:joe-hoogle branch Nov 13, 2020
@FRidh
Copy link
Member

@FRidh 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants