Feature: Lockfile support

EdenEast commented 2 years ago

This started as a idea's discussion post. Creating a feature issue instead to make it an actual feature request.

Describe the feature

Like many other users I store my neovim's configuration in source control. I can use my configuration on many different systems. The problem is that my configuration can only be validated to work with my local plugins at their current git revisions. The state of the local plugins should also be commitable. As neovim's ecosystem is constantly changing this would prevent an update from breaking my config and causing me to maintain it. Now I can choose when I have time to maintain my config.

Why not snapshots?

Setting snapshot config option calls snapshot rollup every time neovim start
- This preforms a git fetch every for every plugin in the snapshot file ever time neovim is started
Not integrated into packer commands like install and update
Snapshot file not formatted for diffing changes and not sorted (not idempotent).
- Having an PackerSnapshotDone autocmd would at least let us call jq to sort and pretty print. This would make snapshot's diff understandable and idempotent.

Snapshots feel more like a git stash. A way of taking a temporary snapshot of my current state while experimenting. Something that I will eventually throw away. It is it's own system tacked onto packer and not integrated into it.

Proposal

Packer should provide a lockfile option. When enabled, packer would apply this lockfile to git plugin type's installer and updater functions. This would integrate the lockfile (if enabled) into the commands install, update and sync.

The lockfile is a lua file that returns a table mapping plugins to their commit hashes. The lockfile entries are also sorted so that multiple calls are idempotent.

Configuration

local config_defaults = {
  -- ...
  lockfile = {
    -- Apply lockfile on packer commands (Defaults to false for an opt-in feature)
    enable = false,
    -- Default lockfile filepath. Defaults to config location as it should be commited into dotfiles
    path = join_paths(stdpath("config"), "lockfile.lua"),
    -- Update lockfile after an upgrade call
    update_on_upgrade = true,
  },
}

Commands

A new command would be added called upgrade. This would temporally disable the lockfile and then execute an update. Once the update is finished the lockfile will also be updated if update_on_upgrade is enabled.

The other new command that would be added is lockfile. This command will update the lockfile with the current state of your local plugins. The internal lockfile table would also be re-loaded. This is useful is someone sets update_on_upgrade to false and only wants to update the lockfile once they have tested the upgrade is stable. If it is not then they can just call update and the lockfile will be reapplied.

Applying lockfile

The lockfile is applied if:

The plugin name is found in lockfile table
The plugin is a git type (not local)
There is no tag key

The lockfile is also checked and applied to any plugin spec defined in requires key.

Updating lockfile

Since local plugin types are ignored by the lockfile, if an entry exists in the current lockfile that value is reused.

Example lockfile

-- Automatically generated by packer.nvim
return {
  ["LuaSnip"] = { commit = "c599c56", date = "1659863559" },
  ["alpha-nvim"] = { commit = "d688f46", date = "1658591867" },
  ["cmp-buffer"] = { commit = "62fc67a", date = "1655319089" },
  ["cmp-emoji"] = { commit = "19075c3", date = "1632834658" },
  ["cmp-nvim-lsp"] = { commit = "affe808", date = "1652705110" },
  ["cmp-nvim-lua"] = { commit = "d276254", date = "1633919304" },
}

Commit dates

With the lockfile applied to both the install and update commands we can take advantage of actually knowing the commit we want. Along with the commit hash we can also save the commit's date. This would then allow the use of --shallow-since instead of --depth 999999. This makes clones and updates of repositories leaner both on network and diskspace. Getting the commit's date in unix time is simple:

git show -s --format="%ct" HEAD

Additional thoughts

Should the upgrade command perform the same operations as update or sync?
- Should this be controlled with a config option? (should it call compile?)

danielo515 commented 2 years ago

I see some projects implementing this in userland, for example, lunar-vim and, more recently, myself in my own config. This is something I really want, because there is nothing more frustrating than running packer sync to get new plugins installed and old ones removed just to discover that another 50+ plugins were updated... and that one of them is totally breaking my config...

wom commented 2 years ago

This. I run same dots on multiple machines; and it's hard to verify same environment if I have to bootstrap a new VM quickly (and it's a PITA to manually grab hashes). Packer Seems the logical place to handle it (as outlined here).

rwjblue commented 2 years ago

I see some projects implementing this in userland, for example, lunar-vim and, more recently, myself in my own config.

Exactly. I've done the same in my vim config.

See [code](https://github.com/rwjblue/dotvim/blob/master/lua/rwjblue/plugins.lua) for _how_ it works. [Example docs](https://github.com/rwjblue/dotvim/blob/master/README.md#plugin-management) (including how to add a new plugin without updating all the other hashes, updating all plugins, rolling back to "last known good" snapshot, etc).

EdenEast commented 2 years ago

@wbthomason, with the discussion here and your first draft of the v2 roadmap, I am wondering your thoughts on this feature request. With a rewrite it is also the time to look at packer's current features mainly snapshots. Or add this as a feature to add in stage 5. Would like to know your thoughts in general on a lockfile in packer.

wbthomason commented 2 years ago

@EdenEast thanks for the feature request, PR, and ping. I still need to review the PR, but my thoughts from this request and discussion are:

This seems like a feature the community wants
This feature is orthogonal to the main thrust of stage 1 of the rewrite plan, moving away from compilation. This could serve as an argument for implementing the feature now (since it won't interfere and will be easy to pull in) or waiting
Honestly, this feature seems like an improved version of snapshots, though I admit I never really groked why people use snapshots

Given this, and the fact that there's already a PR, I'm leaning toward merging a version of this in sooner than later. I think the best course of action might be to talk to the main stakeholders with snapshots and see if they'd be willing to replace the snapshot functionality with this feature, to prevent creep and duplication.

In somewhat more detail:

I'm hesitant to add another command. What about adding arguments to PackerSync/PackerUpdate to do the same thing, e.g. PackerSync no_lockfile or something?
I buy that there should be some new function/probably a corresponding command for regenerating the lockfile
I'm slightly leery of the semantics of applying the lockfile as proposed. It seems that, for users who aren't aware of these details, this could cause confusing situations in which plugins which they expect to be updated are not. If we use this approach, we should at least add an informational output about which plugins are locked (e.g. to the status output); otherwise, maybe the lockfile should always apply for the plugins it contains, regardless of separately specified tag, etc. (or, better still, the lockfile should apply and issue an error if the specified tag is different from the locked version, etc.)

I'll try to take a look over the PR in the next few days; please ping me if I haven't commented on it by early next week. Thanks!

EdenEast commented 2 years ago

Thanks for your initial thoughts on this feature. I too could not grok snapshots in the context of a package manager. I found that people were trying to use snapshots as a lockfile but the feature was not designed in such a way. Adding a lockfile in some form to the operation of packer made more sense to me and easier for others to understand how it works (a lockfile is a known and understood concept)

I would like to merge a simple (MVP) version of a lockfile before really trying to rewrite packer, as the lockfile needs to be integrated into the internals of packer. With that said I would like to get a simple interface for users and functionality that they would understand.

I'm hesitant to add another command. What about adding arguments to PackerSync/PackerUpdate to do the same thing, e.g. PackerSync no_lockfile or something? I buy that there should be some new function/probably a corresponding command for regenerating the lockfile

I used the term upgrade from other package managers. This was an attempt to differentiate between updating your local plugin store to the lockfile and updating the local store to the latest remote changes. Also used sync and update as examples as sync is just an update and compile.

I would propose that in the future rewrite that there would be only two updating commands. One that updates regardless of lockfile, and one that syncs with the lockfile. Should compile on update would be then moved to a config setting. If someone disables the lockfile they would produce the same result.
Another idea is to add this all to a lockfile command. This lockfile command would take arguments like regenerate to recreate the lockfile based on the local plugin store or update which would act update (or sync) the local plugin store with the plugins remote. (I think the current proposal is better than this option as I feel it is more clear and understandable)

I'm slightly leery of the semantics of applying the lockfile as proposed

I was not on handling the lockfile with tag and branch keys. I felt that if someone put a branch that they would want to track a different branch but still be able to lock at a commit. So that was fine but tag was different. If someone has a tag and a lockfile entry I was not sure how to handle this. I like the idea of issuing an error if the commit is not the same as the tag specified. That would make the most sense.

Adding lockfile information to packer's status is a good idea to help users understand the current packer state.

danielo515 commented 2 years ago

Will there be a command to upgrade a single package or list of packages? It’s also very important the ability to specify the lock file location

On Fri, 2 Sep 2022 at 04:54, EdenEast @.***> wrote:

Thanks for your initial thoughts on this feature. I too could not grok snapshots in the context of a package manager. I found that people were trying to use snapshots as a lockfile but the feature was not designed in such a way. Adding a lockfile in some form to the operation of packer made more sense to me and easier for others to understand how it works (a lockfile is a known and understood concept)

I would like to merge a simple (MVP) version of a lockfile before really trying to rewrite packer, as the lockfile needs to be integrated into the internals of packer. With that said I would like to get a simple interface for users and functionality that they would understand.

I'm hesitant to add another command. What about adding arguments to PackerSync/PackerUpdate to do the same thing, e.g. PackerSync no_lockfile or something? I buy that there should be some new function/probably a corresponding command for regenerating the lockfile

I used the term upgrade from other package managers. This was an attempt to differentiate between updating your local plugin store to the lockfile and updating the local store to the latest remote changes. Also used sync and update as examples as sync is just an update and compile.

-

I would propose that in the future rewrite that there would be only two updating commands. One that updates regardless of lockfile, and one that syncs with the lockfile. Should compile on update would be then moved to a config setting. If someone disables the lockfile they would produce the same result.

Another idea is to add this all to a lockfile command. This lockfile command would take arguments like regenerate to recreate the lockfile based on the local plugin store or update which would act update (or sync) the local plugin store with the plugins remote. (I think the current proposal is better than this option as I feel it is more clear and understandable)

I'm slightly leery of the semantics of applying the lockfile as proposed

I was not on handling the lockfile with tag and branch keys. I felt that if someone put a branch that they would want to track a different branch but still be able to lock at a commit. So that was fine but tag was different. If someone has a tag and a lockfile entry I was not sure how to handle this. I like the idea of issuing an error if the commit is not the same as the tag specified. That would make the most sense.

Adding lockfile information to packer's status is a good idea to help users understand the current packer state.

— Reply to this email directly, view it on GitHub https://github.com/wbthomason/packer.nvim/issues/1009#issuecomment-1235012346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWLDDU3JHKLRIZ5JABDV4FT7JANCNFSM56JDXWGA . You are receiving this because you commented.Message ID: @.***>

--

https://danielorodriguez.com

wbthomason commented 2 years ago

@danielo515: You can already specify a single package/list to upgrade with update() or sync(). I think that + lockfile regeneration should be enough to upgrade subsets of the lockfile at a time? And yes, specifying lockfile output location is important - for new lockfile creation, this should be an explicit parameter.

EdenEast commented 2 years ago

I can add an optional path argument to the lockfile command to generate to the filepath specified.

danielo515 commented 2 years ago

Yes please, because currently the generate snapshot command only accepts a file name, which is not very useful.

El sáb., 3 sept. 2022 3:09, EdenEast @.***> escribió:

I can add an optional path argument to the lockfile command to generate to the filepath specified.

— Reply to this email directly, view it on GitHub https://github.com/wbthomason/packer.nvim/issues/1009#issuecomment-1236014513, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWK2Q65I4436ZU5I3GLV4KQOBANCNFSM56JDXWGA . You are receiving this because you were mentioned.Message ID: @.***>

EdenEast commented 2 years ago

With this change I would also need some way of setting which lockfile to apply. This could be added as an argument to any install /update command or the lockfile command would have subcommands. The lockfile command could have two subcommands generate and (set / apply). Not sure on which method would be preferred. (Initially I lean towards an optional argument to install / update commands)

wbthomason commented 2 years ago

I strongly prefer an optional argument over more commands. Maybe something like this:

A config setting defines the default lockfile path
The lockfile creation function/command accepts an optional path to override the config value
A config setting defines whether lockfiles should be used by default or not
In install/update, if either the "auto-lockfile" setting is set and a lockfile exists at the default path OR an optional path to a lockfile is passed to the install/update functions, the lockfile is applied.

Does that seem reasonable?

danielo515 commented 2 years ago

What about using the first lock file found in runtimepath and ask the user in case more than one is found? And throw an error in headless mode in that case

El sáb., 3 sept. 2022 21:39, Wil Thomason @.***> escribió:

I strongly prefer an optional argument over more commands. Maybe something like this:

A config setting defines the default lockfile path

The lockfile creation function/command accepts an optional path to override the config value

A config setting defines whether lockfiles should be used by default or not

In install/update, if either the "auto-lockfile" setting is set and a lockfile exists at the default path OR an optional path to a lockfile is passed to the install/update functions, the lockfile is applied.

Does that seem reasonable?

— Reply to this email directly, view it on GitHub https://github.com/wbthomason/packer.nvim/issues/1009#issuecomment-1236187894, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWLUYACKS5ITGXWLCTTV4OSQVANCNFSM56JDXWGA . You are receiving this because you were mentioned.Message ID: @.***>

wbthomason commented 2 years ago

@danielo515 Can you elaborate on the benefit there vs. having a default lockfile setting?

danielo515 commented 2 years ago

Nothing, other than a little convenience for certain use cases

On Sun, 4 Sep 2022 at 02:45, Wil Thomason @.***> wrote:

@danielo515 https://github.com/danielo515 Can you elaborate on the benefit there vs. having a default lockfile setting?

— Reply to this email directly, view it on GitHub https://github.com/wbthomason/packer.nvim/issues/1009#issuecomment-1236224686, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWI2PDBFDFFGG4SHSYTV4PWLFANCNFSM56JDXWGA . You are receiving this because you were mentioned.Message ID: @.***>

--

https://danielorodriguez.com

EdenEast commented 2 years ago

Ok with the current conversation I propose a new command workflow. This will remove the upgrade command and instead add optional flags to the existing install, update and sync commands:

" Updating without applying lockfile
PackerUpdate --nolockfile

" Updating a specific plugin without applying lockfile
PackerUpdate --nolockfile plenary.nvim

" Generating a lockfile to some other path
PackerLockfile --path="/some/other/path.lua"

" Updating plugins and applying a specific lockfile
PackerUpdate --lockfile="/some/other/path.lua"

" Updating a specific plugin with a specific lockfile
PackerUpdate --lockfile="/some/other/path.lua" plenary.nvim

The same interface would be applied to PackerInstall and PackerSync. I still think there needs to be one new command for regenerating the lockfile.

EdenEast commented 1 year ago

@wbthomason and @lewis6991, seeing that there is now a new project called rewrite and there and there is lockfile support there, should this be the tracking issue for that or another one be created?

lewis6991 commented 1 year ago

It's still very early days on the rewrite, though I'm now accepting feedback if you want to test.

I don't mind how you want to track it, but it's not really top priority for me right now.

wbthomason / packer.nvim