Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flake: defragment lib+wrappers, also cleanup tests #2104

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
2 changes: 0 additions & 2 deletions flake-modules/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
{
imports = [
./dev
./helpers.nix
./lib.nix
./legacy-packages.nix
./overlays.nix
./packages.nix
./templates.nix
Expand Down
7 changes: 0 additions & 7 deletions flake-modules/helpers.nix

This file was deleted.

15 changes: 0 additions & 15 deletions flake-modules/legacy-packages.nix

This file was deleted.

29 changes: 25 additions & 4 deletions flake-modules/lib.nix
Original file line number Diff line number Diff line change
@@ -1,11 +1,32 @@
{
self,
config,
lib,
withSystem,
getSystem,
...
}:
{
flake.lib = lib.genAttrs config.systems (
lib.flip withSystem ({ pkgs, ... }: import ../lib { inherit pkgs lib; })
);
perSystem =
{ self', pkgs, ... }:
{
# `helpers` is used throughout the flake modules
_module.args = {
inherit (self'.lib) helpers;
};

legacyPackages = {
# Export nixvim's lib in legacyPackages
lib = import ../lib {
inherit pkgs lib;
flake = self;
};

# Historically, we exposed these at the top-level of legacyPackages
# TODO: Consider renaming the new location, and keeping the old name here?
inherit (self'.legacyPackages.lib) makeNixvimWithModule makeNixvim;
};
};

# Also expose `legacyPackages.<system>.lib` as `lib.<system>`
flake.lib = lib.genAttrs config.systems (system: (getSystem system).legacyPackages.lib);
}
59 changes: 23 additions & 36 deletions flake-modules/tests.nix
Original file line number Diff line number Diff line change
@@ -1,53 +1,40 @@
{ self, ... }:
{ self, lib, ... }:
{
perSystem =
{
self',
pkgs,
pkgsUnfree,
system,
helpers,
makeNixvimWithModule,
...
}:
let
evaluatedNixvim = helpers.modules.evalNixvim { check = false; };
inherit (self'.legacyPackages.lib) helpers makeNixvimWithModule;
callTest = lib.callPackageWith (
pkgs
// {
nixvimLib = self'.legacyPackages.lib;
inherit helpers makeNixvimWithModule;
inherit (self'.legacyPackages.lib.check) mkTestDerivationFromNvim mkTestDerivationFromNixvimModule;
evaluatedNixvim = helpers.modules.evalNixvim { check = false; };
}
);
in
{
checks = {
extra-args-tests = import ../tests/extra-args.nix {
inherit pkgs;
inherit makeNixvimWithModule;
};

extend = import ../tests/extend.nix { inherit pkgs makeNixvimWithModule; };

extra-files = import ../tests/extra-files.nix { inherit pkgs makeNixvimWithModule; };

enable-except-in-tests = import ../tests/enable-except-in-tests.nix {
inherit pkgs makeNixvimWithModule;
inherit (self.lib.${system}.check) mkTestDerivationFromNixvimModule;
};

failing-tests = pkgs.callPackage ../tests/failing-tests.nix {
inherit (self.lib.${system}.check) mkTestDerivationFromNixvimModule;
};

no-flake = import ../tests/no-flake.nix {
extra-args-tests = callTest ../tests/extra-args.nix { };
extend = callTest ../tests/extend.nix { };
extra-files = callTest ../tests/extra-files.nix { };
enable-except-in-tests = callTest ../tests/enable-except-in-tests.nix { };
failing-tests = callTest ../tests/failing-tests.nix { };
no-flake = callTest ../tests/no-flake.nix {
inherit system;
inherit (self.lib.${system}.check) mkTestDerivationFromNvim;
nixvim = "${self}";
};

lib-tests = import ../tests/lib-tests.nix {
inherit pkgs helpers;
inherit (pkgs) lib;
};

maintainers = import ../tests/maintainers.nix { inherit pkgs; };

generated = pkgs.callPackage ../tests/generated.nix { };

package-options = pkgs.callPackage ../tests/package-options.nix { inherit evaluatedNixvim; };
} // import ../tests { inherit pkgs pkgsUnfree helpers; };
lib-tests = callTest ../tests/lib-tests.nix { };
maintainers = callTest ../tests/maintainers.nix { };
generated = callTest ../tests/generated.nix { };
package-options = callTest ../tests/package-options.nix { };
} // callTest ../tests { inherit pkgsUnfree; };
};
}
4 changes: 0 additions & 4 deletions flake-modules/wrappers.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
perSystem =
{ system, pkgs, ... }:
{
_module.args = {
makeNixvimWithModule = import ../wrappers/standalone.nix pkgs self;
};

checks =
{
home-manager-module =
Expand Down
10 changes: 7 additions & 3 deletions lib/default.nix
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
# Args probably only needs pkgs and lib
{
flake,
pkgs,
lib ? pkgs.lib,
_nixvimTests ? false,
...
}@args:
{
lib.fix (self: {
# Add all exported modules here
check = import ./tests.nix { inherit lib pkgs; };
helpers = import ./helpers.nix (args // { inherit _nixvimTests; });
}

# TODO: Consider renaming these?
makeNixvimWithModule = import ../wrappers/standalone.nix pkgs flake;
makeNixvim = module: self.makeNixvimWithModule { inherit module; };
Copy link
Contributor

Choose a reason for hiding this comment

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

what were you thinking for these names?

Copy link
Member Author

Choose a reason for hiding this comment

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

mkNixvim and mkNixvimWith

  • it seems we usually prefer mk over make, although it's not consistent
  • the With suffix is conventionally used in nixpkgs for a more powerful function variant that takes structured attr args
  • WithModule is misleading, because both functions take a module

Copy link
Contributor

Choose a reason for hiding this comment

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

* `WithModule` is misleading, because both functions take a module

This is what was throwing me when looking at their definitions, as well...

Copy link
Member Author

@MattSturgeon MattSturgeon Sep 7, 2024

Choose a reason for hiding this comment

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

While I'd like to rename these, I'm hesitant to do so yet as there's other changes I'd like to make and I think we should minimize user-facing refactoring where possible.

In particular, I'd like to get to a point where our lib (including the standalone wrapper) doesn't depend on pkgs or system. This matches the design of other similar flakes like nixos and home-manager, whose equivalent function (e.g. nixpkgs.lib.nixosSystem) is made available without being in the "perSystem" part of the flake outputs.

Achieving this goal may be fairly involved though and require a few breaking changes and ugly compromises, including re-imagining the standalone wrapper and it's function signature. That'd be the optimal time to rename it, I think.

EDIT: #2186

})
17 changes: 7 additions & 10 deletions tests/default.nix
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
lib ? pkgs.lib,
helpers,
lib,
linkFarm,
pkgs,
pkgsUnfree,
mkTestDerivationFromNixvimModule,
}:
let
fetchTests = import ./fetch-tests.nix;
test-derivation = import ../lib/tests.nix { inherit pkgs lib; };
inherit (test-derivation) mkTestDerivationFromNixvimModule;
fetchTests = import ./fetch-tests.nix { inherit lib pkgs helpers; };

moduleToTest =
name: module:
Expand All @@ -17,10 +17,7 @@ let
};

# List of files containing configurations
testFiles = fetchTests {
inherit lib pkgs helpers;
root = ./test-sources;
};
testFiles = fetchTests ./test-sources;

exampleFiles = {
name = "examples";
Expand All @@ -44,13 +41,13 @@ in
lib.pipe (testFiles ++ [ exampleFiles ]) [
(builtins.map (file: {
inherit (file) name;
path = pkgs.linkFarm file.name (builtins.mapAttrs moduleToTest file.cases);
path = linkFarm file.name (builtins.mapAttrs moduleToTest file.cases);
}))
(helpers.groupListBySize 10)
(lib.imap1 (
i: group: rec {
name = "test-${toString i}";
value = pkgs.linkFarm name group;
value = linkFarm name group;
}
))
builtins.listToAttrs
Expand Down
6 changes: 4 additions & 2 deletions tests/enable-except-in-tests.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{
pkgs,
linkFarm,
runCommandNoCCLocal,
mkTestDerivationFromNixvimModule,
makeNixvimWithModule,
}:
Expand All @@ -19,7 +21,7 @@ let
let
nvim = makeNixvimWithModule { inherit pkgs module; };
in
pkgs.runCommand "enable-except-in-tests-not-in-test"
runCommandNoCCLocal "enable-except-in-tests-not-in-test"
{ printConfig = "${nvim}/bin/nixvim-print-init"; }
''
if ! "$printConfig" | grep 'require("image").setup'; then
Expand All @@ -31,7 +33,7 @@ let
touch $out
'';
in
pkgs.linkFarm "enable-except-in-tests" [
linkFarm "enable-except-in-tests" [
{
name = "in-test";
path = inTest;
Expand Down
7 changes: 5 additions & 2 deletions tests/extend.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{ makeNixvimWithModule, pkgs }:
{
makeNixvimWithModule,
runCommandNoCCLocal,
}:
let
firstStage = makeNixvimWithModule {
module = {
Expand All @@ -10,7 +13,7 @@ let

generated = secondStage.extend { extraConfigLua = "-- third stage"; };
in
pkgs.runCommand "extend-test" { printConfig = "${generated}/bin/nixvim-print-init"; } ''
runCommandNoCCLocal "extend-test" { printConfig = "${generated}/bin/nixvim-print-init"; } ''
config=$($printConfig)
for stage in "first" "second" "third"; do
if ! "$printConfig" | grep -q -- "-- $stage stage"; then
Expand Down
7 changes: 5 additions & 2 deletions tests/extra-args.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{ makeNixvimWithModule, pkgs }:
{
makeNixvimWithModule,
runCommandNoCCLocal,
}:
let
defaultModule =
{ regularArg, ... }:
Expand Down Expand Up @@ -28,7 +31,7 @@ let
};
};
in
pkgs.runCommand "special-arg-test" { printConfig = "${generated}/bin/nixvim-print-init"; } ''
runCommandNoCCLocal "special-arg-test" { printConfig = "${generated}/bin/nixvim-print-init"; } ''
config=$($printConfig)
if ! "$printConfig" | grep -- '-- regularArg=regularValue'; then
echo "Missing regularArg in config"
Expand Down
7 changes: 5 additions & 2 deletions tests/extra-files.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{ makeNixvimWithModule, pkgs }:
{
makeNixvimWithModule,
runCommandNoCCLocal,
}:
let
extraFiles = {
"one".text = "one";
Expand All @@ -12,7 +15,7 @@ let
};
};
in
pkgs.runCommand "extra-files-test"
runCommandNoCCLocal "extra-files-test"
{
root = build.config.filesPlugin;
files = builtins.attrNames extraFiles;
Expand Down
3 changes: 1 addition & 2 deletions tests/fetch-tests.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
root,
lib,
pkgs,
helpers,
Expand Down Expand Up @@ -49,4 +48,4 @@ let
builtins.concatLists
];
in
fetchTests root [ ]
root: fetchTests root [ ]
4 changes: 2 additions & 2 deletions tests/generated.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
lib,
runCommand,
pkgs,
runCommandNoCCLocal,
}:
let
# Format a list of errors with an error message and trailing newline
Expand Down Expand Up @@ -68,7 +68,7 @@ let
}
);
in
runCommand "generated-sources-test" { inherit errors; } ''
runCommandNoCCLocal "generated-sources-test" { inherit errors; } ''
if [ -n "$errors" ]; then
echo -n "$errors"
exit 1
Expand Down
15 changes: 8 additions & 7 deletions tests/lib-tests.nix
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# For shorter test iterations run the following in the root of the repo:
# `echo ':b checks.${builtins.currentSystem}.lib-tests' | nix repl .`
{
lib,
pkgs,
helpers,
lib,
runCommandNoCCLocal,
writeText,
}:
let
luaNames = {
Expand Down Expand Up @@ -45,9 +46,9 @@ let
];
};

drv = pkgs.writeText "example-derivation" "hello, world!";
drv = writeText "example-derivation" "hello, world!";

results = pkgs.lib.runTests {
results = lib.runTests {
testToLuaObject = {
expr = helpers.toLuaObject {
foo = "bar";
Expand Down Expand Up @@ -377,11 +378,11 @@ let
};
in
if results == [ ] then
pkgs.runCommand "lib-tests-success" { } "touch $out"
runCommandNoCCLocal "lib-tests-success" { } "touch $out"
else
pkgs.runCommand "lib-tests-failure"
runCommandNoCCLocal "lib-tests-failure"
{
results = pkgs.lib.concatStringsSep "\n" (
results = lib.concatStringsSep "\n" (
builtins.map (result: ''
${result.name}:
expected: ${lib.generators.toPretty { } result.expected}
Expand Down
6 changes: 3 additions & 3 deletions tests/maintainers.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
pkgs ? import <nixpkgs> { },
lib ? pkgs.lib,
lib,
runCommandNoCCLocal,
}:
let
inherit (lib) attrNames filter length;
Expand All @@ -9,7 +9,7 @@ let
duplicates = filter (name: nixpkgsList ? ${name}) (attrNames nixvimList);
count = length duplicates;
in
pkgs.runCommand "maintainers-test" { inherit count duplicates; } ''
runCommandNoCCLocal "maintainers-test" { inherit count duplicates; } ''
if [ $count -gt 0 ]; then
echo "$count nixvim maintainers are also nixpkgs maintainers:"
for name in $duplicates; do
Expand Down