Derivation JSON `env` does not adhere to the JSON guidelines

[roberth]

Describe the bug

NOTE: Changing how derivations works is rather invasive, so the question for the new-cli effort is whether we want to design this now to be forward compatible with these changes.

Essentially this is inherited from builtins.derivation and the ATerm-based format, but it is exposed on the command line.
This is some deep legacy, but nothing that can't be solved. Solving it is worthwhile because it improves the user experience, complying with the guidelines, and answers other questions such as #9846 without having to come up with something completely arbitrary and subject to change.

Steps To Reproduce

nix derivation show nixpkgs#hello

• Observe that each derivation has fields

  • args
  • builder
  • env
  • inputDrvs
  • inputSrcs
  • name
  • outputs
  • system

  These look well defined, but env is not. It may contain various attributes, such as preferLocalBuild, etc, which affect scheduling and have no business being in the process environment.
  (They should not be part of the output fingerprint either; Make output paths/hashes indifferent to possible CA/FOD origin of sources #9259)

• Observe that the top level is a dictionary. (So the command can't summarize any results without violating the guidelines or switching formats. That may be acceptable.)

Expected behavior

• Derivations do not rely on env for scheduling attributes and such, because those are Nix-specific fields, not dict entries.
• Derivations have a section of non-hashed fields; putting these together as fields in a top-level object means that we can add non-output-hashed fields without breaking compatibility with earlier Nix versions.

nix-env --version output

Additional context

Priorities

Add 👍 to issues you find important.

[edolstra]

Derivations do not rely on env for scheduling attributes and such, because those are Nix-specific fields, not dict entries.

This seems orthogonal to the JSON representation, because this is just how Nix derivation attributes are passed from the language to the store (i.e. via the environment). The derivation primop takes a more-or-less arbitrary attrset in which some attributes have a special meaning to Nix. Changing that seems infeasible without causing massive incompatibilities.

[roberth]

I don't think this change is worthwhile on its own, but in conjunction with other issues such as adding improved input hashing or a JSON-based derivation format, it may make sense to make such changes. Realistically, the opportunity cost of doing this before the CLI is greater than the cost of changing the derivation JSON on an otherwise stable CLI.
Perhaps the new format wouldn't be too dissimilar anyway.

[Ericson2314]

#9846 and this are companions, but neither is about changing the very stable A-Term format and builtins.derivation. Instead they are about changing unstable and/or private things that we are are allowed to change.

Moving away from the A-Term format and builtins.derivation can still happen, but on much longer time scales. These anticipates it but are still useful on their own.

[edolstra]

I don't think this change is worthwhile on its own

It's actually not clear to me what is being proposed here. nix derivation show just shows the contents of the .drv file, and in .drvs, attributes like preferLocalBuild are part of the environment. Changing that is out of scope for nix derivation show. It shouldn't try to be clever about whether some attributes have a special meaning.

[Ericson2314]

@edolstra Then think about this dependency order

1. Merge ParsedDerivation into BasicDerivation #9846 No more ParsedDerivation. Things which are not actually env vars are stored separately in Derivation. The A-Term parser/printer becomes more complicated. Many other thing things become simpler.
2. This issue. If the JSON implementation is to continue to not be clever, and not become clever like the A-Term implementation did in step 1, then the JSON representation must change.

[edolstra]

Attributes like preferLocalBuild are environment variables. That's how it has always worked.

What problem is being solved here?

[Ericson2314]

The environment variables map should be just that --- arbitrary environment variables. That we are smuggling in special meaning with some of them is clearly not a good design decision made in a vacuum, but way to try to shove in more functionality in the pre-existing non-extensible A-Term format.

When one sets preferLocalBuild = true with builtins.derivation, it's OK if that continues to set an environment variable too -- indeed, for backwards compat, it's important that it continue to do so. But we should store the special preferLocalBuild state in Derivation separately from the environment variable, because the environment variable itself should have no special special meaning.

The JSON format should match the new Derivation struct and also separate the special flag from the regular environment variable.