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.72k stars 262 forks source link

Sync fails when upstream force pushes Git tags #1100

Closed antoineco closed 10 months ago

antoineco commented 1 year ago

Some projects update certain Git tags by force pushing them on a regular basis. Typical example: a nighly tag may change every day.

When this occurs anytime after packer has already been synced at least once, subsequent updates (sync) fail consistently with the message [rejected] ... would clobber existing tag. (see details below)

Steps to reproduce

  1. return require'packer'.startup(function(use)
      use 'kyazdani42/nvim-tree.lua'
    end)
  2. :PackerSync
  3. Force push any existing tag / wait until upstream does.

  4. :PackerSync

Actual behaviour

 ✗ Failed to update kyazdani42/nvim-tree.lua
  Errors:
    remote: Enumerating objects: 455, done.
...
    Resolving deltas: 100% (95/95), completed with 29 local objects.
    From https://github.com/kyazdani42/nvim-tree.lua
       875d38e..2b97063  master                   -> origin/*
     ! [rejected]        nightly                  -> *  (would clobber existing tag)
    875d38e

Expected behaviour

 ✓ Updated kyazdani42/nvim-tree.lua

Additional notes

Fetching from Git with the --force flag is the easiest solution that comes to mind: https://stackoverflow.com/q/58031165/4716370

My current workaround is to add this flag to config.git.subcommands.update while calling init():

require'packer'.init{
  git = {
    subcommands = {
      update = 'pull --ff-only --progress --rebase=false --force'
    }
  }
}

packer files

Not relevant or already provided above.

overhacked commented 1 year ago

My current workaround is to add this flag to config.git.subcommands.update while calling init():

require'packer'.init{
  git = {
    subcommands = {
      update = 'pull --ff-only --progress --rebase=false --force'
    }
  }
}

Turns out this isn't quite enough if plugin.commit or plugin.tag is specified, because of this condition: https://github.com/wbthomason/packer.nvim/blob/64ae65fea395d8dc461e3884688f340dd43950ba/lua/packer/plugin_types/git.lua#L221-L223

The workaround for me was:

require'packer'.init{
    git = {
      subcommands = {
        -- Fix for https://github.com/wbthomason/packer.nvim/issues/1100,
        -- which blocks updating the nvim-tree nightly tag
        update = 'pull --ff-only --progress --rebase=false --force',
        fetch = 'fetch --depth 999999 --progress --force',
      }
    },
}
antoineco commented 1 year ago

@overhacked good point, sorry if I didn't understand your comment on the linked PR.

dave-kennedy commented 1 year ago

Setting --force on all plugins might be excessive. Consider instead setting the --no-tags option for just the nvim-tree repository, e.g.:

$ cd ~/.local/share/nvim/site/pack/packer/start/nvim-tree.lua # or wherever it's installed on your machine
$ git config --add remote.origin.fetch '^refs/tags/*'
$ git config remote.origin.tagOpt --no-tags

I'm not sure why the negative refspec (^refs/tags/*) is required. Seems like --no-tags should be sufficient by itself. It isn't.

overhacked commented 1 year ago

I think you're right that --force is overkill and (maybe??) destructive if someone is hacking on a plugin by editing and committing inside of the nvim pack directory. I read through the git fetch man page, and it seems that fetch behavior for tags changed in Git version 2.20:

Until Git version 2.20, and unlike when pushing with git-push(1), any updates to refs/tags/* would be
accepted without + in the refspec (or --force). When fetching, we promiscuously considered all tag
updates from a remote to be forced fetches. Since Git version 2.20, fetching to update refs/tags/*
works the same way as when pushing. I.e. any updates will be rejected without + in the refspec (or
--force).

Unlike when pushing with git-push(1), any updates outside of refs/{tags,heads}/* will be accepted
without + in the refspec (or --force), whether that’s swapping e.g. a tree object for a blob, or a
commit for another commit that’s doesn’t have the previous commit as an ancestor etc.

This looks to me like packer should add a + in front of the refspec if the plugin use spec includes tag or branch. I'll open a PR for discussion.

antoineco commented 1 year ago

@overhacked please note that the issue isn't only occurring when a branch or tag is specified (I have no such thing in my config). It occurs any time upstream force pushes a tag, even one that's not used in the Packer config.

Regarding the suggestion to manually edit the Git config for the problematic repo, I don't think this really solves anything considering the main advantage of Packer's is to have a declarative configuration.

I didn't know about the + form of the refspec. It sounds reasonable if done right.

overhacked commented 11 months ago

I've done some more experimenting with --force and come to understand that it has zero impact on local branch development. The --force option when used with pull or fetch only impacts:

  1. The configured remote.<name>.fetch refspec(s) (usually default: +refs/heads/*:refs/remotes/origin/*)
  2. Tags fetched with refs/tags/*:refs/tags/*
  3. Any explicit refspecs on the command line (not used by Packer)

Because the only local refs affected by (1) and (2) are refs/tags/* and refs/remotes/origin/*, there's no possibility of the local branch heads in refs/heads/* being clobbered. refs/remotes/... is by definition coming from the remote and refs/tags/... would only be an issue if you were running all over a local repo tagging commits with names that don't yet exist on origin but might someday. If you try to use a tag name that already has been fetched, you'd have to git tag -f locally.

tl; dr:fetch --force and pull --force have...

I argue that Packer should change config_defaults.git.subcommands.update and ...fetch to include --force.