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

Add support for user-extensible shellHook to haskell.lib.buildStackProject #44568

Merged

Conversation

uskudnik
Copy link
Contributor

@uskudnik uskudnik commented Aug 6, 2018

haskell.lib.buildStackProject is overridding shell hook and doesn't append
user-specified shellHook to it, resulting in user's shellHook
never executing.

I believe shellHook should be appended after addStackArgsHook has been executed, but I can see a case for being executed beforehand too so if that is preferred approach let me know and I'll fix it.

Motivation for this change

Ability to add shellHook commands to stack nix environments.

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)
  • Fits CONTRIBUTING.md.

Testing:

# shell.nix

{ pkgs ? import <nixpkgs> {}
}:

pkgs.haskell.lib.buildStackProject {
  inherit ghc;
  name = "stacktest";
  buildInputs = [
    pkgs-master.glpk
    pkgs-master.pcre
  ];
  
  shellHook = ''
    echo "Hello https://github.com/NixOS/nixpkgs!"
    '';

}

Result:

$ cd ~/dev/nixpkgs && git co upstream/master && cd - && NIX_PATH=nixpkgs=/path/to/nixpkgs nix-shell
HEAD is now at 9934f0bb51a AgdaStdlib: 0.15 -> 0.16 (#44550)

[nix-shell:~/dev/haskell-test]$ exit

$ cd ~/dev/nixpkgs && git co add-shellhook-support-to-buildstackproject && cd - && NIX_PATH=nixpkgs=/path/to/nixpkgs nix-shell
Previous HEAD position was 9934f0bb51a AgdaStdlib: 0.15 -> 0.16 (#44550)
Switched to branch 'add-shellhook-support-to-buildstackproject'
Hello https://github.com/NixOS/nixpkgs!

@uskudnik uskudnik changed the title Add support for user-extensible shellHook to buildStackProject Add support for user-extensible shellHook to haskell.lib.buildStackProject Aug 6, 2018
@@ -33,7 +33,7 @@ in stdenv.mkDerivation (args // {
STACK_PLATFORM_VARIANT="nix";
STACK_IN_NIX_SHELL=1;
STACK_IN_NIX_EXTRA_ARGS = extraArgs;
shellHook = addStackArgsHook;
shellHook = addStackArgsHook + args.shellHook;
Copy link
Member

Choose a reason for hiding this comment

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

Will throw an error when shellHook isn't set, so you need + args.shellHook or ""

buildStackProject is overridding shell hook and doesn't append
user-specified shellHook to it, resulting in user's shellHook
never executing.
@uskudnik uskudnik force-pushed the add-shellhook-support-to-buildstackproject branch from ab03d16 to 01a8fa1 Compare August 7, 2018 12:28
@uskudnik
Copy link
Contributor Author

uskudnik commented Aug 7, 2018

@infinisil Ah, nice catch, thanks!

Verified the issue and applied suggested patch, seems to work as intended. If you see any other issues let me know.

@infinisil infinisil merged commit a57c857 into NixOS:master Aug 13, 2018
@uskudnik uskudnik deleted the add-shellhook-support-to-buildstackproject branch August 13, 2018 18:45
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

3 participants