modules/performance/combinePlugins: propagate dependencies

[MattSturgeon]

An attempt at having combinePlugins correctly propagate dependencies from the input plugins.

Marking as a draft for now until we can verify it works correctly, but otherwise ready for review.

Fixes #3140

[MattSturgeon]

Seems like this still isn't working. Tests for combined plugins are still failing with missing plenary modules.

I wonder if buildNeovimPlugin does something special with propagated inputs? Or if I'm overlooking something else fundamental.

It'd help if I understood why vim plugins packaged with buildNeovimPlugin seem to work fine when the underlying lua package was using propagatedBuildInputs to declare its dependencies.

Maybe @teto or @mrcjkb have some insights?

[MattSturgeon]

Hm, I was hoping this would help. Perhaps the pathsToLink argument is causing this to not work correctly?

• https://buildbot.nix-community.org/#/builders/2795/builds/3822/steps/1/logs/stdio
• https://buildbot.nix-community.org/#/builders/2920/builds/3481/steps/1/logs/stdio

[MattSturgeon]

I've updated this to use the requiredLuaModules attr, but this still doesn't seem to be helping. Looking at the nixpkgs lua wrapper, there may be some more work needed below in the "plugin pack" env package, such as using buildLuaPath?

[teto]

I haven't looked at the code but if you look at the neovim wrapper, it doesn't add all lua dependencies in packpath but wraps neovim with LUA_PATH and LUA_CPATH.

[MattSturgeon]

Yeah, looks like the wrapper is using the _addToLuaPath lua util to build LUA_PATH and LUA_CPATH from the plugin pack.

I'm trying to figure out why our performance.combinePlugins feature is breaking that functionality.

To be specific, if any of the plugins being "combined" have any required lua modules, these do not seem to be included in the LUA_PATH. If the same plugin is installed normally, then it works correctly.

[MattSturgeon]

It turns out we were testing incorrectly (see #3121). This was not resolved by NixOS/nixpkgs@f9af047 after all.

[MattSturgeon]

Testing this out in the repl, I can see plenary makes it into the combinedPlugin's propagatedBuildInputs attr:

nix-repl> p = builtins.head checks.x86_64-linux.test-30.entries.plugins-by-name-telescope-fzf-native.entries.combine-plugins.

nix-repl> p.plugin.propagatedBuildInputs
[
  «derivation /nix/store/7rvzbnx1fb6d69jdzgc3bcm83xljafgw-luajit2.1-plenary.nvim-scm-1.drv»
  «derivation /nix/store/p8nl4zm9m03jldrmmln7dnkji1fw1d2b-luajit-2.1.1741730670.drv»
  «repeated»
  «repeated»
  «repeated»
]

However the test still fails:

$ nix build .#checks.x86_64-linux.test-30.entries.plugins-by-name-telescope-fzf-native.entries.combine-plugins
error: builder for '/nix/store/l894cwcld2xavbjhs4xjdiwvxwrzy367-combine-plugins.drv' failed with exit code 1;
       last 13 log lines:
       > ERROR: Error detected while processing /nix/store/9v2z5rgvc38jk4cg9fj2518cdv74fjml-init.lua:
       > E5113: Error while calling lua chunk: ...ges/start/vimplugin-plugin-pack/lua/telescope/config.lua:1: module 'plenary.strings' not found:
       >        no field package.preload['plenary.strings']
       >    no file '/nix/store/0hvhcdk5i176a5qxfrv95p09zvg96290-luajit-2.1.1741730670-env/share/lua/5.1/plenary/strings.lua'
       >      no file '/nix/store/0hvhcdk5i176a5qxfrv95p09zvg96290-luajit-2.1.1741730670-env/share/lua/5.1/plenary/strings/init.lua'
       >         no file '/nix/store/0hvhcdk5i176a5qxfrv95p09zvg96290-luajit-2.1.1741730670-env/lib/lua/5.1/plenary/strings.so'
       >         no file '/nix/store/0hvhcdk5i176a5qxfrv95p09zvg96290-luajit-2.1.1741730670-env/lib/lua/5.1/plenary.so'
       > stack traceback:
       >       [C]: in function 'require'
       >     ...ges/start/vimplugin-plugin-pack/lua/telescope/config.lua:1: in main chunk
       >   [C]: in function 'require'
       >     ...kages/start/vimplugin-plugin-pack/lua/telescope/init.lua:133: in function 'setup'
       >   /nix/store/9v2z5rgvc38jk4cg9fj2518cdv74fjml-init.lua:11: in main chunk
       For full logs, run 'nix log /nix/store/l894cwcld2xavbjhs4xjdiwvxwrzy367-combine-plugins.drv'.

Inspecting what's going on in the plugin pack:

nix-repl> :b p.plugin

This derivation produced the following outputs:
  out -> /nix/store/00yccslmc75dbp2ax4gmndjccbgj66nn-vimplugin-plugin-pack


$ ls /nix/store/00yccslmc75dbp2ax4gmndjccbgj66nn-vimplugin-plugin-pack -la
total 0
dr-xr-xr-x 1 root root         74 Jan  1  1970 ./
drwxrwxr-t 1 root nixbld 34583300 Apr  1 20:37 ../
dr-xr-xr-x 1 root root         12 Jan  1  1970 autoload/
dr-xr-xr-x 1 root root         18 Jan  1  1970 build/
dr-xr-xr-x 1 root root         10 Jan  1  1970 data/
dr-xr-xr-x 1 root root        100 Jan  1  1970 doc/
dr-xr-xr-x 1 root root         78 Jan  1  1970 ftplugin/
dr-xr-xr-x 1 root root        126 Jan  1  1970 lua/
dr-xr-xr-x 1 root root         68 Jan  1  1970 plugin/

$ ls /nix/store/00yccslmc75dbp2ax4gmndjccbgj66nn-vimplugin-plugin-pack/nix-support
ls: cannot access '/nix/store/00yccslmc75dbp2ax4gmndjccbgj66nn-vimplugin-plugin-pack/nix-support': No such file or directory

Notably, there is no nix-support directory in the plugin-pack package (i.e. no nix-support/propagated-build-inputs file), despite it having a propagatedBuildInputs attr.

[MattSturgeon]

I'm not too familiar with $out/nix-support/propagated-build-inputs, but with some digging it looks like stdenv says the fixupPhase is responsible for writing propagated-build-inputs.

buildEnv uses runCommand under the hood, which in turn specifies buildCommand (not buildPhase), which bypasses the entire phase system.

I've updated the PR to manually write the $out/nix-support/propagated-build-inputs file, instead of attempting to define a propagatedBuildInputs attr.

[MattSturgeon]

It might be possible to use the recordPropagatedDependencies command?

[MattSturgeon]

Writing the $out/nix-support/propagated-build-inputs file has resolved most of the issues, however we now have multiple plenary modules in some tests:

ERROR: Error detected while processing /nix/store/l7pk31f2vpg40azx5z76pwk36fv1i6vf-init.lua:
E5113: Error while calling lua chunk: /nix/store/l7pk31f2vpg40azx5z76pwk36fv1i6vf-init.lua:22: more than one copy of plenary in runtime
stack traceback:
	[C]: in function 'assert'
	/nix/store/l7pk31f2vpg40azx5z76pwk36fv1i6vf-init.lua:22: in function 'is_standalone'
	/nix/store/l7pk31f2vpg40azx5z76pwk36fv1i6vf-init.lua:33: in main chunk

[MattSturgeon]

Seems like test-2.entries.modules-performance-combine-plugins.entries.standalone-plugins is one of the only failing case? That's this one:

nixvim/tests/test-sources/modules/performance/combine-plugins.nix

Lines 314 to 380 in 754b8df

	# Test that standalonePlugins option works
	standalone-plugins =
	{ config, ... }:
	{
	performance.combinePlugins = {
	enable = true;
	standalonePlugins = with pkgs.vimPlugins; [
	# By plugin name
	"nvim-treesitter"
	# By package itself
	nvim-lspconfig
	# Its dependency, plenary-nvim, not in this list, so will be combined
	telescope-nvim
	# Dependency of other plugin
	"nvim-cmp"
	];
	};
	extraPlugins = with pkgs.vimPlugins; [
	nvim-treesitter
	nvim-lspconfig
	telescope-nvim
	# Only its dependency (nvim-cmp) won't be combined, but not the plugin itself
	cmp-dictionary
	# We have to manually add cmp-dictionary's dependence: nvim-cmp
	nvim-cmp
	# More plugins
	gitsigns-nvim
	luasnip
	];
	extraConfigLuaPost = ''
	-- Plugins are loadable
	require("nvim-treesitter")
	require("lspconfig")
	require("telescope")
	require("plenary")
	require("cmp_dictionary")
	require("cmp")
	require("gitsigns")
	require("luasnip")
	-- Verify if plugin is standalone or combined
	local function is_standalone(name, dirname)
	local paths = vim.api.nvim_get_runtime_file("lua/" .. name, true);
	assert(#paths == 1, "more than one copy of " .. name .. " in runtime")
	return paths[1]:match("^(.+)/lua/"):find(dirname or name, 1, true) ~= nil
	end
	-- Standalone plugins
	assert(is_standalone("nvim-treesitter"), "nvim-treesitter is combined, expected standalone")
	assert(is_standalone("lspconfig"), "nvim-lspconfig is combined, expected standalone")
	assert(is_standalone("telescope"), "telescope-nvim is combined, expected standalone")
	-- Add trailing slash to ensure that it doesn't match cmp_dictionary
	assert(is_standalone("cmp/", "nvim-cmp"), "nvim-cmp is combined, expected standalone")
	-- Combined plugins
	assert(not is_standalone("plenary"), "plenary-nvim is standalone, expected combined")
	assert(not is_standalone("cmp_dictionary", "cmp-dictionary"), "cmp-dictionary is standalone, expected combined")
	assert(not is_standalone("gitsigns"), "gitsigns-nvim is standalone, expected combined")
	assert(not is_standalone("luasnip"), "luasnip is standalone, expected combined")
	'';
	assertions = [
	{
	# plugin-pack, nvim-treesitter, nvim-lspconfig, telescope-nvim, nvim-cmp
	assertion = pluginCount config.build.nvimPackage config.build.extraFiles "start" == 5;
	message = "Wrong number of plugins in packpathDirs";
	}
	];
	};

Ah, test-2.entries.modules-performance-byte-compile-lua.entries.plugins-combined is also failing:

nixvim/tests/test-sources/modules/performance/byte-compile-lua.nix

Lines 233 to 299 in 754b8df

	# Two equal tests, one with combinePlugins.enable = true
	pkgs.lib.genAttrs
	[
	"plugins"
	"plugins-combined"
	]
	(name: {
	performance = {
	byteCompileLua = {
	enable = true;
	plugins = true;
	};
	combinePlugins.enable = pkgs.lib.hasSuffix "combined" name;
	};
	extraPlugins = with pkgs.vimPlugins; [
	nvim-lspconfig
	# Depends on plenary-nvim
	telescope-nvim
	# buildCommand plugin with python3 dependency
	((pkgs.writeTextDir "/plugin/test.lua" "vim.opt.tabstop = 2").overrideAttrs {
	passthru.python3Dependencies = ps: [ ps.pyyaml ];
	})
	# Plugin with invalid lua file tests/indent/lua/cond.lua (should be ignored)
	nvim-treesitter
	];
	extraConfigLuaPost = ''
	${isByteCompiledFun}
	-- Plugins are loadable
	require("lspconfig")
	require("telescope")
	require("plenary")
	require("nvim-treesitter")
	-- Python modules are importable
	vim.cmd.py3("import yaml")
	-- nvim-lspconfig
	test_rtp_file("lua/lspconfig.lua", true)
	test_rtp_file("lua/lspconfig/configs/nixd.lua", true)
	test_rtp_file("plugin/lspconfig.lua", true)
	test_rtp_file("doc/lspconfig.txt", false)
	-- telescope-nvim
	test_rtp_file("lua/telescope/init.lua", true)
	test_rtp_file("lua/telescope/builtin/init.lua", true)
	test_rtp_file("plugin/telescope.lua", true)
	test_rtp_file("autoload/health/telescope.vim", false)
	test_rtp_file("doc/telescope.txt", false)
	-- Dependency of telescope-nvim (plenary-nvim)
	test_rtp_file("lua/plenary/init.lua", true)
	test_rtp_file("plugin/plenary.vim", false)
	-- Test plugin
	test_rtp_file("plugin/test.lua", true)
	-- nvim-treesitter
	test_rtp_file("lua/nvim-treesitter/health.lua", true)
	test_rtp_file("lua/nvim-treesitter/install.lua", true)
	test_rtp_file("plugin/nvim-treesitter.lua", true)
	test_rtp_file("queries/nix/highlights.scm", false)
	'';
	})

[MattSturgeon]

cc @stasjok

[stasjok]

combinePlugins functionality is based on internal implementation details from nixpkgs, so I'll need to debug what was changed and how it works now. I'll check if I can get it fixed or find a workaround. Or maybe some tests can be wrong, because they were written on the assumption on what plugin has what dependency. No ETA.