Skip to content

Commit 67e8392

Browse files
vcunatvcunat
committed
Merge #33057: stdenv meta checks: make them lazy
Closes #22277 - it's superseded; I have some WIP on evaluation performance, but best do that in a separate PR/thread.
2 parents 1a05448 + 799b941 commit 67e8392

6 files changed

Lines changed: 86 additions & 50 deletions

File tree

‎lib/customisation.nix‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ rec {
3636
overrideDerivation = drv: f:
3737
let
3838
newDrv = derivation (drv.drvAttrs // (f drv));
39-
in addPassthru newDrv (
39+
in lib.flip (extendDerivation true) newDrv (
4040
{ meta = drv.meta or {};
4141
passthru = if drv ? passthru then drv.passthru else {};
4242
}
@@ -131,8 +131,8 @@ rec {
131131

132132

133133
/* Add attributes to each output of a derivation without changing
134-
the derivation itself. */
135-
addPassthru = drv: passthru:
134+
the derivation itself and check a given condition when evaluating. */
135+
extendDerivation = condition: passthru: drv:
136136
let
137137
outputs = drv.outputs or [ "out" ];
138138

@@ -142,13 +142,23 @@ rec {
142142
outputToAttrListElement = outputName:
143143
{ name = outputName;
144144
value = commonAttrs // {
145-
inherit (drv.${outputName}) outPath drvPath type outputName;
145+
inherit (drv.${outputName}) type outputName;
146+
drvPath = assert condition; drv.${outputName}.drvPath;
147+
outPath = assert condition; drv.${outputName}.outPath;
146148
};
147149
};
148150

149151
outputsList = map outputToAttrListElement outputs;
150-
in commonAttrs // { outputUnspecified = true; };
152+
in commonAttrs // {
153+
outputUnspecified = true;
154+
drvPath = assert condition; drv.drvPath;
155+
outPath = assert condition; drv.outPath;
156+
};
151157

158+
/* Add attributes to each output of a derivation without changing
159+
the derivation itself. */
160+
addPassthru = lib.warn "`addPassthru` is deprecated, replace with `extendDerivation true`"
161+
(extendDerivation true);
152162

153163
/* Strip a derivation of all non-essential attributes, returning
154164
only those needed by hydra-eval-jobs. Also strictly evaluate the

‎lib/default.nix‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ let
8787
inherit (stringsWithDeps) textClosureList textClosureMap
8888
noDepEntry fullDepEntry packEntry stringAfter;
8989
inherit (customisation) overrideDerivation makeOverridable
90-
callPackageWith callPackagesWith addPassthru hydraJob makeScope;
90+
callPackageWith callPackagesWith extendDerivation addPassthru
91+
hydraJob makeScope;
9192
inherit (meta) addMetaAttrs dontDistribute setName updateName
9293
appendToName mapDerivationAttrset lowPrio lowPrioSet hiPrio
9394
hiPrioSet;

‎nixos/doc/manual/release-notes/rl-1803.xml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ following incompatible changes:</para>
113113
</listitem>
114114
<listitem>
115115
<para>
116-
<literal>cc-wrapper</literal>has been split in two; there is now also a <literal>bintools-wrapper</literal>.
116+
<literal>cc-wrapper</literal> has been split in two; there is now also a <literal>bintools-wrapper</literal>.
117117
The most commonly used files in <filename>nix-support</filename> are now split between the two wrappers.
118118
Some commonly used ones, like <filename>nix-support/dynamic-linker</filename>, are duplicated for backwards compatability, even though they rightly belong only in <literal>bintools-wrapper</literal>.
119119
Other more obscure ones are just moved.
@@ -131,6 +131,11 @@ following incompatible changes:</para>
131131
Other types dependencies should be unaffected.
132132
</para>
133133
</listitem>
134+
<listitem>
135+
<para>
136+
<literal>lib.addPassthru</literal> is removed. Use <literal>lib.extendDerivation true</literal> instead. <emphasis role="strong">TODO: actually remove it before branching 18.03 off.</emphasis>
137+
</para>
138+
</listitem>
134139
<listitem>
135140
<para>
136141
The <literal>memcached</literal> service no longer accept dynamic socket

‎pkgs/os-specific/linux/kernel/generic.nix‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,12 @@ let
134134
passthru = kernel.passthru // (removeAttrs passthru [ "passthru" ]);
135135
};
136136

137-
nativeDrv = lib.addPassthru kernel.nativeDrv passthru;
137+
addPassthru' = lib.extendDerivation true passthru;
138138

139-
crossDrv = lib.addPassthru kernel.crossDrv passthru;
139+
nativeDrv = addPassthru' kernel.nativeDrv;
140+
141+
crossDrv = addPassthru' kernel.crossDrv;
140142

141143
in if kernel ? crossDrv
142144
then nativeDrv // { inherit nativeDrv crossDrv; }
143-
else lib.addPassthru kernel passthru
145+
else addPassthru' kernel

‎pkgs/stdenv/generic/check-meta.nix‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
# Extend a derivation with checks for brokenness, license, etc. Throw a
2-
# descriptive error when the check fails; return `derivationArg` otherwise.
3-
# Note: no dependencies are checked in this step.
1+
# Checks derivation meta and attrs for problems (like brokenness,
2+
# licenses, etc).
43

54
{ lib, config, system, meta, derivationArg, mkDerivationArg }:
65

@@ -154,12 +153,14 @@ let
154153

155154
# Weirder stuff that doesn't appear in the documentation?
156155
knownVulnerabilities = listOf str;
156+
name = str;
157157
version = str;
158158
tag = str;
159159
updateWalker = bool;
160160
executables = listOf str;
161161
outputsToInstall = listOf str;
162162
position = str;
163+
evaluates = bool;
163164
repositories = attrsOf str;
164165
isBuildPythonPackage = platforms;
165166
schedulingPriority = int;
@@ -196,13 +197,11 @@ let
196197
{ valid = false; reason = "unknown-meta"; errormsg = "has an invalid meta attrset:${lib.concatMapStrings (x: "\n\t - " + x) res}"; }
197198
else { valid = true; };
198199

200+
validity = checkValidity attrs;
201+
202+
in validity // {
199203
# Throw an error if trying to evaluate an non-valid derivation
200-
validityCondition =
201-
let v = checkValidity attrs;
202-
in if !v.valid
203-
then handleEvalIssue (removeAttrs v ["valid"])
204-
else true;
205-
206-
in
207-
assert validityCondition;
208-
derivationArg
204+
handled = if !validity.valid
205+
then handleEvalIssue (removeAttrs validity ["valid"])
206+
else true;
207+
}

‎pkgs/stdenv/generic/make-derivation.nix‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ rec {
8181
inherit erroneousHardeningFlags hardeningDisable hardeningEnable supportedHardeningFlags;
8282
})
8383
else let
84+
references = nativeBuildInputs ++ buildInputs
85+
++ propagatedNativeBuildInputs ++ propagatedBuildInputs;
86+
8487
dependencies = map (map lib.chooseDevOutputs) [
8588
[
8689
(map (drv: drv.__spliced.buildBuild or drv) depsBuildBuild)
@@ -140,9 +143,12 @@ rec {
140143
(lib.concatLists propagatedDependencies));
141144
in
142145
{
143-
name = name + lib.optionalString
146+
# A hack to make `nix-env -qa` and `nix search` ignore broken packages.
147+
# TODO(@oxij): remove this assert when something like NixOS/nix#1771 gets merged into nix.
148+
name = assert validity.handled; name + lib.optionalString
144149
(stdenv.hostPlatform != stdenv.buildPlatform)
145150
("-" + stdenv.hostPlatform.config);
151+
146152
builder = attrs.realBuilder or stdenv.shell;
147153
args = attrs.args or ["-e" (attrs.builder or ./default-builder.sh)];
148154
inherit stdenv;
@@ -197,46 +203,59 @@ rec {
197203
doInstallCheck = doInstallCheck && (stdenv.hostPlatform == stdenv.buildPlatform);
198204
});
199205

206+
validity = import ./check-meta.nix {
207+
inherit lib config meta derivationArg;
208+
mkDerivationArg = attrs;
209+
# Nix itself uses the `system` field of a derivation to decide where
210+
# to build it. This is a bit confusing for cross compilation.
211+
inherit (stdenv) system;
212+
};
213+
200214
# The meta attribute is passed in the resulting attribute set,
201215
# but it's not part of the actual derivation, i.e., it's not
202216
# passed to the builder and is not a dependency. But since we
203217
# include it in the result, it *is* available to nix-env for queries.
204-
meta = { }
218+
meta = {
219+
# `name` above includes cross-compilation cruft (and is under assert),
220+
# lets have a clean always accessible version here.
221+
inherit name;
222+
205223
# If the packager hasn't specified `outputsToInstall`, choose a default,
206224
# which is the name of `p.bin or p.out or p`;
207225
# if he has specified it, it will be overridden below in `// meta`.
208226
# Note: This default probably shouldn't be globally configurable.
209227
# Services and users should specify outputs explicitly,
210228
# unless they are comfortable with this default.
211-
// { outputsToInstall =
212-
let
213-
outs = outputs'; # the value passed to derivation primitive
214-
hasOutput = out: builtins.elem out outs;
215-
in [( lib.findFirst hasOutput null (["bin" "out"] ++ outs) )];
229+
outputsToInstall =
230+
let
231+
outs = outputs'; # the value passed to derivation primitive
232+
hasOutput = out: builtins.elem out outs;
233+
in [( lib.findFirst hasOutput null (["bin" "out"] ++ outs) )];
216234
}
217235
// attrs.meta or {}
218-
# Fill `meta.position` to identify the source location of the package.
219-
// lib.optionalAttrs (pos != null)
220-
{ position = pos.file + ":" + toString pos.line; }
221-
;
236+
# Fill `meta.position` to identify the source location of the package.
237+
// lib.optionalAttrs (pos != null) {
238+
position = pos.file + ":" + toString pos.line;
239+
# Expose the result of the checks for everyone to see.
240+
} // {
241+
evaluates = validity.valid
242+
&& (if config.checkMetaRecursively or false
243+
then lib.all (d: d.meta.evaluates or true) references
244+
else true);
245+
};
222246

223247
in
224248

225-
lib.addPassthru
226-
(derivation (import ./check-meta.nix
227-
{
228-
inherit lib config meta derivationArg;
229-
mkDerivationArg = attrs;
230-
# Nix itself uses the `system` field of a derivation to decide where
231-
# to build it. This is a bit confusing for cross compilation.
232-
inherit (stdenv) system;
233-
}))
234-
( {
235-
overrideAttrs = f: mkDerivation (attrs // (f attrs));
236-
inherit meta passthru;
237-
} //
238-
# Pass through extra attributes that are not inputs, but
239-
# should be made available to Nix expressions using the
240-
# derivation (e.g., in assertions).
241-
passthru);
249+
lib.extendDerivation
250+
validity.handled
251+
({
252+
overrideAttrs = f: mkDerivation (attrs // (f attrs));
253+
inherit meta passthru;
254+
} //
255+
# Pass through extra attributes that are not inputs, but
256+
# should be made available to Nix expressions using the
257+
# derivation (e.g., in assertions).
258+
passthru)
259+
(derivation derivationArg);
260+
242261
}

0 commit comments

Comments
 (0)