Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load and use snapshots as part of init for installation without 'startup' #849

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smhc
Copy link
Contributor

@smhc smhc commented Mar 4, 2022

Not a complete fix but it attempts to better describe the problem of needing to use a snapshot at install/sync time for configurations that are not calling 'packer.startup'. (e.g config similar to https://github.com/wbthomason/dotfiles/blob/linux/neovim/.config/nvim/lua/plugins.lua and https://github.com/wbthomason/dotfiles/blob/linux/neovim/.config/nvim/init.lua#L100 )

@smhc
Copy link
Contributor Author

smhc commented Mar 9, 2022

I've reworked this in the follow up commit above and it seems to work ok, I've been using it for some time.

"PackerInstall" will honour the commits from the snapshot file, which I believe should be the behavior.

It's designed such that "PackerUpdate" will ignore the snapshot commit values and perform an update, but if the commit is explicitly defined (in the 'use' block) it will honour it and not update it. PackerSync will also do a 'PackerUpdate' and ignore the snapshot file as per defined behaviour, but imho this is incorrect.

The other benefit of this approach (as opposed to doing a revert at startup) is PackerInstall should eventually be enhanced to do a minimal clone that includes just the required commit, instead of the full depth clone it currently is doing. This is only possible by determining the commits from the snapshot file prior to installation - which this PR does.

@smhc
Copy link
Contributor Author

smhc commented Apr 20, 2022

@wbthomason - can you confirm whether something similar to this enhancement will be possible in the v2 refactoring?

I think a common use case for snapshots is the ability to "take a snapshot" and clean install plugins some time in the future or on a different machine using the snapshot. i.e the config, setup, install hooks etc may all be tied to a specific commit of a plugin and it is important this exact commit is used at "PackerInstall" time when using snapshots.

I believe the only way to do it currently is to do a 'PackerInstall' which ignores the snapshot and attempts to install all plugins at HEAD (which may fail if the plugins are incompatible etc at head) then a 'PackerSnapShotRollback' to actually use the snapshot file after the fact. imho this is undesirable.

This PR attempts to resolve this issue.

@desaster
Copy link

desaster commented Apr 21, 2022

I'm trying to create a neovim configuration with locked package versions, and I found this PR as it seems to bring the snapshot feature closer to how I think it should work.

In my setup I'm placing the snapshot file in the configuration directory, since I want to include it in version control. I think this PR doesn't work quite in such setup, since it displays an error message during startup (Couldn't read snapshot file packer-lock.json), and packer ends up installing the wrong versions of packages.

I've setup an experimental configuration here: https://github.com/desaster/neovim-locked-example

I don't know if this is the preferred method, but seems cleaner than having startup() run some snapshot revert on startup.

This includes a simple patch like so:

--- ./data/nvim/site/pack/packer/start/packer.nvim/lua/packer.lua	2022-04-21 13:09:43.087827106 +0300
+++ fixed-packer.lua	2022-04-21 13:22:55.933711145 +0300
@@ -132,9 +132,10 @@
     vim.notify("Couldn't create " .. config.snapshot_path, vim.log.levels.WARN)
   end
 
-  if config.snapshot then
+  local snapshot_file = join_paths(config.snapshot_path, config.snapshot)
+  if snapshot_file then
     local status, content = pcall(function()
-      return vim.fn.readfile(config.snapshot)
+      return vim.fn.readfile(snapshot_file)
     end)
     if (status and content ~= nil) then
       config.snapshot_commits = vim.fn.json_decode(content)

@smhc
Copy link
Contributor Author

smhc commented Apr 21, 2022

I believe the existing PR should work ok without change, you just need to specify the full path for the 'snapshot' config. e.g

    local snappath = vim.fn.stdpath('config')..'/lua/conf'
    local snapfile = 'packer_snap.json'
    packer.init {
        snapshot_path = snappath,
        snapshot = snappath..'/'..snapfile
    }

In any case - I agree this is surely how the snapshot system should work.

I also think PackerSync should synchronise the plugin checkouts with the snapshot file and not do any updates. However it ignores the snapshot file and does updates by design - I didn't change this in this PR but I do think the sync behaviour is also wrong.

@smhc smhc mentioned this pull request Sep 4, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants