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

Improve composability of mkShell #63701

Merged
merged 2 commits into from Jun 23, 2019

Conversation

basvandijk
Copy link
Member

Motivation for this change

This improves the composability of mkShell in two ways:

  • shellHook is now also composed. For example running the following expression with nix-shell:
let
  pkgs = import <nixpkgs> {};

  shell1 = pkgs.mkShell {
    shellHook = ''
      echo shell1
    '';
  };

  shell2 = pkgs.mkShell {
    shellHook = ''
      echo shell2
    '';
  };

  shell3 = pkgs.mkShell {
    inputsFrom = [ shell1 shell2 ];
    shellHook = ''
      echo shell3
    '';
  };
in shell3

will now result in:

shell2
shell1
shell3

Note that packages in the front of inputsFrom have precedence over packages in the back. The outermost mkShell has precedence over all.

  • mergeInputs will now result in a more sensible order of $PATH. For example running following with nix-shell:
let
  pkgs = import <nixpkgs> {};

  shell1 = pkgs.mkShell {
    buildInputs = [ pkgs.htop ];
  };

  shell2 = pkgs.mkShell {
    buildInputs = [ pkgs.hello ];
  };

  shell3 = pkgs.mkShell {
    inputsFrom = [ shell1 shell2 ];
    buildInputs = [ pkgs.tree ];
  };

in shell3

results in the following $PATH:

$ echo $PATH
...
/nix/store/yifq4bikf7m07160bpia7z48ciqddbfi-tree-1.8.0/bin:
/nix/store/vhxqk81234ivqw1a7j200a1c69k8mywi-htop-2.2.0/bin:
/nix/store/n9vm3m58y1n3rg3mlll17wanc9hln58k-hello-2.10/bin
...

Previously the order was:

/nix/store/n9vm3m58y1n3rg3mlll17wanc9hln58k-hello-2.10/bin
/nix/store/vhxqk81234ivqw1a7j200a1c69k8mywi-htop-2.2.0/bin:
/nix/store/yifq4bikf7m07160bpia7z48ciqddbfi-tree-1.8.0/bin:

The new order makes more sense because it allows to override $PATH in the outermost mkShell and this order is consistent with the shellHook precedence.

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 nix-review --run "nix-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.

Running the following expression with nix-shell:

  let
    pkgs = import <nixpkgs> {};

    shell1 = pkgs.mkShell {
      shellHook = ''
        echo shell1
      '';
    };

    shell2 = pkgs.mkShell {
      shellHook = ''
        echo shell2
      '';
    };

    shell3 = pkgs.mkShell {
      inputsFrom = [ shell1 shell2 ];
      shellHook = ''
        echo shell3
      '';
    };
  in shell3

Will now results in:
shell2
shell1
shell3

Note that packages in the front of inputsFrom have precedence over
packages in the back. The outermost mkShell has precedence over all.
mergeInputs is now simply defined in terms of `concatLists` and
`catAttrs` instead of a more complicated `foldr`.

Note that the order of PATH has also changed. For example running the
following with nix-shell:

  let
    pkgs = import <nixpkgs> {};

    shell1 = pkgs.mkShell {
      buildInputs = [ pkgs.htop ];
    };

    shell2 = pkgs.mkShell {
      buildInputs = [ pkgs.hello ];
    };

    shell3 = pkgs.mkShell {
      inputsFrom = [ shell1 shell2 ];
      buildInputs = [ pkgs.tree ];
    };

  in shell3

Results in the following PATH:

$ echo $PATH
...
/nix/store/yifq4bikf7m07160bpia7z48ciqddbfi-tree-1.8.0/bin:
/nix/store/vhxqk81234ivqw1a7j200a1c69k8mywi-htop-2.2.0/bin:
/nix/store/n9vm3m58y1n3rg3mlll17wanc9hln58k-hello-2.10/bin
...

Previously the order was:

/nix/store/n9vm3m58y1n3rg3mlll17wanc9hln58k-hello-2.10/bin
/nix/store/vhxqk81234ivqw1a7j200a1c69k8mywi-htop-2.2.0/bin:
/nix/store/yifq4bikf7m07160bpia7z48ciqddbfi-tree-1.8.0/bin:

I think the new order makes more sense because it allows to override
the PATH in the outermost mkShell.
Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

good idea

@basvandijk
Copy link
Member Author

Note I also backported this to release-19.03 in #63718.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-shell-tips-tricks-and-best-practices/22332/1

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