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.

For those users that have been accustomed to the patching facility
and do not suffer from the regression-causing old implementation
of nixpkgs config handling, I set up
https://github.com/gytis-ivaskevicius/flake-utils-plus/tree/staging-with-nixpkgs-patching
which reverts this commit and can be continously rebased until
it will be no longer necesary as upstream inputs patching will
be implemented.
  • Loading branch information
@blaggacao
blaggacao committed Sep 5, 2021
1 parent 6f261d7 commit a1e8206
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 @@ -21,7 +21,6 @@ jobs:
# Execute /tests/*
- run: nix develop --command check-derivation-outputs
- run: nix develop --command check-derivation-outputs-old
- 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 @@ -16,7 +16,6 @@ Main flake-utils-plus features (Attributes visible from `flake.nix`):
- `lib.mkFlake { ... }` - Clean and pleasant to use flakes abstraction.
- Option `nix.generateRegistryFromInputs` - Generates `nix.registry` from flake `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 @@ -83,9 +82,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,12 +68,11 @@ devshell.mkShell {
command = "fd --extension nix --exec nix-instantiate --parse --quiet {} >/dev/null";
}

(test "channel-patching")
(test "derivation-outputs")
(test "derivation-outputs-old")
(test "hosts-config")
(test "overlays-flow")
(test "all" // { command = "check-channel-patching && check-derivation-outputs && check-derivation-outputs-old && check-hosts-config && check-overlays-flow"; })
(test "all" // { command = "check-derivation-outputs && check-derivation-outputs-old && 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 @@ -52,15 +52,6 @@
else value
)
rhs;

patchChannel = system: channel: patches:
if patches == [ ] then channel else
(import channel { inherit system; }).pkgs.applyPatches {
name = "nixpkgs-patched-${channel.shortRev}";
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 @@ -34,15 +34,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 @@ -52,10 +50,13 @@ let
foldl'
genList
head
isAttrs
isFunction
isString
length
listToAttrs
mapAttrs
pathExists
removeAttrs
split
tail
Expand Down Expand Up @@ -118,6 +119,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 @@ -130,99 +145,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"); };
;

# 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; };
}
{
# 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;
})

# 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";
})
(optionalAttrs (options ? nix.package) {
nix.package = lib.mkDefault pkgs.nixUnstable;
})

{
# at this point we assume, that an evaluator at least
# uses nixpkgs.lib to evaluate modules.
_module.args = { inherit inputs; } // host.extraArgs;
}
];
(optionalAttrs (options ? nix.extraOptions) {
nix.extraOptions = "extra-experimental-features = nix-command ca-references flakes";
})
] ++ host.modules;
];
};

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 @@ -232,16 +259,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
Loading

0 comments on commit a1e8206

Please sign in to comment.