Skip to content
Closed
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
3 changes: 3 additions & 0 deletions lib/systems/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,9 @@ let
"-uefi"
];
};
}
// {
override = allArgs.override or (newArgs: elaborate (allArgs // newArgs));
Copy link
Contributor

@MattSturgeon MattSturgeon Apr 3, 2025

Choose a reason for hiding this comment

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

Does this work as intended when chained?

E.g.

(
  (
    lib.systems.elaborate "x86_64-linux"
  ).override { foo = true; }
).override { bar = true; }
Using lib.pipe

lib.pipe "x86_64-linux" [
  lib.systems.elaborate
  (p: p.override { foo = true; })
  (p: p.override { bar = true; })
]

In the above example, the "original" allArgs is { system = "x86_64-linux"; }, so:

override = newArgs: { system = "x86_64-linux"; } // newArgs

The first time .override is used, I belive you'd get { system = "x86_64-linux"; } // { foo = true; },
while the second time I think you'd get { system = "x86_64-linux"; } // { bar = true; }.

Notice that the foo attr is lost on subsequent uses of .override. If .override was used a third time, bar would also be missing.

EDIT: actually, I think I oversimplified this in my head. Since override usually isn't part of allArgs, I believe it should chain correctly. At lease, up until the point when an elaborated system is supplied to override.
I think I'll need to experiment in the repl to verify my thought process here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EDIT: actually, I think I oversimplified this in my head. Since override usually isn't part of allArgs, I believe it should chain correctly. At lease, up until the point when an elaborated system is supplied to override.
I think I'll need to experiment in the repl to verify my thought process here.

I'd think so, too - because I have quite a few tests chaining things (which is the whole point of this PR!), so I assume it does work correctly.

};
in
assert final.useAndroidPrebuilt -> final.isAndroid;
Expand Down
1 change: 1 addition & 0 deletions lib/tests/systems.nix
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ lib.runTests (
emulatorAvailable = null;
staticEmulatorAvailable = null;
isCompatible = null;
override = null;
} ? ${platformAttrName};
};

Expand Down
122 changes: 122 additions & 0 deletions pkgs/test/top-level/stage.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# run like this:
# nix-build pkgs/test/top-level/stage.nix
{
localSystem ? {
system = builtins.currentSystem;
},
}:

with import ../../top-level { inherit localSystem; };

let
# To silence platform specific evaluation errors
discardEvaluationErrors = e: (builtins.tryEval e).success -> e;

# Basic test for idempotency of the package set, i.e:
# Applying the same package set twice should work and
# not change anything.
isIdempotent = set: discardEvaluationErrors (pkgs.${set}.stdenv == pkgs.${set}.${set}.stdenv);

# Some package sets should be noops in certain circumstances.
# This is very similar to the idempotency test, but not going
# via the super' overlay.
isNoop =
parent: child:
discardEvaluationErrors (
(lib.getAttrFromPath parent pkgs).stdenv == (lib.getAttrFromPath parent pkgs).${child}.stdenv
);

allMuslExamples = builtins.attrNames (
lib.filterAttrs (_: system: lib.hasSuffix "-musl" system.config) lib.systems.examples
);

allLLVMExamples = builtins.attrNames (
lib.filterAttrs (_: system: system.useLLVM or false) lib.systems.examples
);

# A package set should only change specific configuration, but needs
# to keep all other configuration from previous layers in place.
# Each package set has one or more key characteristics for which we
# test here. Those should be kept, even when applying the "set" package
# set.
isComposable =
set:
lib.all discardEvaluationErrors (
[
(pkgsCross.ppc64-musl.${set}.stdenv.hostPlatform.gcc.abi == "elfv2")
(pkgs.pkgsLLVM.${set}.stdenv.hostPlatform.useLLVM)
(pkgs.pkgsArocc.${set}.stdenv.hostPlatform.useArocc)
(pkgs.pkgsZig.${set}.stdenv.hostPlatform.useZig)
(pkgs.pkgsLinux.${set}.stdenv.buildPlatform.isLinux)
(pkgs.pkgsStatic.${set}.stdenv.hostPlatform.isStatic)
(pkgs.pkgsi686Linux.${set}.stdenv.hostPlatform.isx86_32)
(pkgs.pkgsx86_64Darwin.${set}.stdenv.hostPlatform.isx86_64)
(builtins.elem "trivialautovarinit" pkgs.pkgsExtraHardening.${set}.stdenv.cc.defaultHardeningFlags)
] # Can't compose two different libcs...
++ lib.optionals (!builtins.elem set [ "pkgsLLVMLibc" ]) [
(pkgsCross.mingwW64.${set}.stdenv.hostPlatform.config == "x86_64-w64-mingw32")
(pkgsCross.mingwW64.${set}.stdenv.hostPlatform.libc == "msvcrt")
(pkgs.pkgsMusl.${set}.stdenv.hostPlatform.isMusl)
]
++
lib.optionals
(
!builtins.elem set [
"pkgsMusl"
"pkgsStatic"
]
)
[
(pkgs.pkgsLLVMLibc.${set}.stdenv.hostPlatform.isLLVMLibc)
]
);
in

# Appends same defaultHardeningFlags again on each .pkgsExtraHardening - thus not idempotent.
# assert isIdempotent "pkgsExtraHardening";
# TODO: Remove the isDarwin condition, which currently results in infinite recursion.
# Also see https://github.com/NixOS/nixpkgs/pull/330567#discussion_r1894653309
assert (stdenv.hostPlatform.isDarwin || isIdempotent "pkgsLLVM");
# TODO: This currently results in infinite recursion, even on Linux
# assert isIdempotent "pkgsLLVMLibc";
assert isIdempotent "pkgsArocc";
assert isIdempotent "pkgsZig";
assert isIdempotent "pkgsLinux";
assert isIdempotent "pkgsMusl";
assert isIdempotent "pkgsStatic";
assert isIdempotent "pkgsi686Linux";
assert isIdempotent "pkgsx86_64Darwin";

assert isNoop [ "pkgsStatic" ] "pkgsMusl";
assert lib.all (sys: isNoop [ "pkgsCross" sys ] "pkgsMusl") allMuslExamples;
# This will fail on all systems which are not handled in pkgs/os-specific/bsd/openbsd/pkgs/mkDerivation.nix,
# for example armv6l-linux, powerpc64le-linux, riscv64-linux
assert lib.all (sys: isNoop [ "pkgsCross" sys ] "pkgsLLVM") allLLVMExamples;

assert isComposable "pkgsExtraHardening";
assert isComposable "pkgsLLVM";
# TODO: Results in infinite recursion
# assert isComposable "pkgsLLVMLibc";
assert isComposable "pkgsArocc";
assert isComposable "pkgsZig";
assert isComposable "pkgsMusl";
assert isComposable "pkgsStatic";
assert isComposable "pkgsi686Linux";

# Special cases regarding buildPlatform vs hostPlatform
assert discardEvaluationErrors (pkgsCross.gnu64.pkgsMusl.stdenv.hostPlatform.isMusl);
assert discardEvaluationErrors (pkgsCross.gnu64.pkgsi686Linux.stdenv.hostPlatform.isx86_32);
assert discardEvaluationErrors (pkgsCross.mingwW64.pkgsLinux.stdenv.hostPlatform.isLinux);
assert discardEvaluationErrors (
pkgsCross.aarch64-darwin.pkgsx86_64Darwin.stdenv.hostPlatform.isx86_64
);

# pkgsCross should keep upper cross settings
assert discardEvaluationErrors (
with pkgsStatic.pkgsCross.gnu64.stdenv.hostPlatform; isGnu && isStatic
);
assert discardEvaluationErrors (
with pkgsLLVM.pkgsCross.musl64.stdenv.hostPlatform; isMusl && useLLVM
);

emptyFile
16 changes: 6 additions & 10 deletions pkgs/top-level/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -134,22 +134,18 @@ let
config = lib.showWarnings configEval.config.warnings configEval.config;

# A few packages make a new package set to draw their dependencies from.
# (Currently to get a cross tool chain, or forced-i686 package.) Rather than
# give `all-packages.nix` all the arguments to this function, even ones that
# don't concern it, we give it this function to "re-call" nixpkgs, inheriting
# whatever arguments it doesn't explicitly provide. This way,
# `all-packages.nix` doesn't know more than it needs too.
# Rather than give `all-packages.nix` all the arguments to this function,
# even ones that don't concern it, we give it this function to "re-call"
# nixpkgs, inheriting whatever arguments it doesn't explicitly provide. This
# way, `all-packages.nix` doesn't know more than it needs to.
#
# It's OK that `args` doesn't include default arguments from this file:
# they'll be deterministically inferred. In fact we must *not* include them,
# because it's important that if some parameter which affects the default is
# substituted with a different argument, the default is re-inferred.
#
# To put this in concrete terms, this function is basically just used today to
# use package for a different platform for the current platform (namely cross
# compiling toolchains and 32-bit packages on x86_64). In both those cases we
# want the provided non-native `localSystem` argument to affect the stdenv
# chosen.
# To put this in concrete terms, we want the provided non-native `localSystem`
# and `crossSystem` arguments to affect the stdenv chosen.
#
# NB!!! This thing gets its `config` argument from `args`, i.e. it's actually
# `config0`. It is important to keep it to `config0` format (as opposed to the
Expand Down
Loading