after key does not work: plugin loading order is wrong for two opt plugins

[jdhao]

• nvim --version: v0.6.0-dev+3-gd791274a9
• git --version: 2.21.0
• Operating system/version: macOS 10.15.7
• Terminal name/version: kitty

Steps to reproduce

Use the following minimal config:

lua << EOF
require('packer').startup(function()
    use {'vim-airline/vim-airline', event = 'VimEnter', after = 'vim-airline-themes'}
    use {'vim-airline/vim-airline-themes', event = 'VimEnter'}
end)
EOF

1. start nvim with the above config: nvim -u init.vim
2. Use PakcerCompile to generate packer_compiled.lua.
3. Use scriptnames to check the order of loading vim-airline and vim-airline-themes

Actual behaviour

THe plugin vim-airline-themes is loaded after vim-airline.

Expected behaviour

The plugin vim-airline-themes should be loaded before vim-airline.

packer log file

[DEBUG Thu Jul 8 23:34:46 2021 8.7970025648637e+14] ...e/nvim/site/pack/packer/start/packer.nvim/lua/packer.lua:302: Processing plugin specs [INFO Thu Jul 8 23:34:46 2021 8.7970026048322e+14] ...e/nvim/site/pack/packer/start/packer.nvim/lua/packer.lua:676: Finished compiling lazy-loaders!

packer compiled file

``` -- Automatically generated packer.nvim plugin loader code if vim.api.nvim_call_function('has', {'nvim-0.5'}) ~= 1 then vim.api.nvim_command('echohl WarningMsg | echom "Invalid Neovim version for packer.nvim! | echohl None"') return end vim.api.nvim_command('packadd packer.nvim') local no_errors, error_msg = pcall(function() local time local profile_info local should_profile = false if should_profile then local hrtime = vim.loop.hrtime profile_info = {} time = function(chunk, start) if start then profile_info[chunk] = hrtime() else profile_info[chunk] = (hrtime() - profile_info[chunk]) / 1e6 end end else time = function(chunk, start) end end local function save_profiles(threshold) local sorted_times = {} for chunk_name, time_taken in pairs(profile_info) do sorted_times[#sorted_times + 1] = {chunk_name, time_taken} end table.sort(sorted_times, function(a, b) return a[2] > b[2] end) local results = {} for i, elem in ipairs(sorted_times) do if not threshold or threshold and elem[2] > threshold then results[i] = elem[1] .. ' took ' .. elem[2] .. 'ms' end end _G._packer = _G._packer or {} _G._packer.profile_output = results end time([[Luarocks path setup]], true) local package_path_str = "/Users/jdhao/.cache/nvim/packer_hererocks/2.1.0-beta3/share/lua/5.1/?.lua;/Users/jdhao/.cache/nvim/packer_hererocks/2.1.0-beta3/share/lua/5.1/?/init.lua;/Users/jdhao/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/luarocks/rocks-5.1/?.lua;/Users/jdhao/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/luarocks/rocks-5.1/?/init.lua" local install_cpath_pattern = "/Users/jdhao/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/lua/5.1/?.so" if not string.find(package.path, package_path_str, 1, true) then package.path = package.path .. ';' .. package_path_str end if not string.find(package.cpath, install_cpath_pattern, 1, true) then package.cpath = package.cpath .. ';' .. install_cpath_pattern end time([[Luarocks path setup]], false) time([[try_loadstring definition]], true) local function try_loadstring(s, component, name) local success, result = pcall(loadstring(s)) if not success then vim.schedule(function() vim.api.nvim_notify('packer.nvim: Error running ' .. component .. ' for ' .. name .. ': ' .. result, vim.log.levels.ERROR, {}) end) end return result end time([[try_loadstring definition]], false) time([[Defining packer_plugins]], true) _G.packer_plugins = { ["vim-airline"] = { load_after = { ["vim-airline-themes"] = true }, loaded = false, needs_bufread = false, path = "/Users/jdhao/.local/share/nvim/site/pack/packer/opt/vim-airline" }, ["vim-airline-themes"] = { after = { "vim-airline" }, loaded = false, needs_bufread = false, path = "/Users/jdhao/.local/share/nvim/site/pack/packer/opt/vim-airline-themes" } } time([[Defining packer_plugins]], false) vim.cmd [[augroup packer_load_aucmds]] vim.cmd [[au!]] -- Event lazy-loads time([[Defining lazy-load event autocommands]], true) vim.cmd [[au VimEnter * ++once lua require("packer.load")({'vim-airline', 'vim-airline-themes'}, { event = "VimEnter *" }, _G.packer_plugins)]] time([[Defining lazy-load event autocommands]], false) vim.cmd("augroup END") if should_profile then save_profiles() end end) if not no_errors then vim.api.nvim_command('echohl ErrorMsg | echom "Error in packer_compiled: '..error_msg..'" | echom "Please check your config for correctness" | echohl None') end ```

[jdhao]

This issue is similar to #272 and #435, though I set both the two plugins as opt plugins.

It seems that I need to put vim-airline-themes before vim-airline. The following works:

lua << EOF
require('packer').startup(function()
    use {'vim-airline/vim-airline-themes', event = 'VimEnter'}
    use {'vim-airline/vim-airline', event = 'VimEnter', after = 'vim-airline-themes'}
end)
EOF

I am not sure if this is a bug or my misunderstanding of usage of after 😿

[akinsho]

@jdhao can you try without adding the event = 'VimEnter' to the plugin that has the after key. When you add event to both it will create an autocommand on VimEnter to load both so you will have a race condition. You only need the after key for vim-airline not both. @wbthomason if this is the case ie. an after and event conflict maybe we should add a warning when a user sets an after but also sets an event since guaranteeing ordering gets very weird in that scenario 🤔 .

[jdhao]

@akinsho Removing VimEnter for vim-airline also seems to work as expected, i.e., the following works:

lua << EOF
require('packer').startup(function()
    use {'vim-airline/vim-airline-themes', event = 'VimEnter'}
    use {'vim-airline/vim-airline', after = 'vim-airline-themes'}
end)
EOF

[wbthomason]

@akinsho Adding a warning might be a good idea, but: this happens less because of the race condition (which does exist here) and more because we treat sequencing constraints as we do the other load-control operators, i.e. as a union of conditions rather than a conjunction. We could change the semantics of this in load.lua, so that a plugin loaded like in @jdhao's example only loads if its sequencing requirements are all satisfied, which effectively nullifies the event loader (we wouldn't want to make it load only on both the event and sequencing requirements, since that could lead to e.g. a plugin not loading the first time an event happens and needing a second instance of the same event to load). This might be more what people expect?

[akinsho]

@wbthomason just to clarify,

so that a plugin loaded like in @jdhao's example only loads if its sequencing requirements are all satisfied

By all sequencing requirements do you mean the chain of after's essentially? do other sequencing requirements currently exist? How would this look in practice, adding some logic to load to test if the sequencing requirements have all been met and if not, not loading the plugin. So for example the event loader would fire but check and see that the after requirement hasn't been met yet so would be a noop?

[wbthomason]

@akinsho Thinking about how to answer led me to discover a possible bug, so thanks!

Right now, we handle after by (1) in the compile step, doing a topological sort of the graph formed by sequencing dependency relations, then (2) in the load step, checking if any of the plugins which are supposed to load after a plugin currently being loaded are "ready" (i.e. have all of their dependencies satisfied). Step (2) happens here: https://github.com/wbthomason/packer.nvim/blob/c1aa0c773f764950d5e11efb8cba62d6e1b462fc/lua/packer/load.lua#L57-L65

The potential issue with this is that, because we haven't finished loading the original plugin by the time we load the sequenced plugin at line 62, it's possible that the sequenced plugin gets loaded before all of the effects of loading the original plugin have applied, e.g. the needs_bufread condition, triggering the FileType event, etc.

What I had been referencing in my response above was changing the check in lines 57-65 to not only require that a plugin's sequencing dependencies have all been loaded, but also that its other conditions (e.g. a specific filetype or command) have been met. This would work by simplifying the above code to just not call packer_load on the sequenced plugin if it has other lazy-loaders; instead, once a plugin's sequencing dependencies are satisfied it will get loaded like "normal" the next time one of its lazy-loaders gets triggered. This would also require changing the load function to not load plugins on lazy-load triggers if they still have unsatisfied sequencing dependencies.

While this provides behavior like @jdhao expected (i.e. by making sequencing requirements a block for other load conditions), it has the potential to create counterintuitive behavior where e.g. you may have to trigger a filetype event twice to get a plugin to load. I'd need to test this idea to make sure that it wouldn't cause breakage.

[lars-vc]

Is there any way to get this sort of behaviour right now? Because I would like to lazy load multiple plugins that depend on eachother

[EdenEast]

@lars-vc here is an example of me lazy-loading multiple plugins that all depend on eachother as an example. I am lazyloading in the setup function of autopairs but that can also be used with an event like VimEnter or entering insert mode.

I have it so that one root plugin gets loaded and then everything else is loaded by the chain of after. Hope that helps.

[lars-vc]

Thanks a bunch for this example! I managed to get it working with this :+1: