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
Changes from 1 commit
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
Next Next commit
lib: include standalone wrapper in top-level lib
- Defragment lib-related flake-modules
- Expose the standalone-wrapper functions in the `lib`
MattSturgeon committed Sep 7, 2024
commit 7aa0fa372dc14d9f7fadb9737d398a33e8c72504
2 changes: 0 additions & 2 deletions flake-modules/default.nix
Original file line number Diff line number Diff line change
@@ -2,9 +2,7 @@
{
imports = [
./dev
./helpers.nix
./lib.nix
./legacy-packages.nix
./overlays.nix
./packages.nix
./templates.nix
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);
}
18 changes: 7 additions & 11 deletions flake-modules/tests.nix
Original file line number Diff line number Diff line change
@@ -2,39 +2,35 @@
{
perSystem =
{
self',
pkgs,
pkgsUnfree,
system,
helpers,
makeNixvimWithModule,
...
}:
let
inherit (self'.legacyPackages.lib) 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;
};
extra-args-tests = import ../tests/extra-args.nix { inherit pkgs 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;
inherit pkgs makeNixvimWithModule mkTestDerivationFromNixvimModule;
};

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

no-flake = import ../tests/no-flake.nix {
inherit system;
inherit (self.lib.${system}.check) mkTestDerivationFromNvim;
inherit system mkTestDerivationFromNvim;
nixvim = "${self}";
};

4 changes: 0 additions & 4 deletions flake-modules/wrappers.nix
Original file line number Diff line number Diff line change
@@ -3,10 +3,6 @@
perSystem =
{ system, pkgs, ... }:
{
_module.args = {
makeNixvimWithModule = import ../wrappers/standalone.nix pkgs self;
};

checks =
{
home-manager-module =
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

})