wbthomason / packer.nvim

A use-package inspired plugin manager for Neovim. Uses native packages, supports Luarocks dependencies, written in Lua, allows for expressive config
MIT License
7.9k stars 264 forks source link

Support git revert #981

Open HawkinsT opened 2 years ago

HawkinsT commented 2 years ago

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.

ur4ltz commented 2 years ago

Maybe he should make coffee too? :-)

nat-418 commented 2 years ago

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.

HawkinsT commented 2 years ago

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.

nat-418 commented 2 years ago

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.

HawkinsT commented 2 years ago

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.

nat-418 commented 2 years ago

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!

Shougo commented 2 years ago

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

HawkinsT commented 2 years ago

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.

nat-418 commented 2 years ago

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