mirror of
https://github.com/neovim/neovim.git
synced 2025-10-26 12:27:24 +00:00
fix(pack)!: do not trigger PackChanged[Pre] kind=update during install
Problem: `PackChanged[Pre]` events with `kind=update` are triggered both during plugin's initial installation and after already installed plugin was updated. It was a deliberate decision to allow writing only a single update hook to act as a dedicated "build" entry point (like execute `make` or `cargo build —release`). This mimics how other plugin managers have a single "build" command. This was a result of 'mini.deps' experience with the different approach: "update" hooks are not run during install. This proved to be confusing as it requires to write two hooks. But also the reason might be that 'mini.deps' names it "checkout" hook instead of "update". However, the `vim.pack` event approach makes it lower cost to handle separate "update" and "install" events. Something like `if ev.data.kind == 'install' or ev.data.kind == 'update' then` instead of two autocommands. Plus this makes clearer separation of events. Solution: do not trigger `PackChanged[Pre] kind=update` event during install.
This commit is contained in:
@@ -588,14 +588,8 @@ end
|
||||
--- @async
|
||||
--- @param p vim.pack.Plug
|
||||
--- @param timestamp string
|
||||
--- @param skip_same_sha boolean
|
||||
local function checkout(p, timestamp, skip_same_sha)
|
||||
local function checkout(p, timestamp)
|
||||
infer_states(p)
|
||||
if skip_same_sha and p.info.sha_head == p.info.sha_target then
|
||||
return
|
||||
end
|
||||
|
||||
trigger_event(p, 'PackChangedPre', 'update')
|
||||
|
||||
local msg = ('vim.pack: %s Stash before checkout'):format(timestamp)
|
||||
git_cmd({ 'stash', '--quiet', '--message', msg }, p.path)
|
||||
@@ -604,8 +598,6 @@ local function checkout(p, timestamp, skip_same_sha)
|
||||
|
||||
plugin_lock.plugins[p.spec.name].rev = p.info.sha_target
|
||||
|
||||
trigger_event(p, 'PackChanged', 'update')
|
||||
|
||||
-- (Re)Generate help tags according to the current help files.
|
||||
-- Also use `pcall()` because `:helptags` errors if there is no 'doc/'
|
||||
-- directory or if it is empty.
|
||||
@@ -638,12 +630,8 @@ local function install_list(plug_list, confirm)
|
||||
-- Prefer revision from the lockfile instead of using `version`
|
||||
p.info.sha_target = (plugin_lock.plugins[p.spec.name] or {}).rev
|
||||
|
||||
-- Do not skip checkout even if HEAD and target have same commit hash to
|
||||
-- have new repo in expected detached HEAD state and generated help files.
|
||||
checkout(p, timestamp, false)
|
||||
checkout(p, timestamp)
|
||||
|
||||
-- "Install" event is triggered after "update" event intentionally to have
|
||||
-- it indicate "plugin is installed in its correct initial version"
|
||||
trigger_event(p, 'PackChanged', 'install')
|
||||
end
|
||||
run_list(plug_list, do_install, 'Installing plugins')
|
||||
@@ -1037,9 +1025,11 @@ function M.update(names, opts)
|
||||
-- Compute change info: changelog if any, new tags if nothing to update
|
||||
infer_update_details(p)
|
||||
|
||||
-- Checkout immediately if not need to confirm
|
||||
if opts.force then
|
||||
checkout(p, timestamp, true)
|
||||
-- Checkout immediately if no need to confirm
|
||||
if opts.force and p.info.sha_head ~= p.info.sha_target then
|
||||
trigger_event(p, 'PackChangedPre', 'update')
|
||||
checkout(p, timestamp)
|
||||
trigger_event(p, 'PackChanged', 'update')
|
||||
end
|
||||
end
|
||||
local progress_title = opts.force and (opts._offline and 'Applying updates' or 'Updating')
|
||||
@@ -1070,7 +1060,9 @@ function M.update(names, opts)
|
||||
--- @async
|
||||
--- @param p vim.pack.Plug
|
||||
local function do_checkout(p)
|
||||
checkout(p, timestamp2, true)
|
||||
trigger_event(p, 'PackChangedPre', 'update')
|
||||
checkout(p, timestamp2)
|
||||
trigger_event(p, 'PackChanged', 'update')
|
||||
end
|
||||
run_list(plugs_to_checkout, do_checkout, 'Applying updates')
|
||||
|
||||
|
||||
@@ -674,23 +674,13 @@ describe('vim.pack', function()
|
||||
local find_event = make_find_packchanged(log)
|
||||
local installpre_basic = find_event('Pre', 'install', 'basic', 'feat-branch', false)
|
||||
local installpre_defbranch = find_event('Pre', 'install', 'defbranch', nil, false)
|
||||
local updatepre_basic = find_event('Pre', 'update', 'basic', 'feat-branch', false)
|
||||
local updatepre_defbranch = find_event('Pre', 'update', 'defbranch', nil, false)
|
||||
local update_basic = find_event('', 'update', 'basic', 'feat-branch', false)
|
||||
local update_defbranch = find_event('', 'update', 'defbranch', nil, false)
|
||||
local install_basic = find_event('', 'install', 'basic', 'feat-branch', false)
|
||||
local install_defbranch = find_event('', 'install', 'defbranch', nil, false)
|
||||
eq(8, #log)
|
||||
eq(4, #log)
|
||||
|
||||
-- NOTE: There is no guaranteed installation order among separate plugins (as it is async)
|
||||
eq(true, installpre_basic < updatepre_basic)
|
||||
eq(true, updatepre_basic < update_basic)
|
||||
-- NOTE: "Install" is after "update" to indicate installation at correct version
|
||||
eq(true, update_basic < install_basic)
|
||||
|
||||
eq(true, installpre_defbranch < updatepre_defbranch)
|
||||
eq(true, updatepre_defbranch < update_defbranch)
|
||||
eq(true, update_defbranch < install_defbranch)
|
||||
eq(true, installpre_basic < install_basic)
|
||||
eq(true, installpre_defbranch < install_defbranch)
|
||||
end)
|
||||
|
||||
it('recognizes several `version` types', function()
|
||||
|
||||
Reference in New Issue
Block a user