vimPlugins: inherit lua & node packages before overrides#390702
vimPlugins: inherit lua & node packages before overrides#390702khaneliman merged 1 commit intoNixOS:masterfrom
Conversation
- Move lua & node package inheritance to a dedicated file - Apply lua & node package inheritance before overrides
MattSturgeon
requested review from
GaetanLepage,
PerchunPak,
khaneliman and
teto
and removed request for
PerchunPak and
khaneliman
NixOS/nixpkgs#390702 Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/5d9b5431f967007b3952c057fc92af49a4c5f3b2?narHash=sha256-nkH2Edu9rClcsQp2PYBe8E6fp8LDPi2uDBQ6wyMdeXI%3D' (2025-03-16) → 'github:NixOS/nixpkgs/0ca1fc33509563e87989fad2cbc3d39f988570de?narHash=sha256-sgwENLuCiEPZvHOKPPjmmNdeO1xZurC29aWiP1n5X1w%3D' (2025-03-17)
NixOS/nixpkgs#390702 Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/5d9b5431f967007b3952c057fc92af49a4c5f3b2?narHash=sha256-nkH2Edu9rClcsQp2PYBe8E6fp8LDPi2uDBQ6wyMdeXI%3D' (2025-03-16) → 'github:NixOS/nixpkgs/0ca1fc33509563e87989fad2cbc3d39f988570de?narHash=sha256-sgwENLuCiEPZvHOKPPjmmNdeO1xZurC29aWiP1n5X1w%3D' (2025-03-17)
khaneliman
left a comment
khaneliman
left a comment
There was a problem hiding this comment.
LGTM, thanks for finding that
|
| }; | ||
|
|
||
| # NOTE: this overrides a luaPackages-based plugin | ||
| gitsigns-nvim = super.gitsigns-nvim.overrideAttrs { |
There was a problem hiding this comment.
the main driver for using buildNeovimPlugin is to not have to add dependencies. Could this have been deleted instead ? same remark for the rest.
There was a problem hiding this comment.
Nixvim's update has been blocked for a while because these overrides were no longer being applied (they we're being silently shadowed by the luaPackages-based plugins). So at least currently, most of them are still needed.
I agree, ideally buildNeovimPlugin and/or toVimPackage would correctly propagate the underlying lua package's dependencies, however it seems like that isn't currently happening.
It's also possible that the dependencies are being propagated, but there is an issue in nixvim's combine-plugins impl. IIRC we only saw errors when combine-pluigins was being used.
Either way, overrides shouldn't be silently ignored, and this PR fixes that.
There was a problem hiding this comment.
It's also possible that the dependencies are being propagated, but there is an issue in nixvim's combine-plugins impl
Looking at nixvim's impl, that may be the case. It looks like we explicitly flat-map plugins and their dependencies but don't consider things like propagatedBuildInputs.
These comments look liked debug comments. Maybe because the plugin definition have been moved to another file while before you could have jsut searched them within the file. The PR changes the infra enough that this could have waited for more feedback rather than a blitzmerge. Some bikeshedding for instance. Like |
NixOS/nixpkgs#390702 Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/5d9b5431f967007b3952c057fc92af49a4c5f3b2?narHash=sha256-nkH2Edu9rClcsQp2PYBe8E6fp8LDPi2uDBQ6wyMdeXI%3D' (2025-03-16) → 'github:NixOS/nixpkgs/0ca1fc33509563e87989fad2cbc3d39f988570de?narHash=sha256-sgwENLuCiEPZvHOKPPjmmNdeO1xZurC29aWiP1n5X1w%3D' (2025-03-17)
NixOS/nixpkgs#390702 Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/5d9b5431f967007b3952c057fc92af49a4c5f3b2?narHash=sha256-nkH2Edu9rClcsQp2PYBe8E6fp8LDPi2uDBQ6wyMdeXI%3D' (2025-03-16) → 'github:NixOS/nixpkgs/0ca1fc33509563e87989fad2cbc3d39f988570de?narHash=sha256-sgwENLuCiEPZvHOKPPjmmNdeO1xZurC29aWiP1n5X1w%3D' (2025-03-17)
|
it would be neat if we could remove those overrides before release or alternatively move them to extra.nix ? |
|
As per my original description, those notes were mainly intended to aid the PR review and highlight overrides that should probably be removed at some point. I think it's fine for IMO once we've figured out which overrides are needed, the ones that aren't needed can be dropped and the others can have the comment removed. At that point it shouldn't matter where the base package came from. I'm starting to suspect that most of the overrides shouldn't be necessary, and the reason not having the None of us in nixvim are too familiar with |
4 participants
I noticed that since #377508 vimPlugins overrides are not being applied to plugins that are now based on luaPackages.
lib.pipeI've added some
NOTEcomments to each of the plugins with overrides affected by this PR, hopefully this'll aid the review. My assumption is that these overrides are still relevant even after #377508. I can remove these comments if you think they aren't needed.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.