libstore: pass the derivation being built to the post-build-hook as JSON by amaanq · Pull Request #16413 · NixOS/nix

amaanq · 2026-09-01T21:19:17Z

Motivation

Currently, a post-build hook only gets DRV_PATH, but the derivation the builder actually ran (the resolved derivation, for CA builds) may not exist on disk as a .drv file at all, so consumers like Hydra can't reconstruct what was actually built.

Context

The hook now receives the JSON form of the BasicDerivation that DerivationBuildingGoal already holds in memory on its standard input.

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Ericson2314 · 2026-09-01T22:19:27Z

I requested that @amaanq do this based on what I understand @NixOS/cuda-maintainers needs to upgrade their Hydra.

xokdvium · 2026-09-01T22:20:30Z

Why JSON and not the stable ATerm?

Ericson2314 · 2026-09-01T22:22:13Z

@xokdvium because we don't have ATerm for resolved derivations, though I suppose we could fudge an empty inputDrvs.

Ericson2314 · 2026-09-01T22:23:05Z

I do agree we should ~~get~~ gate unstable behavior, but I also think this will just be much easier for the hooks in practice.

xokdvium · 2026-09-01T22:30:55Z

because we don't have ATerm for resolved derivations, though I suppose we could fudge an empty inputDrvs.

Isn't that exactly what we want? empty inputDrvs is perfectly fine in this case

xokdvium · 2026-09-01T22:31:58Z

If one wants to opt into converting ATerm -> json can't we carry a plumbing convenience command?

Ericson2314 · 2026-09-01T22:32:30Z

Yes, empty inputDrvs is fine :). (The JSON for resolved derivations just has inputs: [ ....paths.... ] though, which I do like --- "Making illegal states unrepresentable" and all that.

xokdvium · 2026-09-01T22:37:56Z

which I do like --- "Making illegal states unrepresentable" and all that.

But aren't we going to pass in the original derivation in case it didn't need to be resolved first? Or would that to be consistent everywhere with remote building? Wasn't the DRV_PATH the unresolved one in the local case at least?

Ericson2314 · 2026-09-01T22:41:23Z

@xokdvium it is my view that morally we always build a resolved derivation --- the only reason we don't realize input-addressed ones is that it interferes with trustless remote building because of a design flaw in the "derivation masking" (née hashDerivationModulo) algorithm.

Think if we have two input-addressing derivations that produce the same paths, we're only going to build one of them, right? Then it would be a bit arbitrary which one ended up making it to the build hook, right?

Ericson2314 · 2026-09-01T22:43:11Z

Indeed, I wouldn't expose the unresolved drvPath either if I was designing this from scratch.

xokdvium · 2026-09-01T22:43:12Z

+            if (dup2(drvJsonFd.get(), STDIN_FILENO) == -1)
+                throw SysError("dupping stdin");


Seems not terribly extensible. What if we wanted to pass more info to the hook? Also how would the hook feature detect that the version of nix is new enough? Do a non-blocking read of stdin?

Can't we use a non-standard file descriptor maybe and pass its number in an env variable?

Seems not terribly extensible. What if we wanted to pass more info to the hook?

That is why I asked @amaanq to make the top-level record to contain derivation (before I realized the we should include the output map for the CA case too). That puts us in compliance with the JSON guidelines re extensibility.

Can't we use a non-standard file descriptor maybe and pass its number in an env variable?

Yeah sure I am OK with that. I like the socket-activation vibes :)

xokdvium · 2026-09-01T22:52:25Z

I wouldn't expose the unresolved drvPath either if I was designing this from scratch.

Sounds like maybe we should also expose RESOLVED_DRV_PATH to the hook for more clarity. That way the hook wouldn't need to guess which case it is

SomeoneSerge · 2026-09-02T12:12:04Z

...based on what I understand @NixOS/cuda-maintainers needs to upgrade their Hydra

CC #9272 (it's "pre-", but same logic)

Why JSON and not the stable ATerm?

FWIW it's also substantially easier to consume JSON in an external hook/plug-in, than it is to consume ATerm.

The hook now receives the JSON form of the BasicDerivation

I've already argued this before, I believe the "proper" approach (conveniently, we don't necessarily need "proper", imo it's ok to change hook interfaces) would be to have in place a protocol that lets you inquire about any path referenced in the context of the current "build request". Essentially, my reasoning was:

Details

- pre-build-hook gets a drvPath, but it's basically a string of random characters, unless you can access the drv structure, so "why pass the drvPath in the first place?" - the drv structure contains inputDrvs and inputSrcs, but they are, too, basically random strings unless you can also access the corresponding drv structures or file paths, or access a database where you compare these ids against other ids. If can't access that, then by the same token "why include input{Drv,Src}s in the response at all?"

Ericson2314 · 2026-09-02T14:49:03Z

@SomeoneSerge we could definitely e.g. rig up the hook to run with the daemon protocol or something so one can inquire about input and output paths to their heart's content :)

xokdvium · 2026-09-02T18:39:23Z

FWIW it's also substantially easier to consume JSON in an external hook/plug-in, than it is to consume ATerm.

Hence why my suggestion to have a plumbing command to convert the stable ATerm into the experimental and very unstable JSON variant that's only really available in the XP cli if one opts into that. Wouldn't want to use to even bother with that can of worms now and maybe we can just have the internal plumbing be in the usual representation.

Ericson2314 · 2026-09-02T18:53:12Z

Maybe nix derivation show should also support derivations on standard input then? Though an awkward part of that is that the ATerm doesn't contain the derivation's name --- it is not actually self-contained.

xokdvium · 2026-09-02T19:08:20Z

Though an awkward part of that is that the ATerm doesn't contain the derivation's name --- it is not actually self-contained.

But that's already available in the DRV_PATH, no?

Ericson2314 · 2026-09-02T19:15:23Z

True, it is. and we can do DRV_NAME too. (Or stick it in the JSON.)

xokdvium · 2026-09-02T21:18:10Z

+            /* closeExtraFDs() closes everything above stderr, hence the detour via stdin and stdout. */
+            if (dup2(STDIN_FILENO, drvAtermFdNum) == -1)
+                throw SysError("dupping the derivation file onto fd %d", drvAtermFdNum);
+            if (dup2(STDOUT_FILENO, buildInfoJsonFdNum) == -1)
+                throw SysError("dupping the build info file onto fd %d", buildInfoJsonFdNum);
+            if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1)
+                throw SysError("cannot dup stderr into stdout");
+            AutoCloseFD fdDevNull = open("/dev/null", O_RDONLY | O_CLOEXEC);
+            if (!fdDevNull)
+                throw SysError("opening /dev/null");
+            if (dup2(fdDevNull.get(), STDIN_FILENO) == -1)
+                throw SysError("dupping /dev/null onto stdin");


We should just have a generic FD dup action which handles all of this internally. I.e. not use startProcess but instead have spawnProgram that does all of this.

We could land that in a prep PR as I think there are a few places that use it.

I have started working on that locally, so maybe a couple of days until that's beat into shape?

Assisted-by: Claude-Code (Claude Opus 5)

xokdvium · 2026-09-03T21:31:00Z

Btw @Ericson2314, wasn't the issue with CUDA hydra the pre-build-hook, not post? I'm not sure why we are touching post- one here then.

GaetanLepage · 2026-09-04T07:59:37Z

Btw @Ericson2314, wasn't the issue with CUDA hydra the pre-build-hook, not post? I'm not sure why we are touching post- one here then.

Hi! Here is the patch we apply to Hydra to fix our use case: https://github.com/nixos-cuda/infra/blob/master/hosts/hydra/hydra/0001-hydra-queue-runner-make-drv-available-to-remote-pre-build-hook.patch

xokdvium · 2026-09-04T08:50:13Z

Hi! Here is the patch we apply to Hydra to fix our use case: https://github.com/nixos-cuda/infra/blob/master/hosts/hydra/hydra/0001-hydra-queue-runner-make-drv-available-to-remote-pre-build-hook.patch

That's not great... because it ships around the whole derivation closure and unused sources. Hence why the idea here is to stuff ATerm into a side-channel and not write to the store (because then all the references have to be copied as well).

GaetanLepage · 2026-09-04T11:13:46Z

That's not great... because it ships around the whole derivation closure and unused sources. Hence why the idea here is to stuff ATerm into a side-channel and not write to the store (because then all the references have to be copied as well).

Definitely. It's just a dirty workaround. I trust you on the better solution.

amaanq requested review from Ericson2314 and edolstra as code owners September 1, 2026 21:19

github-actions Bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Sep 1, 2026

github-project-automation Bot added this to Nix documentation Sep 1, 2026

Ericson2314 reviewed Sep 1, 2026

View reviewed changes

Comment thread doc/manual/rl-next/post-build-hook-drv-json.md Outdated

Ericson2314 reviewed Sep 1, 2026

View reviewed changes

Comment thread src/libstore/build/derivation-building-goal.cc Outdated

Ericson2314 reviewed Sep 1, 2026

View reviewed changes

Comment thread tests/functional/post-hook.sh Outdated

amaanq force-pushed the post-build-hook-drv-json branch 3 times, most recently from 0d58607 to a47236c Compare September 1, 2026 22:12

amaanq force-pushed the post-build-hook-drv-json branch from a47236c to 7df0419 Compare September 1, 2026 22:21

xokdvium reviewed Sep 1, 2026

View reviewed changes

amaanq force-pushed the post-build-hook-drv-json branch from 7df0419 to bedd51c Compare September 2, 2026 00:32

amaanq force-pushed the post-build-hook-drv-json branch from bedd51c to 59e567b Compare September 2, 2026 19:08

amaanq force-pushed the post-build-hook-drv-json branch 2 times, most recently from c210442 to df9b767 Compare September 2, 2026 20:55

github-actions Bot added the new-cli Relating to the "nix" command label Sep 2, 2026

xokdvium reviewed Sep 2, 2026

View reviewed changes

libstore: pass the derivation being built to the post-build-hook as JSON

9cef817

Assisted-by: Claude-Code (Claude Opus 5)

amaanq force-pushed the post-build-hook-drv-json branch from df9b767 to 9cef817 Compare September 3, 2026 19:44

		if (dup2(drvJsonFd.get(), STDIN_FILENO) == -1)
		throw SysError("dupping stdin");

Uh oh!

Uh oh!

Conversation

amaanq commented Sep 1, 2026

Motivation

Context

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ericson2314 commented Sep 1, 2026

Uh oh!

xokdvium commented Sep 1, 2026

Uh oh!

Ericson2314 commented Sep 1, 2026

Uh oh!

Ericson2314 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xokdvium commented Sep 1, 2026

Uh oh!

xokdvium commented Sep 1, 2026

Uh oh!

Ericson2314 commented Sep 1, 2026

Uh oh!

xokdvium commented Sep 1, 2026

Uh oh!

Ericson2314 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ericson2314 commented Sep 1, 2026

Uh oh!

xokdvium Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

xokdvium commented Sep 1, 2026

Uh oh!

SomeoneSerge commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ericson2314 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xokdvium commented Sep 2, 2026

Uh oh!

Ericson2314 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xokdvium commented Sep 2, 2026

Uh oh!

Ericson2314 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xokdvium Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

xokdvium Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

xokdvium commented Sep 3, 2026

Uh oh!

GaetanLepage commented Sep 4, 2026

Uh oh!

xokdvium commented Sep 4, 2026

Uh oh!

GaetanLepage commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Ericson2314 commented Sep 1, 2026 •

edited

Loading

Ericson2314 commented Sep 1, 2026 •

edited

Loading

SomeoneSerge commented Sep 2, 2026 •

edited

Loading

Ericson2314 commented Sep 2, 2026 •

edited

Loading

Ericson2314 commented Sep 2, 2026 •

edited

Loading

Ericson2314 commented Sep 2, 2026 •

edited

Loading