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
Backport "Improve composability of mkShell" to release-19.03 #63718
Backport "Improve composability of mkShell" to release-19.03 #63718
Conversation
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. (cherry picked from commit cee3573)
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. (cherry picked from commit 76ef802)
I'm not sure that this kind of change is suitable for backporting since there is a risk that it introduces a regression. The stable branch is for bug fixes, not new features. |
I actually expected Shall I revert? |
As long as it doesn't break back-compat it doesn't need to be reverted, but I agree with Eelco, it's a new feature |
It broke our setup at least. This is effectively an API change, and not one I would expect to have to deal with during a normal update. :-) Edit: We basically did the composing of multiple hooks ourself, and threw a list of a single string after |
This reverts commit 0004631. #63718 (comment)
This reverts commit e65b6ff. #63718 (comment)
@adamtulinius Thanks for the report. I've reverted this on |
Motivation for this change
Backport of #63701 to
release-19.03
.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)