vimUtils: plugins shouldn't be overrided by dependencies #355680
Conversation
nix-owners
bot
requested review from
figsoda,
GaetanLepage,
khaneliman,
manveru,
mrcjkb,
PerchunPak,
rvolosatovs and
teto
|
could you share an exemple where it caused problems ? |
|
I add an overrided nvim-treesitter with all grammars. Today I added orgmode and it depends on nvim-treesitter with org grammar only. Then all other grammars disappeared because my nvim-treesitter is overrided. |
Could you send an MRE flake please? |
|
I don't use flake but you can use this package {
symlinkJoin,
vimPlugins,
vimUtils,
}:
let
nvim-treesitter-with-grammars = symlinkJoin {
name = "nvim-treesitter";
paths = [
vimPlugins.nvim-treesitter
] ++ vimPlugins.nvim-treesitter.withAllGrammars.dependencies;
};
lazyPkgs.all.start = with vimPlugins; [
nvim-treesitter-with-grammars
orgmode
];
in
vimUtils.packDir lazyPkgs |
|
Oh it is probably because orgmode specifies dependencies wrongly It should be orgmode = super.orgmode.overrideAttrs {
dependencies = with self; [
nvim-treesitter
nvim-treesitter-parsers.org
];
nvimRequireCheck = "orgmode";
};But this PR is much better and maintainable fix! Will test it when will have opportunity (==please wait for my review before merging) |
|
Hmm, I cannot reproduce the issue on nixos-unstable (24f0d4a) but with this PR cherry-picked, no grammars are installed Wrong MRE flake# flake.nix
{
inputs.nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable";
outputs =
{ nixpkgs, ... }:
let
forAllSys = nixpkgs.lib.genAttrs nixpkgs.lib.platforms.all;
overlayMyNeovim = prev: final: {
myNeovim =
let
luaRC =
final.writeText "init.lua" # lua
''
vim.schedule(function()
vim.g.mapleader = ' '
vim.g.maplocalleader = ' '
vim.cmd("packadd nvim-treesitter");
require("nvim-treesitter.configs").setup({
auto_install = false,
highlight = {
enable = true
},
})
vim.cmd("packadd orgmode");
require('orgmode').setup({
org_agenda_files = '~/orgfiles/**/*',
org_default_notes_file = '~/orgfiles/refile.org',
})
end)
'';
in
final.wrapNeovim final.neovim-unwrapped {
configure = {
customRC = ''lua dofile("${luaRC}")'';
packages.all.start =
with final.vimPlugins;
[
];
packages.all.opt = with final.vimPlugins; [
nvim-treesitter.withAllGrammars
orgmode
];
};
extraMakeWrapperArgs = ''--prefix PATH : "${
final.lib.makeBinPath (with final; [ stdenv.cc.cc ])
}"'';
};
};
in
{
packages = forAllSys (
system:
let
pkgs = import nixpkgs {
inherit system;
overlays = [ overlayMyNeovim ];
};
in
{
default = pkgs.myNeovim;
}
);
};
} |
|
My testcase is that all parsers are symlinked into nvim-treesitter. |
| [ plugin ] ++ ( | ||
| lib.unique (builtins.concatLists (map transitiveClosure plugin.dependencies or [])) | ||
| ); | ||
| reverseUniq = list: lib.reverseList (lib.unique (lib.reverseList list)); |
There was a problem hiding this comment.
Why do we need to reverse list?
There was a problem hiding this comment.
lib.unique keeps the first same element but I want to keep the last one.
|
Oh, I missed that you do MRE flake.nix{
inputs.nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable";
outputs =
{ nixpkgs, ... }:
let
forAllSys = nixpkgs.lib.genAttrs nixpkgs.lib.platforms.all;
overlayMyNeovim = prev: final: {
myNeovim =
let
nvim-treesitter-with-grammars = final.symlinkJoin {
name = "nvim-treesitter";
paths =
with final.vimPlugins;
[
nvim-treesitter
]
++ nvim-treesitter.withAllGrammars.dependencies;
};
luaRC =
final.writeText "init.lua" # lua
''
vim.schedule(function()
vim.g.mapleader = ' '
vim.g.maplocalleader = ' '
require("nvim-treesitter.configs").setup({
auto_install = false,
highlight = {
enable = true
},
})
require('orgmode').setup({
org_agenda_files = '~/orgfiles/**/*',
org_default_notes_file = '~/orgfiles/refile.org',
})
end)
'';
in
final.wrapNeovim final.neovim-unwrapped {
configure = {
customRC = ''lua dofile("${luaRC}")'';
packages.all.start = with final.vimPlugins; [
nvim-treesitter-with-grammars
orgmode
];
packages.all.opt =
with final.vimPlugins;
[
];
};
extraMakeWrapperArgs = ''--prefix PATH : "${
final.lib.makeBinPath (with final; [ stdenv.cc.cc ])
}"'';
};
};
in
{
packages = forAllSys (
system:
let
pkgs = import nixpkgs {
inherit system;
overlays = [ overlayMyNeovim ];
};
in
{
default = pkgs.myNeovim;
}
);
};
}Also, I think this function can be simplified to findDependenciesRecursively = plugins: if plugins == [] then [] else let
# reverse so lib.unique prefers the last duplicate plugin instead of the first one
getPluginDependencies = plugin: lib.reverseList (plugin.dependencies or []);
in
lib.unique ((
findDependenciesRecursively (lib.concatLists (map getPluginDependencies plugins))
) ++ plugins);No need to reiterate the list for reversing so many times. Also, this returns a reversed list of plugins, but it shouldn't matter? |
|
It works for me. :) |
|
Is this file formatted ? |
|
Formatted this function isn't much better findDependenciesRecursively =
plugins:
if plugins == [ ] then
[ ]
else
let
# reverse so lib.unique prefers the last duplicate plugin instead of the first one
getPluginDependencies = plugin: lib.reverseList (plugin.dependencies or [ ]);
in
lib.unique (
(findDependenciesRecursively (lib.concatLists (map getPluginDependencies plugins))) ++ plugins
); |
I find this more readable. But maybe it is just me being used to the formatter's style. |
|
I formatted the whole file. |
|
I haven't reviewd the code but your premise looks odd:
orgmode doesn't depend on nvim-treesitter |
| [ ] | ||
| else | ||
| let | ||
| # reverse so lib.unique prefers the last duplicate plugin instead of the first one |
There was a problem hiding this comment.
why is it better to select the last rather than the first ?
There was a problem hiding this comment.
It seems linkFarm only keeps the last entry with the same path.
There was a problem hiding this comment.
but your fix might break other people setup where it's more important to get the latter. So isn't the proper fix to warn in case 2 files conflict ?
There was a problem hiding this comment.
I fixed the problem that the directly specified plugin is not installed but the dependency is kept.
There was a problem hiding this comment.
Because if we would prefer first element first, we overwrite our plugins with dependencies of other plugins. I tried to come up with an example, but evaluation of this function is too weird for me to understand.
I cannot come up for why you would want your custom plugins to be ignored, while we have a good use case for why you don't
|
I'm not sure how to fix that properly. We can't let orgmode install the treesitter parser by itself on NixOS. By the way, orgmode uses a forked version of the parser. |
We should remove the dependency if upstream has changed. |
Try using |
I do the symlinkJoin trick to make it work properly with lazy.nvim. |
|
I'm not familar with how lazy.nvim works, does it work if you put |
You don't have to use lazy.nvim as it doesn't play well with nix. There are better tools for lazy loading (if you want to use Nix as dependency manager for your plugins), e.g. lz.n or lze. If you really really really want to use lazy.nvim, you can use nixCats which works perfectly with lazy Symlinking all grammars only improves startup performance a bit, but it creates a lot of other problems |
lazy.nvim lazy loads the plugin from specified path. So it only loads the nvim-treesitter plugin but not those grammars.
I thought this should work. I'm not sure how
I use lazy.nvim because I use LazyVim... Thanks for your suggestion! |
Lazy.nvim shouldn't load grammars, |
nvim-treesitter can't find them if they are not loaded or in the plugin path. |
|
Another workaround might be to symlink only the grammars into a plugin, and loading both nvim-treesitter and the grammars plugin |
wegank
added
the
2.status: merge conflict
If the plugin has the same directory name with a dependency of another plugin, it shouldn't be overrided by the dependency.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.