buildEnv: support fixed-point arguments by ShamrockLee · Pull Request #432957 · NixOS/nixpkgs

ShamrockLee · 2025-08-12T00:20:35Z

This PR restructure buildEnv with lib.extendMkDerivation, stdenvNoCC.mkDerivation, and buildCommand. buildEnv can now take fixed-point arguments (finalAttrs: { }).
~~We still rely on the custom overrider <env-pkg>.override, as fixing the __structuredAttrs = true support might change the <pkg>.overrideAttrs interface.~~
PR buildEnv: use structured attribute and support <pkg>.overrideAttrs #434815 proves that the argument overriding is mostly the same after adopting structured attributes. Let's deprecate the custom overrider <env-pkg>.override right away, but keep it until tree-wide transition to <pkg>.overrideAttrs.
This PR introduces the derivationArgs argument for buildEnv so that we won't have to exhaustively list all the stdenv.mkDerivation arguments users might wish to pass.
If tested, this PR minus the release note can be backported.

This PR depends on:

check-meta: defaultCPEParts: accomodate null pname #458529

Things done

Add a 👍 reaction to pull requests you find important.

ShamrockLee · 2025-08-13T11:22:20Z

@ofborg build nixpkgs-vet

ShamrockLee · 2025-08-14T08:19:46Z

I don't know why the CI complains that nixpkgs-vet fails to build. It builds locally. There seemed to be a network issue when the CI vetting jobs run.

Update: Rerun. They are now green.

philiptaron · 2025-08-14T16:19:23Z

Taking a look. Is there a version of these that doesn't rebuild everything?

Ch1keen · 2025-08-14T16:24:13Z

Would take a look. But it is first time reviewing the core of the Nix builtin function, are there any ways that I can test with this change? Would building derivations with the change be helpful?

philiptaron

My sense is that the rebuilds come about because of the switch from runCommand to stdenvNoCC.mkDerivation. Is that right?

The order of the merge operations (derivationArgs first, then applying the functions arguments) gives me some heartburn. It's always odd to have something set in derivationArgs which then doesn't take effect. It's sort of a bug factory. The other way is also perilous however, as certain derivationArgs are actually invalid for this build helper.

My preferred way to handle this is to use something like intersectAttrs to forbid a few things in derivationAttrs that would malform the resulting derivation. See replaceVarsWith for an example of that. Thanks to @wolfgangwalther for the pattern.

pkgs/build-support/buildenv/default.nix

philiptaron · 2025-08-14T16:59:13Z

I'm also interested in how we deprecate and remove the lib.makeOverrideable call.

ShamrockLee · 2025-08-14T17:13:24Z

Taking a look. Is there a version of these that doesn't rebuild everything?

It might be doable by excluding some arguments, but excluding paths from the passing arguments makes it impossible to get its final state from finalAttrs and contradicts the purpose of this PR.

My sense is that the rebuilds come about because of the switch from runCommand to stdenvNoCC.mkDerivation. Is that right?

It's not the main reason of the mass rebuild. The gap between stdenvNoCC.mkDerivation and runCommand can be bridged by prefixing buildCommand to passAsFile (which I think is a good idea and is going to implement).

ShamrockLee · 2025-08-14T17:43:09Z

I'm also interested in how we deprecate and remove the lib.makeOverrideable call.

We can support structured attributes while keeping <pkg>.overrideAttrs unchanged by refactoring buildenv/builder.pl to get variables from the JSON file whose path is inside the environment variable NIX_ATTRS_JSON_FILE if such environment variable is available and nonempty, and falling back to the old behaviour of getting from environment variables of the same names.

Cc: @wolfgangwalther

ShamrockLee · 2025-08-14T17:57:07Z

Would take a look. But it is first time reviewing the core of the Nix builtin function,

Thank you for helping out! To be precise, buildEnv is provided by Nixpkgs instead of Nix.

are there any ways that I can test with this change? Would building derivations with the change be helpful?

Maybe we could build some packages given buildEnv's wide adoption, or build some custom packages constructed by buildEnv.

For example,

nix build --no-link --print-out-paths -L --impure --expr "
  let
    pkgs = import ./. { };
  in
  pkgs.buildEnv (finalAttrs: {
    paths = with pkgs; [ hello figlet ];
  })
"

and compare its output contents with that of hello and figlet.

I have little idea how to test it thoroughly, either. I tried to write some test cases when working on ignoreSingleFileOutputs, but it turned out to be harder than I had thought.

wolfgangwalther

We can support structured attributes while keeping <pkg>.overrideAttrs unchanged by refactoring buildenv/builder.pl to get variables from the JSON file whose path is inside the environment variable NIX_ATTRS_JSON_FILE if such environment variable is available and nonempty, and falling back to the old behaviour of getting from environment variables of the same names.

Sounds like the right approach. But since you're mentioning overrideAttrs explicitly, it sounds like there is another way of fixing structuredAttrs support which would not keep overrideAttrs the same? Which way would that be?

wolfgangwalther · 2025-08-15T19:30:05Z

pkgs/build-support/buildenv/default.nix

+# We currently still rely on this custom overrider `<env-pkg>.override`,
+# as fixing the structured attribute support would change the `<pkg>.overrideAttrs` interface.


Why is "changing the overrideAttrs interface" considered to be a problem?

In which way would this interface change?

The approaches I considered includes:

Moving under env the variables to pass to the builder. This way we don't need to change the Perl build script.

Enforce __structuredAttrs = true for all constructed packages and pass everythong consistently via NIX_ATTRS_JSON_FILE. This way we avoid the manual serialization of [ { paths = ; priority = ; } ] completely.

I looked into the builder a bit, and I think I'd prefer this:

2. Enforce __structuredAttrs = true for all constructed packages and pass everythong consistently via NIX_ATTRS_JSON_FILE. This way we avoid the manual serialization of [ { paths = ; priority = ; } ] completely.

This would not change the overrideAttrs interface either, right?

This would not change the overrideAttrs interface either, right?

Yes, it won't change the overriding interface. (I personally feel like replacing env.pkgs with a structual env.pathSpecs and make it part of the public interface, so that people could add paths with custom priority.)

Nevertheless, it's a tree-wide backward-incompatible change whose impact spreads across several language- and framework-specific toolchain. Python's python3Packagrs.buildEnv is currently a pkgs.buildEnv-constructed package, and python3.withPackabes is implemented by overriding python3Packages.buildEnv. In addition, it provides postBuild to allow appending custom commands. In the worst case scenario, we'll have to test dependent packages and frameworks one by one and do our best to inform users about the breakage.

Nevertheless, it's a tree-wide backward-incompatible change

Wait, which one? Enforcing structured attrs or the pathSpecs you brought into play?

I think enforcing structuredAttrs should not be breaking, right?

Since we set buildCommand, the phases will not be run. I think this means that setup hooks won't be run at all.

So we just need to deal with postBuild, which is manually ran in the buildCommand.

So the breaking change would be, because the structured attrs file is not additionally loaded as bash variables?

I think this would actually already happen through stdenv/setup.sh.

So I don't see the breaking part, yet.

It just came to me that we usually don't provide bavkward compatibility for pre- or post- stuff. Maybe we could try it on the master branch, whose users are expected to tolerate breakage, and fix things that break.

Since we set buildCommand, the phases will not be run. I think this means that setup hooks won't be run at all.

So we just need to deal with postBuild, which is manually ran in the buildCommand.

So the breaking change would be, because the structured attrs file is not additionally loaded as bash variables?

I think this would actually already happen through stdenv/setup.sh.

So I don't see the breaking part, yet.

Ah, i see.

I thought that a user might pass some random attributes and expects them to be passed to buildCommand as environment variables, but that is not possible without the derivationArgs argument introduced in this PR or overrideAttrs. Since both are not currently supported, there shouldn't be backward incompatibility issues.

As for the setup hooks that still fail to support structual attrs, let's consider such failure their bugs.

As for the setup hooks that still fail to support structural attrs, let's consider such failures to be their bugs.

Quite so. I agree with both you and Wolfgang this is the direction the PR ought to go.

pkgs/build-support/buildenv/default.nix

ShamrockLee · 2025-11-04T17:35:26Z

I fixed the evaluation by fixing the check-meta defaultCPEParts with PR #458529
I'll rebase once it is merged and comes to the staging branch.

RossSmyth

Very nice PR, would be cool to see it.

RossSmyth · 2026-02-11T16:16:57Z

pkgs/build-support/buildenv/default.nix

-      eval "$postBuild"
-    ''
+        ++ lib.optional (lib.stringLength finalAttrs.pkgs >= 128 * 1024) "pkgs"
+        ++ derivationArgs.passAsFile or [ ];


I will note that passAsFile is not very compatible with structuredAttrs, so the the goal is to move buildEnv to structuredAttrs, this should probably be removed.

When __structuredAttrs = false, one can find values via $myAttrPath set in the environment, while $myAttr is unset

When __structuredAttrs = true, $myAttrPath is unset, and myAttr contains the value that would be in the file

So I would not recommend exposing that to users.

philiptaron

I added a bunch of tests. Please take a look.

r-ryantm-500plus-rebuild-prs.md

Add an eval-time test harness for buildEnv following the pattern established by `tests.overriding`. Uses `lib.runTests` to compare expr/expected pairs, and the buildCommand reports pass/fail. No tests yet; subsequent commits add them individually.

Verify that: - `name` argument is used directly as the derivation name - `pname` + `version` composes into the derivation name - Omitting all three triggers the buildEnv-specific assertion The assertion test uses `builtins.tryEval` + `builtins.seq` to force evaluation of .drvPath and confirm it throws.

Specify `extraPathsFrom` as a string using `lib.optionalString` instead of a singleton list using `lib.optional` to align with the current behavior of the Perl build script.

…g>.overridAttrs

Merge derivationArgs.passthru into the passthru chain so that passthru attributes passed via derivationArgs are not silently dropped.

- includeClosure -> includeClosures (matching the actual argument name) - extraPrefix default: [ ] -> "" (matching the actual default) - Fix broken sentence about ignoreSingleFileOutputs

buildEnv stashes `paths` into `passthru.paths` (rather than passing it directly as a derivation attribute) to prevent unexpected string context pollution. Verify that: - paths are accessible via passthru.paths - passthru.paths can be overridden with overrideAttrs

This is the core new capability of the PR: `buildEnv` now accepts a function `(finalAttrs: { ... })` enabling self-referencing attributes. Verify that finalAttrs.name is accessible from within the argument function.

Verify that overrideAttrs on a buildEnv result: - Can modify passthru attributes - Preserves the derivation name - Produces a different derivation when a build-affecting attribute (postBuild) is changed

buildEnv merges passthru from three sources: `{ inherit paths; } // derivationArgs.passthru // passthru` Verify that: - Auto-injected paths are present in passthru - derivationArgs.passthru attributes are preserved - Direct passthru attributes are preserved - Direct passthru takes precedence over derivationArgs.passthru

Verify that: - derivationArgs attributes reach the underlying mkDerivation (tested via allowSubstitutes overriding the default false) - The backward-compat layer for top-level nativeBuildInputs still works (these are forwarded through compatArgs)

Add derivations that actually build a buildEnv and verify the output tree. Exposed as passthru.buildTests so they can be built individually or all at once. - basic-symlinking: single package produces correct symlink structure - pathsToLink: only specified subdirectories appear in output - extraPrefix: output is rooted under the specified prefix - postBuild: shell commands execute after the symlink tree - ignoreCollisions: duplicate paths succeed when ignoreCollisions=true

jopejoe1 · 2026-03-03T15:24:15Z

pkgs/development/python-modules/pybamboohr/default.nix

This file does not look like it belongs in here.

I guess it is a leftover during some git-rebase.

Noticed that the commit is authored by @philiptaron. As I have too much on my plate to catch up on these changes , I'll keep it as is.

Yeah, this is my mistake… I am occasionally a noob with git worktrees, and this is me reusing one that I shouldn't have.

ShamrockLee · 2026-03-03T16:14:53Z

I'm tided up with my research and thesis. Sorry for leaving halfway. Many thanks to @philiptaron for pushing this PR.

ShamrockLee force-pushed the buildenv-overriding branch 2 times, most recently from bb27fcf to 09aa8ad Compare August 12, 2025 10:56

ShamrockLee changed the title ~~buildEnv: support fixed-point arguments and <pkg>.overrideAttrs~~ buildEnv: support fixed-point arguments Aug 13, 2025

ShamrockLee marked this pull request as ready for review August 14, 2025 08:21

nix-owners bot requested review from Ch1keen and philiptaron August 14, 2025 08:21

ShamrockLee requested a review from drupol August 14, 2025 08:27

philiptaron reviewed Aug 14, 2025

View reviewed changes

pkgs/build-support/buildenv/default.nix Outdated Show resolved Hide resolved

ShamrockLee force-pushed the buildenv-overriding branch 2 times, most recently from 9b0afe2 to aa291a2 Compare August 14, 2025 17:33

wolfgangwalther reviewed Aug 15, 2025

View reviewed changes

ShamrockLee commented Aug 16, 2025

View reviewed changes

pkgs/build-support/buildenv/default.nix Outdated Show resolved Hide resolved

ShamrockLee force-pushed the buildenv-overriding branch 3 times, most recently from 84b1c5a to 75428e1 Compare August 17, 2025 19:08

ShamrockLee mentioned this pull request Nov 4, 2025

check-meta: defaultCPEParts: accomodate null pname #458529

Merged

13 tasks

ShamrockLee mentioned this pull request Dec 14, 2025

buildEnv: enable structuredAttrs #470402

Open

13 tasks

RossSmyth suggested changes Feb 11, 2026

View reviewed changes

RossSmyth mentioned this pull request Feb 11, 2026

Tracking: fixed-point args for derivation builders #489490

Open

79 tasks

philiptaron approved these changes Feb 16, 2026

View reviewed changes

RossSmyth reviewed Feb 17, 2026

View reviewed changes

r-ryantm-500plus-rebuild-prs.md Outdated Show resolved Hide resolved

philiptaron and others added 16 commits February 16, 2026 16:15

tests.buildenv: init test harness

3e21a8d

Add an eval-time test harness for buildEnv following the pattern established by `tests.overriding`. Uses `lib.runTests` to compare expr/expected pairs, and the buildCommand reports pass/fail. No tests yet; subsequent commits add them individually.

buildEnv: specify extraPathsFrom a string as expected by the builder

3ef335b

Specify `extraPathsFrom` as a string using `lib.optionalString` instead of a singleton list using `lib.optional` to align with the current behavior of the Perl build script.

buildEnv: construct with extendMkDerivation and stdenvNoCC.mkDerivation

b780af0

buildEnv: reference attributes from finalAttrs

f561739

buildEnv: take derivationArgs

05d7906

doc: buildenv.section.md: init

b091441

rl-2511.section.md: document buildEnv's supporting finalAttrs and <pk…

edd8300

…g>.overridAttrs

buildEnv: fix passthru merging

49643c9

Merge derivationArgs.passthru into the passthru chain so that passthru attributes passed via derivationArgs are not silently dropped.

doc: buildenv.section.md: fix argument name, default, and grammar

6807e20

- includeClosure -> includeClosures (matching the actual argument name) - extraPrefix default: [ ] -> "" (matching the actual default) - Fix broken sentence about ignoreSingleFileOutputs

tests.buildenv: test finalAttrs self-reference

2d7d084

This is the core new capability of the PR: `buildEnv` now accepts a function `(finalAttrs: { ... })` enabling self-referencing attributes. Verify that finalAttrs.name is accessible from within the argument function.

tests.buildenv: test overrideAttrs

4bad9e8

Verify that overrideAttrs on a buildEnv result: - Can modify passthru attributes - Preserves the derivation name - Produces a different derivation when a build-affecting attribute (postBuild) is changed

jopejoe1 requested changes Mar 3, 2026

View reviewed changes

xworld21 mentioned this pull request Mar 8, 2026

texlive.withPackages: add withDocs, withSources and expose texliveFullWithDocs at top level #497867

Open

13 tasks

		# We currently still rely on this custom overrider `<env-pkg>.override`,
		# as fixing the structured attribute support would change the `<pkg>.overrideAttrs` interface.

Uh oh!

Conversation

ShamrockLee commented Aug 12, 2025 • edited by philiptaron Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

ShamrockLee commented Aug 13, 2025

Uh oh!

ShamrockLee commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron commented Aug 14, 2025

Uh oh!

Ch1keen commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

philiptaron commented Aug 14, 2025

Uh oh!

ShamrockLee commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShamrockLee commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShamrockLee commented Aug 14, 2025

Uh oh!

wolfgangwalther left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShamrockLee Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShamrockLee commented Nov 4, 2025

Uh oh!

RossSmyth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShamrockLee commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

ShamrockLee commented Aug 12, 2025 •

edited by philiptaron

Loading

ShamrockLee commented Aug 14, 2025 •

edited

Loading

Ch1keen commented Aug 14, 2025 •

edited

Loading

ShamrockLee commented Aug 14, 2025 •

edited

Loading

ShamrockLee commented Aug 14, 2025 •

edited

Loading

ShamrockLee Aug 16, 2025 •

edited

Loading