vim plugins built with vimUtils.buildVimPlugin require a pname attribute

[MangoIV]

Describe the bug

Vim plugins built with vimUtils.buildVimPlugin now require a pname attribute, because otherwise the evaluation of vim-utils.nix will fail with attribute 'pname' missing

Expected behavior

The name attribute should be enough

Notify maintainers

• @teto

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"x86_64-linux"`
 - host os: `Linux 5.19.11, NixOS, 22.11 (Raccoon), 22.11.20220929.c4d0026`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.11.0`
 - channels(mangoiv): `""`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

[MangoIV]

I think the issue originates from this line specifically:
aedd12c#r85553916

cc @teto (I think this was your PR)

[teto]

@MangoIV all derivations should have a pname and pname != name . I am interested in how this broke your setup ? can't you add a pname ?

[MangoIV]

I can add a pname but this is how you're supposed to specify vimPlugins; this also how it's done how it's done in the manual:

  nvim-agda = pkgs.vimUtils.buildVimPlugin rec {
    name = "nvim-agda";
    pname = name; # this is how I fixed the issue
    src = pkgs.fetchFromGitHub {
      owner = "ashinkarov";
      repo = "nvim-agda";
      rev = "4b347ab08157614e5acb2a9d5103115521ffa447";
      sha256 = "idMSFkWoWO3GpK/sJsqChYu34EpMoTmnvuqQI9R8PWk=";
    };
  };

I'm also not the only one who declares their plugins like this; e.g. cornelis-vim which I depend on (from outside of nixpkgs) also broke.

[MangoIV]

@teto also see this relevant nixpkgs manual section https://nixos.org/manual/nixpkgs/unstable/#managing-plugins-with-vim-packages

[MangoIV]

and no, I can't specify pname alone because that will require to also specify a version attribute which I don't wanna track, tbh.

Alternatively version could be generated automatically (from the rev?) but this is more elaborate than just changing it from taking pname to name

[teto]

This is a mistake in the manual, look at the generated plugins they all pname: https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/editors/vim/plugins/generated.nix

Using "name" instead of pname will break the install because the name has the prefix "vimplugin-".

Do you want to update the manual xD ?

[MangoIV]

so what should the workflow for version be then?

[MangoIV]

This is a mistake in the manual, look at the generated plugins they all pnam

I know this, but they're generated, you don't have to bother with the version

[MangoIV]

Yeah, this is definitely how it's supposed to work. the buildVimPlugin function should take a name attribute, it will prefix name with vimPlugin- and make that the name in the drv.

You either pass both pname and version or just name.

I think your change has to account for a vimPlugin not having a pname attribute. @teto

[MangoIV]

another simple fix would be to have buildVimPlugin take pname ? name

[MangoIV]

yeah, I think that would be the most simple fix

[teto]

Yeah, this is definitely how it's supposed to work. the buildVimPlugin function should take a name attribute, it will prefix name with vimPlugin- and make that the name in the drv.

It's the opposite throughout nixpkgs https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/editors/vim/plugins/build-vim-plugin.nix#L12 and if you look at the git log, you will notice a big effort to use pname everywhere.

My question is how do you end up with a plugin that doesn't have a pname ? and why not add pname there ?

[MangoIV]

buildVimPlugin takes a name or pname and version. everybody uses this function like that.

[teto]

if you change pname by lib.getName as it used to be ? I was afraid lib.getName would take the prefix but if there is a pname that's what it uses:
getName = attrs: attrs.name or ("${attrs.pname or "«name-missing»"}-${attrs.version or "«version-missing»"}");
which is kinda weird imho.
Thing is if you use a different name than the one expected, it can break the lua require for neovim (and some vim commands as well potentiall) so I can't think of any reason to set a name != pname. So I would just remove the "name" input from buildVimPlugin, what do you think ? (with some warning eventually).

[MangoIV]

So I would just remove the "name" input from buildVimPlugin, what do you think ? (with some warning eventually).

As long as that doesn't force the user to specify the version, that's fine imho.
Maybe you want attrs.pname or trace "name is deprecated, use pname" (attrs.name)

[MangoIV]

@teto because I don't quite understand what you're proposing to solve it, this is the definition of buildVimPlugin from pkgs/applications/editors/vim/plugins/build-vim-plugin.nix

rec {
  buildVimPlugin = attrs@{
    name ? "${attrs.pname}-${attrs.version}",
    namePrefix ? "vimplugin-",
    src,
    unpackPhase ? "",
    configurePhase ? "",
    buildPhase ? "",
    preInstall ? "",
    postInstall ? "",
    path ? ".",
    addonInfo ? null,
    ...
  }:
    let drv = stdenv.mkDerivation (attrs // {
      name = namePrefix + name;

      inherit unpackPhase configurePhase buildPhase addonInfo preInstall postInstall;

      installPhase = ''
        runHook preInstall

        target=$out/${rtpPath}/${path}
        mkdir -p $out/${rtpPath}
        cp -r . $target

        runHook postInstall
      '';
    });
    in toVimPlugin(drv.overrideAttrs(oa: {
      rtp = "${drv}";
    }));

If we don't allow for passing name, it will become vimplugin-${attrs.pname}-${attrs.version} which would require a version attribute to be passed. Do we want to default this to "«version-missing»"?

[queezle42]

Given that the manual explicitly documents name (https://nixos.org/manual/nixpkgs/unstable/#what-if-your-favourite-vim-plugin-isnt-already-packaged) I don't think it's a good idea to drop name unless absolutely necessary, or this will lead to unnecessary breakages on many private packages.

The fact that nixpkgs uses pname and version isn't a good argument for dropping name. Nixpkgs uses a higher standard since it's a public package set, but buildVimPlugin is a documented api that is also used outside of nixpkgs.

Using pname without a version seems like a very bad idea; ideally buildVimPlugin should behave like mkDerivation regarding package names.

[MangoIV]

I 100% agree with this statement. I do however understand the idea to migrate to pname eventually.

[teto]

Given that the manual explicitly documents name (nixos.org/manual/nixpkgs/unstable/#what-if-your-favourite-vim-plugin-isnt-already-packaged) I don't think it's a good idea to drop name unless absolutely necessary, or this will lead to unnecessary breakages on many private packages.

The easy way out is to replace pname with lib.getName as was done before and update the manual to use pname
I wonder how many private plugins are are out there (my guess is few), also I thought I would mention this method to maintain out-of-tree plugins https://nixos.org/manual/nixpkgs/unstable/#id-1.5.8.36.11 .

[J-Swift]

This broke my setup as well. Tried to upgrade and the build failed, I'm not sure which plugin/plugins are the culprit yet.

[J-Swift]

I deleted a bunch of old plugins, so that got rid of some, then my "actual" plugins that got broke were:

• rainbow_parentheses-vim
• rust-vim

EDIT: Well I feel dumb. This issue was because of my local plugin definitions which were using name but not pname (as mentioned by @MangoIV). Fixed my local expressions, but I definitely think this should be fixed upstream to warn instead of fail.

[MangoIV]

Two PRs related to this issue:

#194069

#194065

[aiotter]

I wonder how many private plugins are are out there

Just to let you know, it broke my 18 private neovim plugins.

[RaitoBezarius]

As I encountered the same, I think it's a good idea to add it to the release notes so that everyone is not that much surprised when they will have errors.
Maybe, we can add an assert with a nice message also? @teto
I can PR those.

[MangoIV]

so what do we do about it? strict pname and version? Or no api change? what do we do in the latter case? pname = name?

[teto]

Seems like this upsetted more people than I anticipated so I'll just merge this quickfix #194483 so that we can find a solution in a less stressful context.

Just to let you know, it broke my 18 private neovim plugins.

I was more thinking in terms of user then ^^ so I am curious how do you define these plugins ? do you use the manual recommendation with an out of tree vimPlugins overlay (I guess not since that uses pname) or flake inputs ? is this public somewhere ?

[aiotter]

I have a repo to provide neovim with my favourite plugins. The flake for plugins itself uses nixpkgs fork so it wasn't broken, but it was broken when referenced from other nixpkgs instance.
https://github.com/aiotter/neovim/blob/master/sources/flake.nix

[DanielSiepmann]

I was also one of the people affected by this change. I didn't commented earlier as the discussion already seemed productive and the fix seemed easy.

I now comment as you want to fix in a less stressful context and it looks like you wanna understand the situation of the people.
Here is my comment with the fixes, where you can see my old setup as well: https://gitea.daniel-siepmann.de/danielsiepmann/nixpkgs/commit/1ea75ea1e4ca13b138d14fe9ef329c8e9b274b9b
I'm using nix unstable with home-manager on an Ubuntu host.

I no longer know how I ended up in that setup, but I guess I followed the docs as mentioned in an earlier comment.

Some Plugins are missing in nixpkgs (maybe I should contribute?). Others are pinned to an older version as I don't like newer changes.

I hope this context helps a bit.