Skip to content

Commit

Permalink
Revert patching functionality and fix config handling
Browse files Browse the repository at this point in the history 
The patching functionality is fundamentally ill designed
and makes the code complex to the point that it is increasingly
hard to audit it for correctness.

The root of evil and ill-design is that it is a work-around for
what the Unoficial Flake Roadmap describes as follows:
- flake inputs are flake inputs
- patched flake inputs are flake inputs

Furthermore this commit fixes a naive implementation of nixpkgs.config
handling that causes nasty regressions on edge cases. Namely, not only
can a nixpkgs config be a function, but also the merging semantics
as defined by the module system requires detailed knowledge of the
potential shape of the cofig attrs / function.
  • Loading branch information
@blaggacao
blaggacao committed Sep 5, 2021
1 parent a4e267e commit ed14e98
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 217 deletions.
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ jobs:

# Execute /tests/*
- run: nix develop --command check-derivation-outputs
- run: nix develop --command check-channel-patching
- run: nix develop --command check-overlays-flow
- run: nix develop --command check-hosts-config

Expand Down
4 changes: 0 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ Main flake-utils-plus features (Attributes visible from `flake.nix`):
- Option [`nix.generateNixPathFromInputs`](./lib/options.nix) - Generate `nix.nixPath` from available inputs.
- Option [`nix.linkInputs`](./lib/options.nix) - Symlink inputs to /etc/nix/inputs.
- Simple and clean support for multiple `nixpkgs` references.
- `nixpkgs` references patching.
- `channelsConfig` - Config applied to all `nixpkgs` references.
- `hostDefaults` - Default configuration shared between host definitions.
- `outputsBuilder` - Clean way to export packages/apps/etc.
Expand Down Expand Up @@ -92,9 +91,6 @@ in flake-utils-plus.lib.mkFlake {
# Channel specific config options.
channels.<name>.config = { allowUnfree = true; };
# Patches to apply on selected channel.
channels.<name>.patches = [ ./someAwesomePatch.patch ];
# Overlays to apply on a selected channel.
channels.<name>.overlaysBuilder = channels: [
(final: prev: { inherit (channels.unstable) neovim; })
Expand Down
3 changes: 1 addition & 2 deletions devShell.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,10 @@ devshell.mkShell {
command = "fd --extension nix --exec nix-instantiate --parse --quiet {} >/dev/null";
}

(test "channel-patching")
(test "derivation-outputs")
(test "hosts-config")
(test "overlays-flow")
(test "all" // { command = "check-channel-patching && check-derivation-outputs && check-hosts-config && check-overlays-flow"; })
(test "all" // { command = "check-derivation-outputs && check-hosts-config && check-overlays-flow"; })

(dry-nixos-build "minimal-multichannel" "Hostname1")
(dry-nixos-build "minimal-multichannel" "Hostname2")
Expand Down
9 changes: 0 additions & 9 deletions flake.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,6 @@
else value
)
rhs;

patchChannel = system: channel: patches:
if patches == [ ] then channel else
(import channel { inherit system; }).pkgs.applyPatches {
name = if channel ? shortRev then "nixpkgs-patched-${channel.shortRev}" else "nixpkgs-patched";
src = channel;
patches = patches;
};

};
};
}
Expand Down
194 changes: 113 additions & 81 deletions lib/mkFlake.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ let
inherit (flake-utils-plus.lib)
eachSystem
mergeAny
patchChannel
;
inherit (flake-utils-plus.lib.internal)
filterAttrs
partitionString
reverseList
;
inherit (builtins)
pathExists
attrNames
attrValues
concatMap
Expand All @@ -38,10 +36,13 @@ let
foldl'
genList
head
isAttrs
isFunction
isString
length
listToAttrs
mapAttrs
pathExists
removeAttrs
split
tail
Expand Down Expand Up @@ -93,6 +94,20 @@ let
)
channels;
getNixpkgs = host: (getChannels host.system).${host.channelName};
mergeNixpkgsConfigs = input: lconf: rconf: (
input.lib.evalModules {
modules = [
"${input}/nixos/modules/misc/assertions.nix"
"${input}/nixos/modules/misc/nixpkgs.nix"
{
nixpkgs.config = lconf;
}
{
nixpkgs.config = rconf;
}
];
}
).config.nixpkgs.config;

configurationBuilder = reverseDomainName: host': (
let
Expand All @@ -105,99 +120,111 @@ let
if domainLabels == [ ] then (lib.mkDefault null) # null is the networking.domain option's default
else concatStringsSep "." domainLabels;

selectedNixpkgs = getNixpkgs host;
host = evalHostArgs (mergeAny hostDefaults host');
patchedChannel = selectedNixpkgs.path;
selectedNixpkgs = getNixpkgs host;
channels' = getChannels host.system;
lib = selectedNixpkgs.lib;

specialArgs = host.specialArgs // { channel = selectedNixpkgs; };

/* nixos specific arguments */
# Use lib from patched nixpkgs
lib = selectedNixpkgs.lib;
# Use nixos modules from patched nixpkgs
baseModules = import (patchedChannel + "/nixos/modules/module-list.nix");
nixosSpecialArgs =
let
f = channelName:
{ "${channelName}ModulesPath" = toString (channels'.${channelName}.input + "/nixos/modules"); };
in
# Add `<channelName>ModulesPath`s
(foldl' (lhs: rhs: lhs // rhs) { } (map f (attrNames channels')))
# Override `modulesPath` because otherwise imports from there will not use patched nixpkgs
// { modulesPath = toString (patchedChannel + "/nixos/modules"); };


# The only way to find out if a host has `nixpkgs.config` set to
# the non-default value is by evalling most of the config.
hostConfig = (lib.evalModules {
prefix = [ ];
check = false;
modules = baseModules ++ host.modules;
args = { inherit inputs; } // host.extraArgs;
specialArgs = nixosSpecialArgs // specialArgs;
}).config;
in
{
${host.output}.${reverseDomainName} = host.builder ({
inherit (host) system;
modules = [
({ pkgs, lib, options, config, ... }: {
# 'mkMerge` to separate out each part into its own module
_type = "merge";
contents = [
(optionalAttrs (options ? networking.hostName) {
networking.hostName = hostname;
})

(optionalAttrs (options ? networking.domain) {
networking.domain = domain;
})

(if options ? nixpkgs.pkgs then
{
nixpkgs.config = selectedNixpkgs.config;
nixpkgs.pkgs =
# Make sure we don't import nixpkgs again if not
# necessary. We can't use `config.nixpkgs.config`
# because that triggers infinite recursion.
if (hostConfig.nixpkgs.config == { }) then
selectedNixpkgs
else
import patchedChannel
{
inherit (host) system;
overlays = selectedNixpkgs.overlays;
config = selectedNixpkgs.config // config.nixpkgs.config;
} // { inherit (selectedNixpkgs) name input; };
}
else { _module.args.pkgs = selectedNixpkgs; })

(optionalAttrs (options ? system.configurationRevision) {
system.configurationRevision = lib.mkIf (self ? rev) self.rev;
})

(optionalAttrs (options ? nix.package) {
nix.package = lib.mkDefault pkgs.nixUnstable;
})

(optionalAttrs (options ? nix.extraOptions) {
nix.extraOptions = "extra-experimental-features = nix-command ca-references flakes";
})
;

# genericModule MUST work gracefully with distinct module sets and
# cannot make any assumption other than the nixpkgs module system
# is used.
# See: https://github.com/NixOS/nixpkgs/blob/master/lib/modules.nix
# Exemplary module sets are: nixos, darwin, home-manager, etc
genericModule = preflight: { pkgs, lib, options, ... }: {
# 'mkMerge` to separate out each part into its own module
_type = "merge";
contents = (
if ((preflight == null) || (!options ? nixpkgs.pkgs)) then
# equivalent to nixpkgs.pkgs = selectedNixpkgs
[{ _module.args.pkgs = selectedNixpkgs; }]
else
# if preflight.nixpkgs.config == {},
# then the memorized evaluation of selectedNixpkgs will be used
# and we won't incur in an additional (expensive) evaluation.
# This works because nixos invokes at some point the same function
# with the same arguments as we already have in importChannel.
# DYOR:
# -> https://github.com/NixOS/nixpkgs/blob/b63a54f81ce96391e6da6aab5965926e7cdbce47/nixos/modules/misc/nixpkgs.nix#L58-L60
# -> https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/default.nix
# -> lib.systems.elaborate is idempotent
[
{
nixpkgs = { inherit (selectedNixpkgs) system overlays; };
}
{
# at this point we assume, that an evaluator at least
# uses nixpkgs.lib to evaluate modules.
_module.args = { inherit inputs; } // host.extraArgs;
# however, let the module system merge -> advanced config merge (sic!)
nixpkgs = { inherit (selectedNixpkgs) config; };
}
];
{
nixpkgs = { inherit (preflight.config.nixpkgs) config; };
}
]
)
++
[
{
_module.args = { inherit inputs; } // host.extraArgs;
}

(optionalAttrs (options ? networking.hostName) {
networking.hostName = hostname;
})

(optionalAttrs (options ? networking.domain) {
networking.domain = domain;
})

(optionalAttrs (options ? system.configurationRevision) {
system.configurationRevision = lib.mkIf (self ? rev) self.rev;
})

(optionalAttrs (options ? nix.package) {
nix.package = lib.mkDefault pkgs.nixUnstable;
})
] ++ host.modules;

(optionalAttrs (options ? nix.extraOptions) {
nix.extraOptions = "extra-experimental-features = nix-command ca-references flakes";
})
];
};

evalArgs = {
inherit (host) system;
modules = [ (genericModule null) ] ++ host.modules;
inherit specialArgs;
} // (optionalAttrs (host.output == "nixosConfigurations") {
inherit lib baseModules;
}
//
(optionalAttrs (host.output == "nixosConfigurations") {
specialArgs = nixosSpecialArgs // specialArgs;
}));
}
);

# The only way to find out if a host has `nixpkgs.config` set to
# the non-default value is by evalling the config.
# If it's not set, repeating the evaluation is cheap since
# all module evaluations except misc/nixpkgs.nix are memorized
# since `pkgs` would not change.
preFlightEvaled = host.builder (evalArgs // {
modules = [ (genericModule null) ] ++ host.modules;
});

in
{
${host.output}.${reverseDomainName} = host.builder (evalArgs // {
modules = [ (genericModule preFlightEvaled) ] ++ host.modules;
});
}
);

Expand All @@ -207,16 +234,21 @@ mergeAny otherArguments (
eachSystem supportedSystems
(system:
let
importChannel = name: value: (import (patchChannel system value.input (value.patches or [ ])) {
importChannel = name: value: (import value.input {
inherit system;
overlays = [
(final: prev: {
__dontExport = true; # in case user uses overlaysFromChannelsExporter, doesn't hurt for others
inherit srcs;
inherit srcs name;
inherit (value) input;
})
] ++ sharedOverlays ++ (if (value ? overlaysBuilder) then (value.overlaysBuilder pkgs) else [ ]) ++ [ flake-utils-plus.overlay ];
config = channelsConfig // (value.config or { });
}) // { inherit name; inherit (value) input; };
]
++
sharedOverlays ++ (if (value ? overlaysBuilder) then (value.overlaysBuilder pkgs) else [ ])
++
[ flake-utils-plus.overlay ];
config = mergeNixpkgsConfigs value.input channelsConfig (value.config or { });
});

pkgs = mapAttrs importChannel ensureChannelsWitsInputs;

Expand Down
80 changes: 0 additions & 80 deletions tests/channel-patching/flake.nix
View file

This file was deleted.

Loading

0 comments on commit ed14e98

Please sign in to comment.