packer.nvim icon indicating copy to clipboard operation
packer.nvim copied to clipboard

Support git revert

Open HawkinsT opened this issue 2 years ago • 9 comments

Describe the feature

It would be useful if packer supported reverting specific git commits.

For example, if I wish to use nvim-lspconfig without commit 7a73b88, I can load the package like this in my config:

use {
    "neovim/nvim-lspconfig",
    run = "git fetch --unshallow && git revert 7a73b88"
}

This works on a fresh installation of the plugin; the latest version of nvim-lspconfig is downloaded and then the specific commit is reversed. Unfortunately, after initial installation, trying to update the package through packer will fail with error 'fatal: Not possible to fast-forward, aborting' due to the difference in the local repo from the one on github.

The simplest method to enable git reverts would be to add a second run option (e.g. 'prerun') that runs before a package is updated (which would allow for git fetch --unshallow to stop the update from failing) and provide additional functionality for other use-cases.

HawkinsT avatar Jul 24 '22 20:07 HawkinsT

Maybe he should make coffee too? :-)

ur4ltz avatar Aug 25 '22 19:08 ur4ltz

The better solution is to maintain your own fork of the plugin and do your git operations there. Packer is not the place for arbitrary git scripting.

nat-418 avatar Oct 05 '22 23:10 nat-418

As a package manager, packer seems to me to be the appropriate place for me to manage packages and is far less messy and time consuming than forking an old version of a repo every time a bug is introduced in a new update.

This is why I recommend a prerun option, however (instead of explicitly implementing git reverts); it's a general solution applicable to many use cases.

HawkinsT avatar Oct 06 '22 12:10 HawkinsT

I have never used a package manager that supports making arbitrary modifications to packages on-the-fly. You cannot apt install foo --pre-install="git revert xyz". That doesn't make any sense. Your issue is with your plugin maintainer upstream, not with Packer. You should close this issue.

nat-418 avatar Oct 06 '22 13:10 nat-418

So because apt lacks this feature it shouldn't be suggested for another package manager? Apt is generally accessed via the command line, packer isn't. If you want a better example, nix can accomplish this just fine - portage too. I will also restate that I'm not suggesting packer support every git command etc. I'm just saying it should allow the user to run any command they specify before the package is update.

HawkinsT avatar Oct 06 '22 13:10 HawkinsT

So looking at ./lua/packer/plugin_utils.lua briefly it seems possible to rework the plugin_utils.post_update_hook function to become a pre_update_hook function by reordering some of the logic. Look at the first few lines:

plugin_utils.post_update_hook = function(plugin, disp)
  local plugin_name = util.get_plugin_full_name(plugin)
  return a.sync(function()
    if plugin.run or not plugin.opt then
      await(a.main)
      plugin_utils.load_plugin(plugin)
    end

    if plugin.run then
      if type(plugin.run) ~= 'table' then
        plugin.run = { plugin.run }
      end
      disp:task_update(plugin_name, 'running post update hooks...')
      local hook_result = result.ok()
      for _, task in ipairs(plugin.run) do

That await and load_plugin(plugin) call could probably be moved if you took the time to make a PR and test it etc. I appreciate both Nix and Portage but I don't really think Packer is trying to reinvent those wheels. If you can use Nix, why use Packer at all? Maybe I am wrong, good luck!

nat-418 avatar Oct 06 '22 14:10 nat-418

Well, this feature is really needed? I think rock revison number or fork is better.

Shougo avatar Oct 07 '22 02:10 Shougo

Thanks @nat-418 .

To me, this seems to be the best solution; it's self-contained, and it's not like it would be adding a lot of bloat to packer. If you believe the fact that packer already has a post update hook isn't a bad thing, then this is simply an extension of that functionality, relevant in more nuanced use cases. To me, having more control over your packages is a selling point to using packer over any of the numerous other neovim package managers.

Of course, everyone can have their own opinion, which is why we're having this discussion now.

HawkinsT avatar Oct 08 '22 10:10 HawkinsT

@HawkinsT I won't do the work to implement this feature, but if you submit a PR and it gets merged, great.

nat-418 avatar Oct 08 '22 11:10 nat-418