Skip to content

Commit

Permalink
stdenv: Fix overriding + overrideAttrs
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Ericson2314
Ericson2314 committed Aug 18, 2021
1 parent aa04562 commit f110a18
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 84 deletions.
1 change: 1 addition & 0 deletions pkgs/os-specific/linux/musl/default.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ stdenv.mkDerivation rec {
outputs = [ "out" "dev" ];

dontDisableStatic = true;
dontAddStaticConfigureFlags = true;
separateDebugInfo = true;

NIX_DONT_SET_RPATH = true;
Expand Down
125 changes: 76 additions & 49 deletions pkgs/stdenv/adapters.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -31,52 +55,52 @@ 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"
];
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
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
});
};
});
}
8 changes: 5 additions & 3 deletions pkgs/stdenv/generic/default.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down
13 changes: 6 additions & 7 deletions pkgs/stdenv/generic/make-derivation.nix
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ lib, config, stdenv }:
{ lib, config }:

stdenv:

let
checkMeta = import ./check-meta.nix {
Expand All @@ -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.
#
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
7 changes: 6 additions & 1 deletion pkgs/top-level/stage.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
24 changes: 0 additions & 24 deletions pkgs/top-level/static.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down Expand Up @@ -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
Expand Down

1 comment on commit f110a18

@bluescreen303
Copy link
Contributor

Choose a reason for hiding this comment

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

this commit broke enableDebugging, which I use in private projects where I need some libraries to be compiled with debugging symbols.
It errors out with:

error: attribute 'override' missing, at ~/nixpkgs/pkgs/stdenv/adapters.nix:210:5

it's not clear to me where 'override' is missing. Both the derivation I pass into enableDebugging as stdenv do have 'override'.

Please sign in to comment.