fix(pack): ensure data spec is passed in events during lockfile sync #38139

Problem: During initial "bootstrap" via lockfile synchronization, the
  whole plugin specification is reconstructed from the lockfile data,
  ignoring potential user changes added in the first `vim.pack.add()`.
  This is enough in most situations since it is the only data needed
  for actual installation.

  However, this affects specification passed to `PackChanged[Pre]`
  events. In particular, `data` field is missing which can be a problem
  if there is a `PackChanged kind=install` hook that uses that field
  (like with some kind of `build` method used during install).
  And there might be different `version` set in `vim.pack.add()`.

Solution: Pass the `specs` input of the first `vim.pack.add()` down to
  lockfile synchronization and use it to reconstruct plugin
  specification for the to-be-installed plugin. If present among the
  user's `specs`, it is used but with forced `src` from the lockfile (as
  it is the one used during installation).

  Note that this still has a caveat when using separate
  `vim.pack.add()`, as only the specs from the first input (when the
  lockfile synchronization happens) is taken into account.

runtime/lua/vim/pack.lua

@@ -843,7 +843,10 @@ end
 --- - Install plugins that have proper lockfile data but are not on disk.
 --- - Repair corrupted lock data for installed plugins.
 --- - Remove unrepairable corrupted lock data and plugins.
-local function lock_sync(confirm)
+--- @param confirm boolean
+--- @param specs vim.pack.Spec[] Plugin specs provided by the user. Can contain
+--- fields outside of what is in the lockfile to be passed down to events.
+local function lock_sync(confirm, specs)
   if type(plugin_lock.plugins) ~= 'table' then
     plugin_lock.plugins = {}
   end
@@ -885,7 +888,22 @@ local function lock_sync(confirm)
       local t = installed[name] == 'directory' and to_repair or to_remove
       t[#t + 1] = name
     elseif not installed[name] then
-      local spec = { src = data.src, name = name, version = data.version }
+      local spec ---@type vim.pack.Spec
+      -- Try reusing spec from user's `vim.pack.add()` (matters for events)
+      -- Delay until this point when shaving milliseconds shouldn't matter much
+      for _, s in ipairs(specs) do
+        local ok, s_norm = pcall(normalize_spec, s)
+        if ok and s_norm.name == name then
+          spec = vim.deepcopy(s_norm)
+        end
+      end
+
+      -- Force fields relevant to actual installation, try to preserve others
+      spec = spec or {}
+      spec.src = data.src
+      spec.name = name
+      spec.version = spec.version or data.version
+
       to_install[#to_install + 1] = new_plug(spec, plug_dir)
     end
   end
@@ -917,7 +935,7 @@ local function lock_sync(confirm)
   end
 end
 
-local function lock_read(confirm)
+local function lock_read(confirm, specs)
   if plugin_lock then
     return
   end
@@ -932,7 +950,7 @@ local function lock_read(confirm)
     plugin_lock = { plugins = {} }
   end
 
-  lock_sync(vim.F.if_nil(confirm, true))
+  lock_sync(vim.F.if_nil(confirm, true), vim.F.if_nil(specs, {}))
 end
 
 --- @class vim.pack.keyset.add
@@ -973,7 +991,7 @@ function M.add(specs, opts)
   opts = vim.tbl_extend('force', { load = vim.v.vim_did_init == 1, confirm = true }, opts or {})
   vim.validate('opts', opts, 'table')
 
-  lock_read(opts.confirm)
+  lock_read(opts.confirm, specs)
 
   local plug_dir = get_plug_dir()
   local plugs = {} --- @type vim.pack.Plug[]

test/functional/plugin/pack_spec.lua

@@ -279,9 +279,9 @@ end
 --- @param log table[]
 local function make_find_packchanged(log)
   --- @param suffix string
-  return function(suffix, kind, repo_name, version, active)
-    local path = pack_get_plug_path(repo_name)
-    local spec = { name = repo_name, src = repos_src[repo_name], version = version }
+  return function(suffix, kind, name, version, active, user_data)
+    local path = pack_get_plug_path(name)
+    local spec = { name = name, src = repos_src[name], version = version, data = user_data }
     local data = { active = active, kind = kind, path = path, spec = spec }
     local entry = { event = 'PackChanged' .. suffix, match = vim.fs.abspath(path), data = data }
 
@@ -626,8 +626,10 @@ describe('vim.pack', function()
 
       mock_confirm(1)
       -- Should use revision from lockfile (pointing at latest 'feat-branch'
-      -- commit) and not use latest `main` commit
-      vim_pack_add({ { src = repos_src.basic, version = 'main' } })
+      -- commit) and not use latest `main` commit. Although should report
+      -- `version = 'main'` inside event data to preserve user input as much as possible.
+      -- Should also preserve `data` field in event data.
+      vim_pack_add({ { src = repos_src.basic, version = 'main', data = { 'd' } } })
       pack_assert_content('basic', 'return "basic feat-branch"')
 
       local confirm_log = exec_lua('return _G.confirm_log')
@@ -641,9 +643,9 @@ describe('vim.pack', function()
       -- Should trigger `kind=install` events
       local log = exec_lua('return _G.event_log')
       local find_event = make_find_packchanged(log)
-      local installpre_basic = find_event('Pre', 'install', 'basic', 'feat-branch', false)
+      local installpre_basic = find_event('Pre', 'install', 'basic', 'main', false, { 'd' })
       local installpre_defbranch = find_event('Pre', 'install', 'defbranch', nil, false)
-      local install_basic = find_event('', 'install', 'basic', 'feat-branch', false)
+      local install_basic = find_event('', 'install', 'basic', 'main', false, { 'd' })
       local install_defbranch = find_event('', 'install', 'defbranch', nil, false)
       eq(4, #log)
       eq(true, installpre_basic < install_basic)
@@ -658,6 +660,15 @@ describe('vim.pack', function()
       ref_lockfile.plugins.basic.rev = git_get_hash('main', 'basic')
       ref_lockfile.plugins.basic.version = "'main'"
       eq(ref_lockfile, get_lock_tbl())
+
+      -- Improper or string spec input should not interfere with initial install
+      vim.fs.rm(pack_get_dir(), { force = true, recursive = true })
+      n.clear()
+
+      mock_confirm(1)
+      pcall_err(vim_pack_add, { repos_src.basic, 1 })
+      eq(true, pack_exists('basic'))
+      eq(true, pack_exists('defbranch'))
     end)
 
     it('handles lockfile during install errors', function()