pkgs/stdenv: normalize attributes where setup.sh makes no distinction#201131
Conversation
This PR normalizes the final attrset immediately before it is passed
to `builtins.derivation`, deleting attributes whose value is `[]`,
`""`, or `false` if `setup.sh` treats that particular value for that
particular attribute indentically to the attribute being absent.
As a result of this PR, it is possible to add, to an existing
derivation, an attribute with the form:
```
hardeningFlags = lib.optionals stdenv.hostPlatform.isFrobnicator [
...
```
without triggering an unnecessary mass-rebuild unless Frobnicator is
one of the Hydra `supportedPlatforms`. Previously it was necessary
to either use the non-idiomatic:
```
if !stdenv.hostPlatform.isFrobnicator then null else [
```
or to trigger a mass-rebuild and suffer OfBorg's "red ovals of doom"
on the PR. The latter case tends to lead to stale and forgotten PRs.
Recent example: #199978 (comment)
|
This will have a significant evaluation performance cost. It also increases the size of Possible optimizations:
- derivationArgBeforeNormalization =
+ derivationArg =
(removeAttrs attrs
["meta" "passthru" "pos"- (derivation derivationArg);
+ (legacyDerivationAttrs // derivationStrict (normalizedAttrs derivationArg))
The implementation details won't be too complicated, but has to match what Nix does internally. Also we're going to need non-automated benchmarks for this PR, because ofborg won't be providing its eval performance report. This one is a mass rebuild after all. |
|
I can't promise that the performance cost can be overcome. Perhaps at some point a fresh and more modular replacement for |
|
@roberth, thank you so much for looking at this! As you surely noticed, it currently does not work. As you probably also noticed, it turns out to be much more difficult than I initially expected. I have to bump it down my priority list a few notches -- so it may be a month or two before I can revisit it. |
This should wait until after
stagingfor 22.11 has branched-off.Description of changes
This PR normalizes the final attrset immediately before it is passed to
builtins.derivation, deleting attributes whose value is[],"", orfalseifsetup.shtreats that particular value for that particular attribute indentically to the attribute being absent.As a result of this PR, it is possible to add, to an existing derivation, an attribute with the form:
without triggering an unnecessary mass-rebuild unless Frobnicator is one of the Hydra
supportedPlatforms. Previously it was necessary to either use the non-idiomatic:or to trigger a mass-rebuild and suffer OfBorg's "red ovals of doom" on the PR. The latter case tends to lead to stale and forgotten PRs.
Recent example: #199978 (comment)
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes