vimPlugins: strictly enforce only overrides in overrides.nix

[MattSturgeon]

Follow up to #438708 (comment), previously @khaneliman explicitly shadowed buildVimPlugin and buildNeoVimPlugin to ensure they are not used to build new plugins.

This PR takes things a step further, by asserting that no attributes are defined in the overrides.nix overlay that aren't already present in the overlay's super (aka prev).

When initially added, the new assertion picked up the following package definitions:

nix-repl> pkgs.vimPlugins

      (stack trace omitted for brevity, see PR edit history)

       error: vimPlugins: the following attributes should not be defined in overrides.nix:
       - corePlugins
       - fzf-lua
       - haskell-tools-nvim
       - notmuch-vim
       - nvim-treesitter-parsers
       - parinfer-rust
       - rocks-nvim
       - rustaceanvim
       - ssr

...which I've resolved as part of this PR.

As the eval CI ignores vimPlugins, I manually verified that all attributes touched in this PR still evaluate to the exact same derivation as on master. Including every vimPlugins.nvim-treesitter-parsers derivation.

I did this by running nix-instantiate -A vimPlugins on master and this branch, then comparing the out-paths:

$ nix-instantiate --expr '(import ./. { config = { allowAliases = false; allowBroken = true; allowUnfree = true; }; }).vimPlugins' > ../vimPlugins-pr.txt

$ git reset --hard upstream/HEAD

$ nix-instantiate --expr '(import ./. { config = { allowAliases = false; allowBroken = true; allowUnfree = true; }; }).vimPlugins' > ../vimPlugins-master.txt

$ diff ../vimPlugins-master.txt ../vimPlugins-pr.txt

I've attached the files for transparency:

• vimPlugins-master.txt
• vimPlugins-pr.txt

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[wolfgangwalther]

As the eval CI ignores vimPlugins,

Why is that btw? Are they not built on hydra and why?

---

I'm not sure whether the semantics are better than before. Could there be a scenario where you'd like to have the same package available as two attributes with some overrides applied? Maybe a different version or something else.

In this case, it's not unthinkable to add a new attribute via override. Now, I don't know whether that hypothetical scenario is something you'd like to deal with differently and still keep out of overrides.nix.

[GaetanLepage]

Why is that btw? Are they not built on hydra and why?

This happened in #377508.
We decided to exclude regular vim plugins from hydra for two main reasons:

1. Having them built by Hydra made any change in neovim (or any related package on which all plugins depend) spawn numerous rebuilds.
2. They are extremely light to "build" (~simply fetching a github repo)

[GaetanLepage]

Why not!

[wolfgangwalther]

We should find a way to at least eval them, but possibly even pass them to nixpkgs-review to build. They should not count as rebuilds though. Does that make sense?

[PerchunPak]

We should find a way to at least eval them, but possibly even pass them to nixpkgs-review to build. They should not count as rebuilds though. Does that make sense?

If only that was possible...

[MattSturgeon]

Could there be a scenario where you'd like to have the same package available as two attributes with some overrides applied? Maybe a different version or something else.

In this case, it's not unthinkable to add a new attribute via override. Now, I don't know whether that hypothetical scenario is something you'd like to deal with differently and still keep out of overrides.nix.

This is certainly something worth considering.

My understanding is that the intention of overrides.nix is purely to provide a way to tweak plugins from generated.nix and luaPackagesPlugins.nix, so I don't think it really needs to handle variants.

The closest I can see to a variant is ssr: https://github.com/NixOS/nixpkgs/pull/438906/files#diff-5a523ef120c1096205b2620474ded6471974da5b75beb6083781cfe767bd72c4

(Although I'm not sure if it is intended that there is both a ssr and ssr-nvim attribute, one with overrides and one without?)

As I've done there, these cases can be handled by adding them to the non-generated tree and accessing the base-variant, e.g:

# plugins/non-generated/foo/default.nix
{ vimPlugins }:
vimPlugins.bar.overrideAttrs {
  # ...
}

I'll let the vim team decide whether they'd prefer the strict enforcement proposed in this PR or another approach that allows introducing new attributes in overrides.nix to allow for multiple variants of the same plugin.

---

As the eval CI ignores vimPlugins,

Why is that btw? Are they not built on hydra and why?

This happened in #377508. We decided to exclude regular vim plugins from hydra for two main reasons:

I agree with the reasoning, but it does make it difficult to verify a PR doesn't break anything. I struggle to get nixpkgs-review to build the vimPluins set, for example, although I'm sure the vim team know some tricks.

We should find a way to at least eval them, but possibly even pass them to nixpkgs-review to build. They should not count as rebuilds though. Does that make sense?

That'd be great.

I do feel like something should be automatically evaluating and building modified plugins to avoid unnoticed regressions in PRs.

I'm guessing this doesn't apply more broadly to all packages with meta.hydraPlatforms = [] and there are good reasons not to eval/build them normally?

[GaetanLepage]

Thanks @MattSturgeon