Gitsigns often fail to retrieve hunk information when staged signs is enabled #924

wookayin · 2023-12-10T06:43:04Z

Description

I know _signs_staged_enable is still an experimental feature, but I find this really a great and useful feature. I've run into some bugs where gitsigns stop working -- gitsigns fail to display any signs (changes on the working copy and/or the git index) sporadically.

It's not very reproducible but happens quite frequently, once in a while. (Like every hour or so in my normal use) It appears to have some race condition or asynchronous operations when multiple buffers are involved.

I tried to make a minimal reproducible example but the bug was really hard to reproduce. Please excuse me not providing a fully reproducible repo, but I can share what I've found so far with some debugging efforts.

The relevant log message I see is: "compare_text was invalid, aborting" on M.buf_check().

Neovim version

both on nvim 0.9.4 and nightly 0.10.0-dev-1800+gc651fb304-dirty

Operating system and version

macOS

Expected behavior

Gitsigns are displayed at all times and is consistent with the actual git status, even if staged signs are enabled.

Actual behavior

Example: No gitsigns are displayed on a "broken" buffer.

Minimal config

I will skip at this point because it's really difficult to make an independent minimal repro. But will add later if I can come up with a reliable minimal repro. Below I put my current config instead.

  gitsigns.setup {
    signcolumn = true,
    _signs_staged_enable = true, -- experimental

    signs = {
      add          = { hl = 'GitSignsAdd'   , text = '┃', numhl = 'GitSignsAddNr'   , linehl = 'GitSignsAddLn' },
      change       = { hl = 'GitSignsChange', text = '┃', numhl = 'GitSignsChangeNr', linehl = 'GitSignsChangeLn' },
      delete       = { hl = 'GitSignsDelete', text = '_', numhl = 'GitSignsDeleteNr', linehl = 'GitSignsDeleteLn' },
      topdelete    = { hl = 'GitSignsDelete', text = '‾', numhl = 'GitSignsDeleteNr', linehl = 'GitSignsDeleteLn' },
      changedelete = { hl = 'GitSignsChange', text = '┃', numhl = 'GitSignsChangeNr', linehl = 'GitSignsChangeLn' },
    },
    _signs_staged = {
      add          = { hl = 'GitSignsStagedAdd'   , text = '┋ ', numhl = 'GitSignsStagedAddNr'   , linehl = 'GitSignsStagedAddLn' },
      change       = { hl = 'GitSignsStagedChange', text = '┋ ', numhl = 'GitSignsStagedChangeNr', linehl = 'GitSignsStagedChangeLn' },
      delete       = { hl = 'GitSignsStagedDelete', text = '﹍', numhl = 'GitSignsStagedDeleteNr', linehl = 'GitSignsStagedDeleteLn' },
      topdelete    = { hl = 'GitSignsStagedDelete', text = '﹉', numhl = 'GitSignsStagedDeleteNr', linehl = 'GitSignsStagedDeleteLn' },
      changedelete = { hl = 'GitSignsStagedChange', text = '┋ ', numhl = 'GitSignsStagedChangeNr', linehl = 'GitSignsStagedChangeLn' },
    },

    diff_opts = {
      internal = true,
      algorithm = 'patience',
      indent_heuristic = true,
      linematch = 60,
      ignore_whitespace_change = false,
      ignore_blank_lines = false,
      ignore_whitespace = false,
      ignore_whitespace_change_at_eol = false,
    },

    debug_mode = true,
  }

Steps to reproduce

I will skip at this point because it's really difficult to make an independent minimal repro. But will add later if I can come up with a reliable one.

Gitsigns debug messages

Buffers:

 [1]   nvim/colors/xoria256-wook.vim
 [12]  nvim/lua/config/plugins.lua
 [14]  nvim/lua/config/git.lua

[1], [12] are working fine, but [14] is the buffer where gitsigns is broken.

 attach(12): Attaching (trigger=BufReadPost)
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 config user.name
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show :0:nvim/colors/xoria256-wook.vim
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --eol /Users/wookayin/.dotfiles/nvim/lua/config/plugins.lua
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show HEAD:nvim/colors/xoria256-wook.vim
 watch_gitdir(12): Watching git dir
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show :0:nvim/lua/config/plugins.lua
 update(1): updates: 2, jobs: 13
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show HEAD:nvim/lua/config/plugins.lua
 update(12): updates: 3, jobs: 14
 update(12): updates: 4, jobs: 14
 watcher_cb(1): Git dir update: 'index.lock' { rename = true } (ignoring)
 watcher_cb(12): Git dir update: 'index.lock' { rename = true } (ignoring)
 watcher_cb(1): Git dir update: 'index.lock' { rename = true } (ignoring)
 watcher_cb(12): Git dir update: 'index.lock' { rename = true } (ignoring)
 ...
-attach(14): Attaching (trigger=BufReadPost)
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 config user.name
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show :0:nvim/colors/xoria256-wook.vim
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --eol /Users/wookayin/.dotfiles/nvim/lua/config/git.lua
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show HEAD:nvim/colors/xoria256-wook.vim
-watch_gitdir(14): Watching git dir
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show :0:nvim/lua/config/git.lua
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show HEAD:nvim/lua/config/git.lua
 update(1): updates: 5, jobs: 21
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show :0:nvim/lua/config/plugins.lua
-(14): compare_text was invalid, aborting
 run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/wookayin/.dotfiles/.git show HEAD:nvim/lua/config/plugins.lua
 update(12): updates: 6, jobs: 23
 cli.run: Running action 'next_hunk' with arguments {}
 ....

I can see the line

(14): compare_text was invalid, aborting

for the buffer where gitsigns has failed. This line was added in fef5d90 (relevant to #873)

At this point, cache[bufnr] has:

{
  bufnr = 14,
  compare_text_head = { ... } -- some length text
  file = "/Users/wookayin/.dotfiles/nvim/lua/config/git.lua",
  git_obj = {
    encoding = "utf-8",
    file = "/Users/wookayin/.dotfiles/nvim/lua/config/git.lua",
    i_crlf = false,
    mode_bits = "100644",
    object_name = "90cd9a6409cb6f4f26221aabae5481c83a9c8526",
    relpath = "nvim/lua/config/git.lua",
    repo = {
      abbrev_head = "master",
      detached = false,
      gitdir = "/Users/wookayin/.dotfiles/.git",
      toplevel = "/Users/wookayin/.dotfiles",
      username = "Jongwook Choi",
      <metatable> = {
        __index = {
          command = <function 1>,
          files_changed = <function 2>,
          get_show_text = <function 3>,
          new = <function 4>,
          update_abbrev_head = <function 5>
        }
      }
    },
    w_crlf = false,
    <metatable> = {
      __index = {
        command = <function 6>,
        file_info = <function 7>,
        get_show_text = <function 8>,
        has_moved = <function 9>,
        new = <function 10>,
        run_blame = <function 11>,
        stage_hunks = <function 12>,
        stage_lines = <function 13>,
        unstage_file = <function 14>,
        update_file_info = <function 15>
      }
    }
  },
  gitdir_watcher = <userdata 1>,
  staged_diffs = {},
  <metatable> = {
    __index = {
      destroy = <function 16>,
      get_blame = <function 17>,
      get_compare_rev = <function 18>,
      get_rev_bufname = <function 19>,
      invalidate = <function 20>,
      run_blame = <function 21>,
      wait_for_hunks = <function 22>
    }
  }
}

So indeed there is no compare_text entry. But there is compare_text_head. So I suppose the logic introduced in #873 wasn't aware of "staged sign" (compare_text_head), and this is the only helpful information and context I could provide you with at the moment.

Execution context:

  if config._signs_staged_enable and not file_mode then
    if not bcache.compare_text_head or config._refresh_staged_on_update then
      local staged_compare_rev = bcache.commit and string.format('%s^', bcache.commit) or 'HEAD'
      bcache.compare_text_head = git_obj:get_show_text(staged_compare_rev)
      M.buf_check(bufnr, true) -- <==================== probably here. and the remaining lines won't be executed.
    end
    local hunks_head = run_diff(bcache.compare_text_head, buftext)
    M.buf_check(bufnr)
    bcache.hunks_staged = gs_hunks.filter_common(hunks_head, bcache.hunks)
  end

A possible fix in M.buf_check(...):

-      if check_compare_text and not cache[bufnr].compare_text then
+      if check_compare_text and not cache[bufnr].compare_text and not cache[bufnr].compare_text_head then
         dprint('compare_text was invalid, aborting')
         return
       end

The text was updated successfully, but these errors were encountered:

wookayin · 2023-12-10T10:40:39Z

There is also a case where both of cache[bufnr].compare_text and cache[bufnr].compare_text_head were nil. So it "aborted" and no signs were updating since then on that buffer. Until reattaching to the (new) buffer via ":bwipeout" and re-opening the file. I'll need to figure out why gitsigns doesn't reattach to the buffer again when it was aborted due to cache invalidation.

lewis6991 · 2023-12-10T11:27:07Z

Without a good repro I can't really promise anything, I just haven't got the time. However it looks like you're on the right track to debugging this.

Basically whenever we need to call an async function, anything could have happened in the buffer and this is where things go wrong. buf_check() was added to prevent doing anything that can cause an error but as you've found it can just silently give up on things.

Let me know if you need help in understanding any of the code better.

Problem: If `buf_check(...)` fails (mostly when cache is outdated), it would abort updating gitsigns, but this actually leads to the coroutine never resolving and stuck forever. It is because the asynchronous callback `cb()` is never called. As a result, gitsigns might be stuck forever and fail to update silently (see lewis6991#924), and manual reattach or refresh also doesn't work, because of the unresolved coroutine that is recognized to be "still running", i.e., any subsequent calls to "throttled" `manager.update()` will not be executed. Solution: Make `buf_check()` always yield a value (true/false) to ensure that `manager.update()` will resolve and finish its execution after all.

wookayin · 2023-12-10T14:37:04Z

Thanks for your comments. While debugging, I found that manual reattaching or gitsigns.refresh() did not execute manager.update(). This was a hint that the coroutine was stuck and never resolving. #925 is a possible fix for this (at least gitsigns can work afterwards when it's trying to update hunks again).

I think the logics around compare_text and compare_text_head might still need to be fixed, maybe the "root" cause that makes buf_check() fails is on somewhere else (which doesn't seem to happen when staged_signs is not used?). But I wasn't so sure about the logic here so I'll leave that part to you.

Problem: If `buf_check(...)` fails (mostly when cache is outdated), it would abort updating gitsigns, but this actually leads to the coroutine never resolving and stuck forever. It is because the asynchronous callback `cb()` is never called. As a result, gitsigns might be stuck forever and fail to update silently (see #924), and manual reattach or refresh also doesn't work, because of the unresolved coroutine that is recognized to be "still running", i.e., any subsequent calls to "throttled" `manager.update()` will not be executed. Solution: Make `buf_check()` always yield a value (true/false) to ensure that `manager.update()` will resolve and finish its execution after all.

wookayin · 2024-01-30T04:55:32Z

the "root" cause that makes buf_check() fails

This can often happen for some reason but once invalidated, gitsigns will (asynchronously) try to fix everything to work very soon, so this shouldn't be a problem.

Since the fix #925, I've never seen gitsigns stop working and fail to update sign. Closing as fixed.

wookayin added the bug Something isn't working label Dec 10, 2023

wookayin changed the title ~~Gitsigns are not showing anything when staged signs is enabled~~ Gitsigns often fail to retrieve hunk information when staged signs is enabled Dec 10, 2023

wookayin mentioned this issue Dec 10, 2023

fix(manager): manager.update() never resolve when buf_check() fails #925

Merged

wookayin closed this as completed Jan 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gitsigns often fail to retrieve hunk information when staged signs is enabled #924

Gitsigns often fail to retrieve hunk information when staged signs is enabled #924

wookayin commented Dec 10, 2023 •

edited

Loading

wookayin commented Dec 10, 2023 •

edited

Loading

lewis6991 commented Dec 10, 2023

wookayin commented Dec 10, 2023 •

edited

Loading

wookayin commented Jan 30, 2024

Gitsigns often fail to retrieve hunk information when staged signs is enabled #924

Gitsigns often fail to retrieve hunk information when staged signs is enabled #924

Comments

wookayin commented Dec 10, 2023 • edited Loading

Description

Neovim version

Operating system and version

Expected behavior

Actual behavior

Minimal config

Steps to reproduce

Gitsigns debug messages

wookayin commented Dec 10, 2023 • edited Loading

lewis6991 commented Dec 10, 2023

wookayin commented Dec 10, 2023 • edited Loading

wookayin commented Jan 30, 2024

wookayin commented Dec 10, 2023 •

edited

Loading

wookayin commented Dec 10, 2023 •

edited

Loading

wookayin commented Dec 10, 2023 •

edited

Loading