From 16a6559ec6e18d053f161755684656551a3c3b83 Mon Sep 17 00:00:00 2001 From: Evgeni Chasnovski Date: Thu, 23 Oct 2025 18:35:27 +0300 Subject: [PATCH] fix(pack)!: do not trigger `PackChanged[Pre] kind=update` during install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- runtime/lua/vim/pack.lua | 28 ++++++++++------------------ test/functional/plugin/pack_spec.lua | 16 +++------------- 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/runtime/lua/vim/pack.lua b/runtime/lua/vim/pack.lua index f17fce56a9..168b9febcd 100644 --- a/runtime/lua/vim/pack.lua +++ b/runtime/lua/vim/pack.lua @@ -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') diff --git a/test/functional/plugin/pack_spec.lua b/test/functional/plugin/pack_spec.lua index eeb1fe7be6..3ff360511e 100644 --- a/test/functional/plugin/pack_spec.lua +++ b/test/functional/plugin/pack_spec.lua @@ -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()