-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 a post build hook #2995
Add a post build hook #2995
Conversation
src/libstore/build.cc
Outdated
for (auto outputPath: outputPaths) | ||
args.push_front(outputPath); | ||
args.push_front("--"); | ||
args.push_front(drvPath); |
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.
Rather than using command line arguments, it would be nicer to use environment variables (e.g. OUTPUT_PATHS=...
and DRV_PATH=...
) because then the hook doesn't have to parse the command line.
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 used cli arguments because that was the simplest to implement (because the runProgram2
interface doesn't allow to pass extra environment variables, so it requires extending it a bit) and to be more consistent with the pre-build-hook
. But environment variables would make the hooks simpler indeed. I'll change that
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.
(that being said the hook would then have to parse the OUTPUT_PATHS
variable to get back a list. But that's probably already better)
It looks like the test I've added doesn't work inside a @edolstra @grahamc any idea of what could cause that? The file seems to be present in the sandbox but maybe there's something special about the way nix runs in here which prevents it from seeing the file |
Nvm, I was using |
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.
Really excited to see this - some comments :-)
I have some additional patches here around logging the output from the post build hook. @edolstra can we setup a time to do some pair programming to finish it off? |
I gave this a try inside a CI environment, to sign and upload derivations after they're built, and it works just beautifully 😍! We should add a note that this will only run for derivations being actually built, not those that are substituted from a binary cache. In my CI environment, this shouldn't be a problem (as the Do we intend to add a similar hook for substituted ones? |
985bae8
to
74461e9
Compare
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.
Do we intend to add a similar hook for substituted ones?
probably not for now :)
|
||
<itemizedlist> | ||
<listitem><para>The hook executes after an evaluation-time build.</para></listitem> | ||
<listitem><para>The hook does not execute on substituted paths.</para></listitem> |
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.
We should add a note that this will only run for derivations being actually built, not those that are substituted from a binary cache.
Passing `--post-build-hook /foo/bar` to a nix-* command will cause `/foo/bar` to be executed after each build with the following environment variables set: DRV_PATH=/nix/store/drv-that-has-been-built.drv OUT_PATHS=/nix/store/...build /nix/store/...build-bin /nix/store/...build-dev This can be useful in particular to upload all the builded artifacts to the cache (including the ones that don't appear in the runtime closure of the final derivation or are built because of IFD). This new feature prints the stderr/stdout output to the `nix-build` and `nix build` client, and the output is printed in a Nix 2 compatible format: [nix]$ ./inst/bin/nix-build ./test.nix these derivations will be built: /nix/store/ishzj9ni17xq4hgrjvlyjkfvm00b0ch9-my-example-derivation.drv building '/nix/store/ishzj9ni17xq4hgrjvlyjkfvm00b0ch9-my-example-derivation.drv'... hello! bye! running post-build-hook '/home/grahamc/projects/github.com/NixOS/nix/post-hook.sh'... post-build-hook: + sleep 1 post-build-hook: + echo 'Signing paths' /nix/store/qr213vjmibrqwnyp5fw678y7whbkqyny-my-example-derivation post-build-hook: Signing paths /nix/store/qr213vjmibrqwnyp5fw678y7whbkqyny-my-example-derivation post-build-hook: + sleep 1 post-build-hook: + echo 'Uploading paths' /nix/store/qr213vjmibrqwnyp5fw678y7whbkqyny-my-example-derivation post-build-hook: Uploading paths /nix/store/qr213vjmibrqwnyp5fw678y7whbkqyny-my-example-derivation post-build-hook: + sleep 1 post-build-hook: + printf 'very important stuff' /nix/store/qr213vjmibrqwnyp5fw678y7whbkqyny-my-example-derivation [nix-shell:~/projects/github.com/NixOS/nix]$ ./inst/bin/nix build -L -f ./test.nix my-example-derivation> hello! my-example-derivation> bye! my-example-derivation (post)> + sleep 1 my-example-derivation (post)> + echo 'Signing paths' /nix/store/c263gzj2kb2609mz8wrbmh53l14wzmfs-my-example-derivation my-example-derivation (post)> Signing paths /nix/store/c263gzj2kb2609mz8wrbmh53l14wzmfs-my-example-derivation my-example-derivation (post)> + sleep 1 my-example-derivation (post)> + echo 'Uploading paths' /nix/store/c263gzj2kb2609mz8wrbmh53l14wzmfs-my-example-derivation my-example-derivation (post)> Uploading paths /nix/store/c263gzj2kb2609mz8wrbmh53l14wzmfs-my-example-derivation my-example-derivation (post)> + sleep 1 my-example-derivation (post)> + printf 'very important stuff' [1 built, 0.0 MiB DL] Co-authored-by: Graham Christensen <graham@grahamc.com> Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
74461e9
to
7c55967
Compare
I think this is ready to go! |
Here is a demo: https://asciinema.org/a/JGe4UHRYMfj554e34gE97h45v |
shell to perform word splitting to make each output path its | ||
own argument to <command>nix sign-paths</command>. Nix guarantees | ||
the paths will only contain characters which are safe for word | ||
splitting, and free of any globs. |
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.
This is not quite true. In particular paths can contain ?
and +
. So doing for i in $OUT_PATHS
may cause globbing (and this has caused performance issues in the past, because globbing a path like ...-gtk+-...
requires the entire Nix store to be read.
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.
Addressed.
doc/manual/command-ref/conf-file.xml
Outdated
<varlistentry> | ||
<term><envar>OUT_PATHS</envar></term> | ||
<listitem> | ||
<para>Output paths of the built derivation, separated by a space (<literal> </literal>) character.</para> |
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.
Drop <literal> </literal>
since it doesn't show anything useful.
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.
Addressed.
<listitem><para>The hook executes after an evaluation-time build.</para></listitem> | ||
<listitem><para>The hook does not execute on substituted paths.</para></listitem> | ||
<listitem><para>The hook's output always goes to the user's terminal.</para></listitem> | ||
<listitem><para>If the hook fails, the build succeeds but no further builds execute.</para></listitem> |
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.
It's also worth noting that the hook is executed synchronously and so blocks other builds from progressing.
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.
Addressed.
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 added a commit addressing the latest round of feedback. I think it is good again :)
Add a new
post-build-hook
mechanism such that passing--option post-build-hook /foo/bar
to a nix-* command will cause/foo/bar /nix/store/drv-that/has-been-built.drv -- out1 out2 ... outn
to be executed after each build.One specially interesting use-case for this is to upload all the builded artifacts to a binary cache (including the ones that don't appear in the runtime closure of the final derivation or are built because of IFD)
cc @grahamc