-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
mkDerivation: Introduce .inputDerivation for shell.nix build convenience #95536
Conversation
_derivation_original_builder = derivationArg.builder; | ||
_derivation_original_args = derivationArg.args; | ||
builder = stdenv.shell; | ||
args = [ "-c" " export > $out" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be nice for those reading the code-base/pr to explain this line.
export or env (my preferred) when called with 0 arguments dumps out all current environment variables.
The end result of this derivation is a file whose contents are the environment variables.
nix-shell works mainly by placing all buildInputs into the subshell's $PATH variable.
They are therefore present in plaintext in the file and can then be found by nix when grepping for runtime dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I chose export
because it's a bash builtin. I added an explanation of how it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I'm still somewhat new to Nix ~6months (non-NixOS user); and a big fan of comments with rationale :)
369225e
to
e841464
Compare
@infinisil can you elaborate on
Are you referring to derivations that produce multiple outputs ? It's not clear what you mean by "not being handled separately"; can you explain it in turns of the graph-closures? |
e841464
to
eb8e69c
Compare
@fzakaria Here's an example I made up, with import ./. {};
let
dep = runCommand "dep" {
outputs = [ "out" "bin" ];
} ''
mkdir -p $out $bin/bin
echo out > $out/out
echo bin > $bin/bin/bin
'';
in
stdenv.mkDerivation {
name = "test";
buildInputs = [ dep.out ];
} Then the
|
This introduces the .inputDerivation attribute on all derivations created with mkDerivation. This is another derivation that can always build successfully and whose runtime dependencies are the build time dependencies of the original derivation. This allows easy building and distributing of all derivations needed to enter a nix-shell with nix-build shell.nix -A inputDerivation
eb8e69c
to
3e1a40d
Compare
I checked what it would produce from the example given in my blog post let nixpkgs = import <nixpkgs> {};
in
with nixpkgs;
with stdenv;
with stdenv.lib;
mkShell {
name = "example-shell";
buildInputs = [chromium];
} $ nix-store -qR $(nix-build --no-out-link -I nixpkgs=/home/fmzakari/code/github.com/NixOS/nixpkgs shell.nix -A inputDerivation) | wc -l
> 218 218 is even smaller than 221 than from my blog post; (unsure if it's due to changes to the closure from HEAD on nixpkgs) 👍 So this looks good to me for a solution; although I really dislike the interface / discoverability. Can we include a friendlier CLI param for Also 👍 would be a matching PR in https://github.com/NixOS/nix to augment the documentation with this new option. |
Discoverability isn't great right now yeah, but I'm thinking we could let this change cook a bit in nixpkgs to see how well it works before introducing any |
Sounds good to me and the PR looks good. Let's open a corresponding Nix issue to track and reference once merged. Thanks for the contribution. |
Let's get this 👶 merged in. |
It would be nice for @Ericson2314 or @matthewbauer to review this, or maybe @edolstra also has something to say regarding this. I'd say let's wait a couple days for that |
Is this not already covered in a more precise way by doing:
You can than upload the derivation path instead of inputDerivation.
or
|
@Mic92 Nah that's something different. What you're proposing seems to be just to with import <nixpkgs> {};
stdenv.mkDerivation {
name = "test";
unpackPhase = ":";
dontBuild = true;
buildInputs = [ hello ];
installPhase = "touch $out";
} Building that and looking at the things it depends on gives you: $ nix-store -qR $(nix-build shell.nix)
/nix/store/qflg8wf892kgvxnn1s5bashrc6g1vc1y-test So it doesn't depend on anything, consequently Compare that with $ nix-store -qR $(nix-build default.nix -A inputDerivation)
/nix/store/r2nywq3ziag55zi6dqcxkpb6yla044kq-libunistring-0.9.10
/nix/store/arb8311fjm1dsrbsy8j7pdanwnz1qwxv-libidn2-2.3.0
/nix/store/mh78fk3x12q2a77srgkzv16h0irl8r61-glibc-2.31
[...]
/nix/store/9krlzvny65gdc8s7kpb6lkx8cd02c25b-default-builder.sh
/nix/store/ab1pfk338f6gzpglsirxhvji4g9w558i-hello-2.10
/nix/store/lk7r5rifwz6hhfsc0xqj88gg4ggnrxvx-test This contains |
The following works for me: { pkgs ? import <nixpkgs> {} }:
with pkgs;
mkShell {
buildInputs = [ hello ];
} $ nix-store -qR $(nix eval --raw --impure --expr '((import ./shell.nix {}).drvPath)')
/nix/store/01n3wxxw29wj2pkjqimmmjzv7pihzmd7-which-2.21.tar.gz.drv
/nix/store/03f77phmfdmsbfpcc6mspjfff3yc9fdj-setup-hook.sh
/nix/store/064jmylcq7h6fa5asg0rna9awcqz3765-0001-x86-Properly-merge-GNU_PROPERTY_X86_ISA_1_USED.patch
/nix/store/b7irlwi2wjlx5aj1dghx4c8k3ax6m56q-busybox.drv
/nix/store/bzq60ip2z5xgi7jk6jgdw8cngfiwjrcm-bootstrap-tools.tar.xz.drv
/nix/store/wzdwpgqf2384hr2npma78mqillg5lv08-unpack-bootstrap-tools.sh
/nix/store/0zhkga32apid60mm7nh92z2970im5837-bootstrap-tools.drv
/nix/store/2adm9mf4gs34vbn1js65v90yq6v9i12p-attr-2.4.48.tar.gz.drv
/nix/store/34vpnpik7b1fb91gwv8pfw7s7v8yir01-gettext-0.21.tar.gz.drv
/nix/store/9krlzvny65gdc8s7kpb6lkx8cd02c25b-default-builder.sh
...
/nix/store/xdvhdjkpbdikmyarlhipyzxxlpiw9dra-nix-shell.drv The derivation file always depends on all build inputs. |
@Mic92 that's actually still incorrect & also very unfriendly (not very developer ergonomic) to boot! let nixpkgs = import <nixpkgs> {};
in
with nixpkgs;
with stdenv;
with stdenv.lib;
mkShell {
name = "example-shell";
buildInputs = [];
TEST="${chromium}";
} Ideally, the closure should only be what is required by chromium $ nix-store -qR $(nix-build --no-out-link -I nixpkgs=/home/fmzakari/code/github.com/NixOS/nixpkgs shell.nix -A inputDerivation) | wc -l
> 218 If however I run your sample, I get 1656 packages $ nix repl
nix-repl> chromium = (import ./shell.nix).drvPath
nix-repl> :p chromium
"/nix/store/2vfizyrcd0i6f9x6faz5j7grq255bxrx-example-shell.drv"
$ nix-store -qR /nix/store/2vfizyrcd0i6f9x6faz5j7grq255bxrx-example-shell.drv | wc -l
1656 |
Yes. The drv file seems to capture more dependencies than |
@Mic92 I think the difference which I noted in the blog post; is that when doing performing a This is actually not the desired outcome when caching the buildInputs; rather we want the immediate build-time dependencies of our derivation (mkShell) but for each dependency, only include their run-time dependencies. |
Ok. Thanks for clarification. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-export-nix-shell-dependency-for-deploy/6539/3 |
I'd say let's merge this for now, as it will be pretty useful for many people, and it's a very simple addition. Any objections @Mic92 or anybody else? |
no. |
The name |
I think it's justified for this case, because the alternative of it being something like
I see this
I'm open to a better name. I chose
Any other/better ideas? |
I'm also happy to contribute to documentation for the new attribute unless the plan is to make progress on something more concrete. Should A really clean user experience would be to allow the following and have it return a dummy output maybe.
|
@fzakaria Having docs for this would be awesome!
This would be pretty inconsistent, because you'd have to decide between
This can't work because |
@infinisil ah; I guess I meant instead of mkShell failing to build (through it's custom builder); have it just build I'll work on docs. |
@infinisil i started with a wiki entry for now |
By the way, how does this differ from the derivation built by
This derivation is constructed from the original derivation here: https://github.com/NixOS/nix/blob/94a043ff3b0e9de92bdb62372f9a82186d8e871c/src/nix/develop.cc#L111-L143 You can subsequently enter a saved shell by doing |
@edolstra i'm not on beta or don't have According to the comment to the line you linked /* Given an existing derivation, return the shell environment as
initialised by stdenv's setup script. We do this by building a
modified derivation with the same dependencies and nearly the same
initial environment variables, that just writes the resulting
environment to a file and exits. */ It might be similar since it's ultimately "building" the derivation; whereas the mkShell fails to build (on purpose). I think @infinisil approach is quite elegant; my only concern is documentation and exposing it through a more friendly UI in the CLI. |
Is this safe to use? I don't see a garbage collection root created for |
I thought it was registered as a GC root but this turned out not to be the case. I've fixed this (NixOS/nix@b07167b). |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/trick-to-utilize-binary-cache-in-flake-devshell/10672/3 |
I found this very useful to cache developer environments. We use I am using now: {
python-env = pkgs.mkShell {
buildInputs = [
(python.withPackages (ps: with ps; [
numpy
scipy
# ... other stuff
]))
exiftool
];
};
python-env-noenv = python-env.inputDerivation;
} This allows me to nix-shell -A python-env to enter the shell, and nix-build -A python-env-noenv --out-link my-gc-root to do the CI and the GC root. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/getting-unique-id-from-nix-expression-to-avoid-redundant-build/11716/3 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/devshell-inputderivation-flake-example/12878/1 |
An interesting further use case for this is discovering why a certain derivation is a build input:
In comparison to
|
When I designed `mkShell`, I didn't have a good idea of what the output should look like and so decided to make the build fail. In practice, this causes quite a bit of confusion and complications because now the shell cannot be part of a normal package set without failing the CI as well. This commit changes that build phase to record all the build inputs in a file. That way it becomes possible to build it, makes sure that all the build inputs get built as well, and also can be used as a GC root. (by applying the same trick as NixOS#95536). The documentation has also been improved to better describe what mkShell does and how to use it.
When I designed `mkShell`, I didn't have a good idea of what the output should look like and so decided to make the build fail. In practice, this causes quite a bit of confusion and complications because now the shell cannot be part of a normal package set without failing the CI as well. This commit changes that build phase to record all the build inputs in a file. That way it becomes possible to build it, makes sure that all the build inputs get built as well, and also can be used as a GC root. (by applying the same trick as #95536). The documentation has also been improved to better describe what mkShell does and how to use it.
This attribute is very useful, but due to the way it's implemented it misses some dependencies. |
@rnhmjoj I believe that's part of this bigger problem: NixOS/nix#719. Also it depends on whether |
Same rationale as upstream, see NixOS/nixpkgs#95536
Same rationale as upstream, see NixOS/nixpkgs#95536
This allows easy building and distributing of all derivations needed to
enter a nix-shell with
Or with cachix:
In more technical terms: This introduces the
.inputDerivation
attribute on all derivations created withmkDerivation
. This is another derivation that can always build successfully and whose runtime dependencies are the build time dependencies of the original derivation.Motivation for this change
The recent blog post by @fzakaria (https://fzakaria.com/2020/08/11/caching-your-nix-shell.html) mentions:
However after testing this I noticed that this caches more than needed due to multiple outputs not being handled separately. Since I had such an
inputDerivation
in the repo, I was able to compare it directly: Above command resulted in 653 paths, whereasinputDerivation
only needed 475.So I'm now upstreaming this such that it can be used by everybody. During my couple years in the #nixos IRC channel this kind of functionality was requested more than once, and with this PR there's finally a good builtin solution for it.
Ping @grahamc who once shared an original version of this
Ping @Ericson2314 @matthewbauer as people who are often involved with
make-derivation.nix
And ping @zupo from Niteo, who sponsored this PR :)
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)