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

vimPlugins: make usage of luaPackages less confusing #195732

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

teto
Copy link
Member

@teto teto commented Oct 12, 2022

Fixes #185397

right now the src is ignored in:

  lush-nvim = buildNeovimPlugin {
    pname = "lush.nvim";
    version = "2022-08-09";
    src = fetchFromGitHub {
      owner = "rktjmp";
      repo = "lush.nvim";
      rev = "6b9f399245de7bea8dac2c3bf91096ffdedfcbb7";
      sha256 = "0rb77rwmbm438bmbjfk5hwrrcn5sihsa1413bdpc27rw3rrn8v8z";
    };
    meta.homepage = "https://github.com/rktjmp/lush.nvim/";
  };

which is very confusing. With this PR, we correctly override the src and the version of the package. We introduce a rockspecVersion attribute of lua package to be able to still find the rockspec when the "version" field needs to be different than "rockspecVersion".

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@teto teto marked this pull request as ready for review October 12, 2022 22:07
@teto teto requested a review from jonringer as a code owner October 12, 2022 22:07
@teto teto requested a review from figsoda October 12, 2022 22:08
@teto
Copy link
Member Author

teto commented Oct 12, 2022

Before the PR, on current master for instance, the src from vim/generated.nix is ignored, e.g.:

nix-build -A vimPlugins.nvim-cmp
this path will be fetched (0.04 MiB download, 0.25 MiB unpacked):
  /nix/store/xmv7cbrhich6lq9q8189zfr195ppwakn-lua5.1-nvim-cmp-0.0.1-2
copying path '/nix/store/xmv7cbrhich6lq9q8189zfr195ppwakn-lua5.1-nvim-cmp-0.0.1-2' from 'https://cache.nixos.org'...
/nix/store/xmv7cbrhich6lq9q8189zfr195ppwakn-lua5.1-nvim-cmp-0.0.1-2

with this PR, the source overrides the lua package src and also updates its version/name (I introduced a rockspecVersion field to keep track of the original rockspec name and still be able to find it in the src).

nix-build -A vimPlugins.nvim-cmp
/nix/store/dqr1w40i614j1zj0jjkisd2n15mzpkcv-lua5.1-nvim-cmp-2022-10-11

^ notice how the name changes. I was able to rebuild on master. Was not able to test on staging.

@@ -32247,7 +32247,7 @@ with pkgs;
};

neovimUtils = callPackage ../applications/editors/neovim/utils.nix {
inherit (lua51Packages) buildLuarocksPackage;
lua = lua5_1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lua = lua5_1;
lua = luajit;

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it should actually be neovim-unwrapped.lua but it just changes which version of luarocks you are going to use to install the packages so I would rather not change it for now to avoid breakage. What matters is the version of lua run by neovim, which is actually luajit so perfs are still the best

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-do-i-get-nvim-cmp-working-in-neovim/22398/2

right now the src is ignored in:

```
  lush-nvim = buildNeovimPlugin {
    pname = "lush.nvim";
    version = "2022-08-09";
    src = fetchFromGitHub {
      owner = "rktjmp";
      repo = "lush.nvim";
      rev = "6b9f399245de7bea8dac2c3bf91096ffdedfcbb7";
      sha256 = "0rb77rwmbm438bmbjfk5hwrrcn5sihsa1413bdpc27rw3rrn8v8z";
    };
    meta.homepage = "https://github.com/rktjmp/lush.nvim/";
  };
```

which is very confusing. With this PR, we correctly override the src and
the version of the package. We introduce a rockspecVersion attribute of
lua package to be able to still find the rockspec when the
"version" field needs to be different than "rockspecVersion".
@teto teto force-pushed the vimPlugins-lua-update branch from 91a3f1a to d2a35ab Compare October 13, 2022 16:54
@teto teto merged commit 2acce7d into NixOS:staging Oct 13, 2022
@teto teto deleted the vimPlugins-lua-update branch October 13, 2022 17:57
@573
Copy link
Contributor

573 commented Oct 25, 2022

Tested against staging (cmp-buffer failing) (here directly against this PR, cachix error) the underlying issue still seems to exist.

@teto
Copy link
Member Author

teto commented Oct 25, 2022

@573 could you come up with a test either in pkgs/applications/editors/vim/plugins/overrides.nix for nvim-cmp. I am using nvim-cmp on unstable (slighlty modified) without any issue (but not cmp-buffer).

@573
Copy link
Contributor

573 commented Oct 25, 2022

@573 could you come up with a test either in pkgs/applications/editors/vim/plugins/overrides.nix for nvim-cmp. I am using nvim-cmp on unstable (slighlty modified) without any issue (but not cmp-buffer).

@teto IDK how to do that properly, would love to contribute if I knew how.

I'm running (link above) a github action. It might be that I'm confusing the possible approaches how to set up neovim with configuration that exist...
Anyway in the gha linked above I'm trying to build the neovim provided by https://github.com/neovim/neovim/tree/master/contrib by applying https://github.com/nix-community/neovim-nightly-overlay as in: inherit (inputs.neovim-nightly-overlay.packages.${pkgs.stdenv.hostPlatform.system}) neovim;. I rewire the follows of the neovim* inputs to follow staging as well.

Also I'm taking the vimPlugins as of branch https://github.com/NixOS/nixpkgs/tree/staging if that is of relevance.

neovim itself I do install via home-manager as in

programs.neovim = {
      enable = true;
      package = pkgs.neovim;
      # and so on
};

I have a gut feeling that this is where it all goes wrong in the end, but I'm looking forward to learn about an approach that stays close to nixpkgs.

@573
Copy link
Contributor

573 commented Oct 26, 2022

To clarify, when I just override unstable neovim like in this code using it in my packages list as to be seen here I get at least v0.8 neovim with the cmp* plugins I wish to use as of currently.

How to now use the neovim-nightly provided by neovim-nightly-overlay is obviously (now for me) another topic, sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants