From da1e1a03cb7d185d22a91e4d1f930946d061dae8 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 16 Nov 2024 20:42:26 +0100 Subject: [PATCH 1/3] stdenv.mkDerivation: refactor output checks This just moves the code around a little bit to be able to build on it in follow up commits without too much of a diff. Adding to removedOrReplacedAttrNames is just a cleanup right now and doesn't change anything functionally, yet. This will be required for later on to avoid having structured outputChecks and those directly on the derivation at the same time. --- pkgs/stdenv/generic/make-derivation.nix | 76 +++++++++++++------------ 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index d4d5ee46638a..c9ae4cf1fd9e 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -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. @@ -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 = @@ -455,41 +493,9 @@ 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; - }; + }) // ( + makeOutputChecks attrs + ); in derivationArg; From 5bff902d5ec322f8ad238c48a050f43a7b5f32cd Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 16 Nov 2024 22:13:30 +0100 Subject: [PATCH 2/3] stdenv.mkDerivation: avoid depending on derivations passed in outputChecks This was added for non-structuredAttrs output checks in #211783. Here we extend the same concept to structuredAttrs-enabled outputChecks, too. The postgresql package worked around this with some conditionals. Those can now be removed - without causing LLVM to be built or substituted. --- pkgs/servers/sql/postgresql/generic.nix | 10 ++++------ pkgs/stdenv/generic/make-derivation.nix | 4 +++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkgs/servers/sql/postgresql/generic.nix b/pkgs/servers/sql/postgresql/generic.nix index 79111b589938..afb06a24b789 100644 --- a/pkgs/servers/sql/postgresql/generic.nix +++ b/pkgs/servers/sql/postgresql/generic.nix @@ -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 = [ diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index c9ae4cf1fd9e..fd3a51e06e42 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -495,7 +495,9 @@ else let __propagatedImpureHostDeps = computedPropagatedImpureHostDeps ++ __propagatedImpureHostDeps; }) // ( makeOutputChecks attrs - ); + ) // lib.optionalAttrs (__structuredAttrs) { + outputChecks = builtins.mapAttrs (_: makeOutputChecks) attrs.outputChecks or {}; + }; in derivationArg; From d37f90a2b3ca3716e9297478e2a6827798ffd7cf Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 16 Nov 2024 22:58:09 +0100 Subject: [PATCH 3/3] stdenv.mkDerivation: support output checks with structuredAttrs Once __structuredAttrs are enabled, nix only supports disallowedReferences and friends inside the outputChecks attribute set, defined specifically for each output. Top-level disallowedReferences, as used throughout nixpkgs without structuredAttrs, throws a warning instead: warning: In a derivation named 'perl-5.40.0', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks..disallowedReferences' instead To support a seamless migration to enabling structuredAttrs by default, those derivation attributes are now mapped to each output separately when structuredAttrs are enabled. Since both top-level disallowedReferences and outputChecks can be given at the same time, those are now merged together. One package that can be simplified this way is neovim, because all checks should be applied to all outputs anyway. --- pkgs/by-name/ne/neovim-unwrapped/package.nix | 13 +------------ pkgs/stdenv/generic/make-derivation.nix | 10 ++++++++-- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/pkgs/by-name/ne/neovim-unwrapped/package.nix b/pkgs/by-name/ne/neovim-unwrapped/package.nix index fb185d85f646..2507934eca13 100644 --- a/pkgs/by-name/ne/neovim-unwrapped/package.nix +++ b/pkgs/by-name/ne/neovim-unwrapped/package.nix @@ -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 = [ diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index fd3a51e06e42..b640474e3ab3 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -493,10 +493,16 @@ else let "/bin/sh" ]; __propagatedImpureHostDeps = computedPropagatedImpureHostDeps ++ __propagatedImpureHostDeps; - }) // ( + }) // lib.optionalAttrs (!__structuredAttrs) ( makeOutputChecks attrs ) // lib.optionalAttrs (__structuredAttrs) { - outputChecks = builtins.mapAttrs (_: makeOutputChecks) attrs.outputChecks or {}; + outputChecks = builtins.listToAttrs (map (name: { + inherit name; + value = lib.zipAttrsWith (_: builtins.concatLists) [ + (makeOutputChecks attrs) + (makeOutputChecks attrs.outputChecks.${name} or {}) + ]; + }) outputs); }; in