Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdenv.mkDerivation: support output checks with and without structuredAttrs #357054

Merged
merged 3 commits into from
Nov 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions pkgs/by-name/ne/neovim-unwrapped/package.nix
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,7 @@ stdenv.mkDerivation (
find "$out" -type f -exec remove-references-to -t ${stdenv.cc} '{}' +
'';
# check that the above patching actually works
outputChecks =
let
disallowedRequisites = [ stdenv.cc ] ++ lib.optional (lua != codegenLua) codegenLua;
in
{
out = {
inherit disallowedRequisites;
};
debug = {
inherit disallowedRequisites;
};
};
disallowedRequisites = [ stdenv.cc ] ++ lib.optional (lua != codegenLua) codegenLua;

cmakeFlags =
[
Expand Down
10 changes: 4 additions & 6 deletions pkgs/servers/sql/postgresql/generic.nix
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,19 @@ let
disallowedReferences = [ "dev" "doc" "man" ];
disallowedRequisites = [
stdenv'.cc
llvmPackages.llvm.out
] ++ (
map lib.getDev (builtins.filter (drv: drv ? "dev") finalAttrs.buildInputs)
) ++ lib.optionals jitSupport [
llvmPackages.llvm.out
];
);
};
outputChecks.lib = {
disallowedReferences = [ "out" "dev" "doc" "man" ];
disallowedRequisites = [
stdenv'.cc
llvmPackages.llvm.out
] ++ (
map lib.getDev (builtins.filter (drv: drv ? "dev") finalAttrs.buildInputs)
) ++ lib.optionals jitSupport [
llvmPackages.llvm.out
];
);
};

buildInputs = [
Expand Down
82 changes: 48 additions & 34 deletions pkgs/stdenv/generic/make-derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ let
"__darwinAllowLocalNetworking"
"__impureHostDeps" "__propagatedImpureHostDeps"
"sandboxProfile" "propagatedSandboxProfile"
"disallowedReferences" "disallowedRequisites"
"allowedReferences" "allowedRequisites"
];

# Turn a derivation into its outPath without a string context attached.
Expand All @@ -143,6 +145,42 @@ let
then builtins.unsafeDiscardStringContext drv.outPath
else drv;

makeOutputChecks = attrs:
# If we use derivations directly here, they end up as build-time dependencies.
# This is especially problematic in the case of disallowed*, since the disallowed
# derivations will be built by nix as build-time dependencies, while those
# derivations might take a very long time to build, or might not even build
# successfully on the platform used.
# We can improve on this situation by instead passing only the outPath,
# without an attached string context, to nix. The out path will be a placeholder
# which will be replaced by the actual out path if the derivation in question
# is part of the final closure (and thus needs to be built). If it is not
# part of the final closure, then the placeholder will be passed along,
# but in that case we know for a fact that the derivation is not part of the closure.
# This means that passing the out path to nix does the right thing in either
# case, both for disallowed and allowed references/requisites, and we won't
# build the derivation if it wouldn't be part of the closure, saving time and resources.
# While the problem is less severe for allowed*, since we want the derivation
# to be built eventually, we would still like to get the error early and without
# having to wait while nix builds a derivation that might not be used.
# See also https://github.com/NixOS/nix/issues/4629
optionalAttrs (attrs ? disallowedReferences) {
disallowedReferences =
map unsafeDerivationToUntrackedOutpath attrs.disallowedReferences;
} //
optionalAttrs (attrs ? disallowedRequisites) {
disallowedRequisites =
map unsafeDerivationToUntrackedOutpath attrs.disallowedRequisites;
} //
optionalAttrs (attrs ? allowedReferences) {
allowedReferences =
mapNullable unsafeDerivationToUntrackedOutpath attrs.allowedReferences;
} //
optionalAttrs (attrs ? allowedRequisites) {
allowedRequisites =
mapNullable unsafeDerivationToUntrackedOutpath attrs.allowedRequisites;
};

makeDerivationArgument =


Expand Down Expand Up @@ -455,40 +493,16 @@ else let
"/bin/sh"
];
__propagatedImpureHostDeps = computedPropagatedImpureHostDeps ++ __propagatedImpureHostDeps;
}) //
# If we use derivations directly here, they end up as build-time dependencies.
# This is especially problematic in the case of disallowed*, since the disallowed
# derivations will be built by nix as build-time dependencies, while those
# derivations might take a very long time to build, or might not even build
# successfully on the platform used.
# We can improve on this situation by instead passing only the outPath,
# without an attached string context, to nix. The out path will be a placeholder
# which will be replaced by the actual out path if the derivation in question
# is part of the final closure (and thus needs to be built). If it is not
# part of the final closure, then the placeholder will be passed along,
# but in that case we know for a fact that the derivation is not part of the closure.
# This means that passing the out path to nix does the right thing in either
# case, both for disallowed and allowed references/requisites, and we won't
# build the derivation if it wouldn't be part of the closure, saving time and resources.
# While the problem is less severe for allowed*, since we want the derivation
# to be built eventually, we would still like to get the error early and without
# having to wait while nix builds a derivation that might not be used.
# See also https://github.com/NixOS/nix/issues/4629
optionalAttrs (attrs ? disallowedReferences) {
disallowedReferences =
map unsafeDerivationToUntrackedOutpath attrs.disallowedReferences;
} //
optionalAttrs (attrs ? disallowedRequisites) {
disallowedRequisites =
map unsafeDerivationToUntrackedOutpath attrs.disallowedRequisites;
} //
optionalAttrs (attrs ? allowedReferences) {
allowedReferences =
mapNullable unsafeDerivationToUntrackedOutpath attrs.allowedReferences;
} //
optionalAttrs (attrs ? allowedRequisites) {
allowedRequisites =
mapNullable unsafeDerivationToUntrackedOutpath attrs.allowedRequisites;
}) // lib.optionalAttrs (!__structuredAttrs) (
makeOutputChecks attrs
) // lib.optionalAttrs (__structuredAttrs) {
outputChecks = builtins.listToAttrs (map (name: {
inherit name;
value = lib.zipAttrsWith (_: builtins.concatLists) [
Comment on lines +496 to +501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functions should be inherited from lib at the top of the file for improved perf

(makeOutputChecks attrs)
(makeOutputChecks attrs.outputChecks.${name} or {})
];
}) outputs);
};

in
Expand Down