From f110a182a66005782c0e58091bcda7243bf2a0ae Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Aug 2021 21:16:29 -0400 Subject: [PATCH] stdenv: Fix overriding + `overrideAttrs` The old stdenv adapters were subtly wrong in two ways: - `overrideAttrs` leaked the original, unoverridden `mkDerivation`. - `stdenv.override` would throw away any manually-set `mkDerivation` from a stdenv reverting to the original. Now, `mkDerivation` is controlled (nearly directly) via an argument, and always correctly closes over the final ("self") stdenv. This means the adapters can work entirely via `.override` without any manual `stdenv // ...`, and both those issues are fixed. Note hashes are changed, because stdenvs no previously overridden like `stdenvNoCC` and `crossLibcStdenv` now are. I had to add some `dontDisableStatic = true` accordingly. The flip side however is that since the overrides compose, we no longer need to override anything but the default `stdenv` from which all the others are created. --- pkgs/os-specific/linux/musl/default.nix | 1 + pkgs/stdenv/adapters.nix | 125 ++++++++++++++---------- pkgs/stdenv/generic/default.nix | 8 +- pkgs/stdenv/generic/make-derivation.nix | 13 ++- pkgs/top-level/stage.nix | 7 +- pkgs/top-level/static.nix | 24 ----- 6 files changed, 94 insertions(+), 84 deletions(-) diff --git a/pkgs/os-specific/linux/musl/default.nix b/pkgs/os-specific/linux/musl/default.nix index ae175a363241..f19c7ea7a44b 100644 --- a/pkgs/os-specific/linux/musl/default.nix +++ b/pkgs/os-specific/linux/musl/default.nix @@ -82,6 +82,7 @@ stdenv.mkDerivation rec { outputs = [ "out" "dev" ]; dontDisableStatic = true; + dontAddStaticConfigureFlags = true; separateDebugInfo = true; NIX_DONT_SET_RPATH = true; diff --git a/pkgs/stdenv/adapters.nix b/pkgs/stdenv/adapters.nix index a8e984d61743..719f67998266 100644 --- a/pkgs/stdenv/adapters.nix +++ b/pkgs/stdenv/adapters.nix @@ -2,7 +2,31 @@ a new stdenv with different behaviour, e.g. using a different C compiler. */ -pkgs: +{ lib, pkgs, config }: + +let + # N.B. Keep in sync with default arg for stdenv/generic. + defaultMkDerivationFromStdenv = import ./generic/make-derivation.nix { inherit lib config; }; + + # Low level function to help with overriding `mkDerivationFromStdenv`. One + # gives it the old stdenv arguments and a "continuation" function, and + # underneath the final stdenv argument it yields to the continuation to do + # whatever it wants with old `mkDerivation` (old `mkDerivationFromStdenv` + # applied to the *new, final* stdenv) provided for convenience. + withOldMkDerivation = stdenvSuperArgs: k: stdenvSelf: let + mkDerivationFromStdenv-super = stdenvSuperArgs.mkDerivationFromStdenv or defaultMkDerivationFromStdenv; + mkDerivationSuper = mkDerivationFromStdenv-super stdenvSelf; + in + k stdenvSelf mkDerivationSuper; + + # Wrap the original `mkDerivation` providing extra args to it. + extendMkDerivationArgs = old: f: withOldMkDerivation old (_: mkDerivationSuper: args: + mkDerivationSuper (args // f args)); + + # Wrap the original `mkDerivation` transforming the result. + overrideMkDerivationResult = old: f: withOldMkDerivation old (_: mkDerivationSuper: args: + f (mkDerivationSuper args)); +in rec { @@ -31,33 +55,32 @@ rec { # Return a modified stdenv that tries to build statically linked # binaries. - makeStaticBinaries = stdenv: - let stdenv' = if stdenv.hostPlatform.libc != "glibc" then stdenv else - stdenv.override (prev: { - extraBuildInputs = (prev.extraBuildInputs or []) ++ [ - stdenv.glibc.static - ]; - }); - in stdenv' // - { mkDerivation = args: - if stdenv'.hostPlatform.isDarwin + makeStaticBinaries = stdenv0: + stdenv0.override (old: { + mkDerivationFromStdenv = withOldMkDerivation old (stdenv: mkDerivationSuper: args: + if stdenv.hostPlatform.isDarwin then throw "Cannot build fully static binaries on Darwin/macOS" - else stdenv'.mkDerivation (args // { + else mkDerivationSuper (args // { NIX_CFLAGS_LINK = toString (args.NIX_CFLAGS_LINK or "") + " -static"; - } // pkgs.lib.optionalAttrs (!(args.dontAddStaticConfigureFlags or false)) { + } // lib.optionalAttrs (!(args.dontAddStaticConfigureFlags or false)) { configureFlags = (args.configureFlags or []) ++ [ "--disable-shared" # brrr... ]; - }); - }; + })); + } // lib.optionalAttrs (stdenv0.hostPlatform.libc == "libc") { + extraBuildInputs = (old.extraBuildInputs or []) ++ [ + stdenv0.glibc.static + ]; + }); # Return a modified stdenv that builds static libraries instead of # shared libraries. - makeStaticLibraries = stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // { + makeStaticLibraries = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (args: { dontDisableStatic = true; - } // pkgs.lib.optionalAttrs (!(args.dontAddStaticConfigureFlags or false)) { + } // lib.optionalAttrs (!(args.dontAddStaticConfigureFlags or false)) { configureFlags = (args.configureFlags or []) ++ [ "--enable-static" "--disable-shared" @@ -65,18 +88,19 @@ rec { cmakeFlags = (args.cmakeFlags or []) ++ [ "-DBUILD_SHARED_LIBS:BOOL=OFF" ]; mesonFlags = (args.mesonFlags or []) ++ [ "-Ddefault_library=static" ]; }); - }; + }); /* Modify a stdenv so that all buildInputs are implicitly propagated to consuming derivations */ - propagateBuildInputs = stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // { + propagateBuildInputs = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (args: { propagatedBuildInputs = (args.propagatedBuildInputs or []) ++ (args.buildInputs or []); buildInputs = []; }); - }; + }); /* Modify a stdenv so that the specified attributes are added to @@ -88,8 +112,9 @@ rec { { NIX_CFLAGS_COMPILE = "-O0"; } stdenv; */ - addAttrsToDerivation = extraAttrs: stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // extraAttrs); }; + addAttrsToDerivation = extraAttrs: stdenv: stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (_: extraAttrs); + }); /* Return a modified stdenv that builds packages with GCC's coverage @@ -110,21 +135,20 @@ rec { # remove all maintainers. defaultStdenv = replaceMaintainersField allStdenvs.stdenv pkgs []; */ - replaceMaintainersField = stdenv: pkgs: maintainers: stdenv // - { mkDerivation = args: - pkgs.lib.recursiveUpdate - (stdenv.mkDerivation args) - { meta.maintainers = maintainers; }; - }; + replaceMaintainersField = stdenv: pkgs: maintainers: + stdenv.override (old: { + mkDerivationFromStdenv = overrideMkDerivationResult (pkg: + lib.recursiveUpdate pkg { meta.maintainers = maintainers; }); + }); /* Use the trace output to report all processed derivations with their license name. */ - traceDrvLicenses = stdenv: stdenv // - { mkDerivation = args: + traceDrvLicenses = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = overrideMkDerivationResult (pkg: let - pkg = stdenv.mkDerivation args; printDrvPath = val: let drvPath = builtins.unsafeDiscardStringContext pkg.drvPath; license = pkg.meta.license or null; @@ -133,8 +157,8 @@ rec { in pkg // { outPath = printDrvPath pkg.outPath; drvPath = printDrvPath pkg.drvPath; - }; - }; + }); + }); /* Abort if the license predicate is not verified for a derivation @@ -152,10 +176,10 @@ rec { use it by patching the all-packages.nix file or by using the override feature of ~/.config/nixpkgs/config.nix . */ - validateLicenses = licensePred: stdenv: stdenv // - { mkDerivation = args: + validateLicenses = licensePred: stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = overrideMkDerivationResult (pkg: let - pkg = stdenv.mkDerivation args; drv = builtins.unsafeDiscardStringContext pkg.drvPath; license = pkg.meta.license or @@ -175,40 +199,43 @@ rec { in pkg // { outPath = validate pkg.outPath; drvPath = validate pkg.drvPath; - }; - }; + }); + }); /* Modify a stdenv so that it produces debug builds; that is, binaries have debug info, and compiler optimisations are disabled. */ - keepDebugInfo = stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // { + keepDebugInfo = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (args: { dontStrip = true; NIX_CFLAGS_COMPILE = toString (args.NIX_CFLAGS_COMPILE or "") + " -ggdb -Og"; }); - }; + }); /* Modify a stdenv so that it uses the Gold linker. */ - useGoldLinker = stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // { + useGoldLinker = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (args: { NIX_CFLAGS_LINK = toString (args.NIX_CFLAGS_LINK or "") + " -fuse-ld=gold"; }); - }; + }); /* Modify a stdenv so that it builds binaries optimized specifically for the machine they are built on. WARNING: this breaks purity! */ - impureUseNativeOptimizations = stdenv: stdenv // - { mkDerivation = args: stdenv.mkDerivation (args // { + impureUseNativeOptimizations = stdenv: + stdenv.override (old: { + mkDerivationFromStdenv = extendMkDerivationArgs old (args: { NIX_CFLAGS_COMPILE = toString (args.NIX_CFLAGS_COMPILE or "") + " -march=native"; NIX_ENFORCE_NO_NATIVE = false; preferLocalBuild = true; allowSubstitutes = false; }); - }; + }); } diff --git a/pkgs/stdenv/generic/default.nix b/pkgs/stdenv/generic/default.nix index 88ca1b2c7903..d7fb1b0ba063 100644 --- a/pkgs/stdenv/generic/default.nix +++ b/pkgs/stdenv/generic/default.nix @@ -48,6 +48,10 @@ let lib = import ../../../lib; in lib.makeOverridable ( , # The platform which build tools (especially compilers) build for in this stage, targetPlatform + +, # The implementation of `mkDerivation`, parameterized with the final stdenv so we can tie the knot. + # This is convient to have as a parameter so the stdenv "adapters" work better + mkDerivationFromStdenv ? import ./make-derivation.nix { inherit lib config; } }: let @@ -155,9 +159,7 @@ let # to correct type of machine. inherit (hostPlatform) system; - inherit (import ./make-derivation.nix { - inherit lib config stdenv; - }) mkDerivation; + mkDerivation = mkDerivationFromStdenv stdenv; inherit fetchurlBoot; diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index d6704d59111a..c151516130a1 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -1,4 +1,6 @@ -{ lib, config, stdenv }: +{ lib, config }: + +stdenv: let checkMeta = import ./check-meta.nix { @@ -7,7 +9,7 @@ let # to build it. This is a bit confusing for cross compilation. inherit (stdenv) hostPlatform; }; -in rec { +in # `mkDerivation` wraps the builtin `derivation` function to # produce derivations that use this stdenv and its shell. # @@ -18,7 +20,6 @@ in rec { # # * https://nixos.org/nix/manual/#ssec-derivation # Explanation about derivations in general - mkDerivation = { # These types of dependencies are all exhaustively documented in @@ -373,7 +374,7 @@ in rec { lib.extendDerivation validity.handled ({ - overrideAttrs = f: mkDerivation (attrs // (f attrs)); + overrideAttrs = f: stdenv.mkDerivation (attrs // (f attrs)); # A derivation that always builds successfully and whose runtime # dependencies are the original derivations build time dependencies @@ -406,6 +407,4 @@ in rec { # should be made available to Nix expressions using the # derivation (e.g., in assertions). passthru) - (derivation derivationArg); - -} + (derivation derivationArg) diff --git a/pkgs/top-level/stage.nix b/pkgs/top-level/stage.nix index dc43bbec9d33..b01ef584d206 100644 --- a/pkgs/top-level/stage.nix +++ b/pkgs/top-level/stage.nix @@ -65,7 +65,12 @@ let stdenvAdapters = self: super: - let res = import ../stdenv/adapters.nix self; in res // { + let + res = import ../stdenv/adapters.nix { + inherit lib config; + pkgs = self; + }; + in res // { stdenvAdapters = res; }; diff --git a/pkgs/top-level/static.nix b/pkgs/top-level/static.nix index 0c9af250e876..73d8d9e53032 100644 --- a/pkgs/top-level/static.nix +++ b/pkgs/top-level/static.nix @@ -35,9 +35,6 @@ self: super: let }; staticAdapters = - # makeStaticDarwin must go first so that the extraBuildInputs - # override does not recreate mkDerivation, removing subsequent - # adapters. optional super.stdenv.hostPlatform.isDarwin makeStaticDarwin ++ [ makeStaticLibraries propagateBuildInputs ] @@ -80,30 +77,9 @@ self: super: let }); }; - llvmStaticAdapter = llvmPackages: - llvmPackages // { - stdenv = foldl (flip id) llvmPackages.stdenv staticAdapters; - libcxxStdenv = foldl (flip id) llvmPackages.libcxxStdenv staticAdapters; - }; - in { stdenv = foldl (flip id) super.stdenv staticAdapters; - gcc49Stdenv = foldl (flip id) super.gcc49Stdenv staticAdapters; - gcc6Stdenv = foldl (flip id) super.gcc6Stdenv staticAdapters; - gcc7Stdenv = foldl (flip id) super.gcc7Stdenv staticAdapters; - gcc8Stdenv = foldl (flip id) super.gcc8Stdenv staticAdapters; - gcc9Stdenv = foldl (flip id) super.gcc9Stdenv staticAdapters; - - llvmPackages_5 = llvmStaticAdapter super.llvmPackages_5; - llvmPackages_6 = llvmStaticAdapter super.llvmPackages_6; - llvmPackages_7 = llvmStaticAdapter super.llvmPackages_7; - llvmPackages_8 = llvmStaticAdapter super.llvmPackages_8; - llvmPackages_9 = llvmStaticAdapter super.llvmPackages_9; - llvmPackages_10 = llvmStaticAdapter super.llvmPackages_10; - llvmPackages_11 = llvmStaticAdapter super.llvmPackages_11; - llvmPackages_12 = llvmStaticAdapter super.llvmPackages_12; - boost = super.boost.override { # Don’t use new stdenv for boost because it doesn’t like the # --disable-shared flag