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

Nil value error due to faulty parsing of blame #1084

Closed
xz47sv opened this issue Jul 12, 2024 · 4 comments · Fixed by #1087 · May be fixed by #1040
Closed

Nil value error due to faulty parsing of blame #1084

xz47sv opened this issue Jul 12, 2024 · 4 comments · Fixed by #1087 · May be fixed by #1040
Labels
bug Something isn't working

Comments

@xz47sv
Copy link

xz47sv commented Jul 12, 2024

Description

Sometimes when opening files the below error shows up:

Error executing luv callback:
...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:130: attempt to index a nil value                                                                                                                                                                                                                                   
stack traceback:                                                                                                                                                                                                                                                                                                                
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:130: in function 'incremental_iter'                                                                                                                                                                                                                         
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:170: in function 'process_incremental'                                                                                                                                                                                                                      
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:228: in function <...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:227>   

Neovim version

NVIM v0.10.0 Build type: MinSizeRel LuaJIT 2.1.1710398010

Operating system and version

Alpine Linux edge

Expected behavior

No response

Actual behavior

This happens inconsistently when opening any file with no uncommited changes when trying to get blame. Happens maybe 50% of the time on my laptop, but on my desktop it took around 50 attempts to get it once. I am pretty sure this is affected by the speed of my machine and git simply not being able to pump out the blame fast enough.

The reason, I believe, is that process_incremental in git/blame.lua does not do any buffering, it will just split data on newlines and pass it to incremental_iter. If it is incomplete and doesn't contain a line starting with filename an error will occur.

As far as I know vim.system makes no guarantees about how much data is sent to the stdout callback so process_incremental should wait until it finds a line starting with "filename" and only process it afterwards or not use the callback and wait for the entire output before parsing.

Minimal config

for name, url in pairs{
  gitsigns = 'https://github.com/lewis6991/gitsigns.nvim',
  -- ADD OTHER PLUGINS _NECESSARY_ TO REPRODUCE THE ISSUE
} do
local install_path = vim.fn.fnamemodify('gitsigns_issue/'..name, ':p')
if vim.fn.isdirectory(install_path) == 0 then
vim.fn.system { 'git', 'clone', '--depth=1', url, install_path }
end
vim.opt.runtimepath:append(install_path)
end

require('gitsigns').setup{
  debug_mode = true, -- You must add this to enable debug messages
  diff_opts = { algorithm = 'minimal', internal = true, linematch = 60 },
  current_line_blame = true,
  numhl = false,
  trouble = true,
}

Steps to reproduce

  1. nvim --clean -u minimal.lua gitsigns_issue/gitsigns/doc/gitsigns.txt
    (the file to open was chosen arbitrarily, it can be any file in any git repo)

As I mentioned, this happens inconsistently and is probably affected by speedslowness of my machine so I wouldn't be surprised if you couldn't reproduce it, but I hope my argument above is sound enough to see the issue.

Gitsigns debug messages

3.22 D dprintf: Deriving GitSignsAdd from Added                                                                                                                                                                                                                                                                        
3.39 D derive: Deriving GitSignsChange from Changed                                                                                                                                                                                                                                                                             
3.54 D derive: Deriving GitSignsDelete from Removed                                                                                                                                                                                                                                                                             
3.64 D derive: Deriving GitSignsChangedelete from GitSignsChange                                                                                                                                                                                                                                                                
3.75 D derive: Deriving GitSignsTopdelete from GitSignsDelete                                                                                                                                                                                                                                                                   
3.80 D derive: Deriving GitSignsUntracked from GitSignsAdd                                                                                                                                                                                                                                                                      
3.90 D derive: Deriving GitSignsAddNr from GitSignsAdd                                                                                                                                                                                                                                                                          
3.98 D derive: Deriving GitSignsChangeNr from GitSignsChange                                                                                                                                                                                                                                                                    
4.09 D derive: Deriving GitSignsDeleteNr from GitSignsDelete                                                                                                                                                                                                                                                                    
4.14 D derive: Deriving GitSignsChangedeleteNr from GitSignsChangeNr                                                                                                                                                                                                                                                            
4.22 D derive: Deriving GitSignsTopdeleteNr from GitSignsDeleteNr                                                                                                                                                                                                                                                               
4.31 D derive: Deriving GitSignsUntrackedNr from GitSignsAddNr                                                                                                                                                                                                                                                                  
4.43 D derive: Deriving GitSignsAddLn from DiffAdd                                                                                                                                                                                                                                                                              
4.53 D derive: Deriving GitSignsChangeLn from DiffChange                                                                                                                                                                                                                                                                        
4.61 D derive: Deriving GitSignsChangedeleteLn from GitSignsChangeLn                                                                                                                                                                                                                                                            
4.69 D derive: Deriving GitSignsUntrackedLn from GitSignsAddLn                                                                                                                                                                                                                                                                  
4.75 D derive: Deriving GitSignsStagedAdd from GitSignsAdd                                                                                                                                                                                                                                                                      
4.82 D derive: Deriving GitSignsStagedChange from GitSignsChange                                                                                                                                                                                                                                                                
4.88 D derive: Deriving GitSignsStagedDelete from GitSignsDelete                                                                                                                                                                                                                                                                
4.93 D derive: Deriving GitSignsStagedChangedelete from GitSignsChangedelete                                                                                                                                                                                                                                                    
4.99 D derive: Deriving GitSignsStagedTopdelete from GitSignsTopdelete                                                                                                                                                                                                                                                          
5.04 D derive: Deriving GitSignsStagedAddNr from GitSignsAddNr                                                                                                                                                                                                                                                                  
5.10 D derive: Deriving GitSignsStagedChangeNr from GitSignsChangeNr                                                                                                                                                                                                                                                            
5.17 D derive: Deriving GitSignsStagedDeleteNr from GitSignsDeleteNr                                                                                                                                                                                                                                                            
5.29 D derive: Deriving GitSignsStagedChangedeleteNr from GitSignsChangedeleteNr                                                                                                                                                                                                                                                
5.47 D derive: Deriving GitSignsStagedTopdeleteNr from GitSignsTopdeleteNr                                                                                                                                                                                                                                                      
5.52 D derive: Deriving GitSignsStagedAddLn from GitSignsAddLn                                                                                                                                                                                                                                                                  
5.57 D derive: Deriving GitSignsStagedChangeLn from GitSignsChangeLn                                                                                                                                                                                                                                                            
5.63 D derive: Could not derive GitSignsStagedDeleteLn                                                                                                                                                                                                                                                                          
5.67 D derive: Deriving GitSignsStagedChangedeleteLn from GitSignsChangedeleteLn                                                                                                                                                                                                                                                
5.73 D derive: Could not derive GitSignsStagedTopdeleteLn                                                                                                                                                                                                                                                                       
5.79 D derive: Deriving GitSignsAddPreview from DiffAdd                                                                                                                                                                                                                                                                         
5.86 D derive: Deriving GitSignsDeletePreview from DiffDelete                                                                                                                                                                                                                                                                   
5.95 D derive: Deriving GitSignsCurrentLineBlame from NonText                                                                                                                                                                                                                                                                   
6.07 D derive: Deriving GitSignsAddInline from TermCursor                                                                                                                                                                                                                                                                       
6.13 D derive: Deriving GitSignsDeleteInline from TermCursor                                                                                                                                                                                                                                                                    
6.18 D derive: Deriving GitSignsChangeInline from TermCursor                                                                                                                                                                                                                                                                    
6.25 D derive: Deriving GitSignsAddLnInline from GitSignsAddInline                                                                                                                                                                                                                                                              
6.31 D derive: Deriving GitSignsChangeLnInline from GitSignsChangeInline                                                                                                                                                                                                                                                        
6.37 D derive: Deriving GitSignsDeleteLnInline from GitSignsDeleteInline                                                                                                                                                                                                                                                        
6.45 D derive: Deriving GitSignsDeleteVirtLn from DiffDelete                                                                                                                                                                                                                                                                    
6.51 D derive: Deriving GitSignsDeleteVirtLnInLine from GitSignsDeleteLnInline                                                                                                                                                                                                                                                  
6.57 D derive: Deriving GitSignsVirtLnum from GitSignsDeleteVirtLn                                                                                                                                                                                                                                                              
25.93 D attach(1): Attaching (trigger=BufReadPost)                                                                                                                                                                                                                                                                              
26.35 D run_job: git --version                                                                                                                                                                                                                                                                                                  
61.47 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD                                                                                                                                                                             
64.90 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 --git-dir /tmp/gitsigns/gitsigns_issue/gitsigns/.git config user.name                                                                                                                                                                      
69.13 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 --git-dir /tmp/gitsigns/gitsigns_issue/gitsigns/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --eol /tmp/gitsigns/gitsigns_issue/gitsigns/doc/gitsigns.txt                                                       
74.32 D watch_gitdir(1): Watching git dir                                                                                                                                                                                                                                                                                       
74.46 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 --git-dir /tmp/gitsigns/gitsigns_issue/gitsigns/.git show bda1a13b1a7efabee38f9d42fb8a5c6981aa2fb4                                                                                                                                         
100.93 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 --git-dir /tmp/gitsigns/gitsigns_issue/gitsigns/.git show HEAD:doc/gitsigns.txt                                                                                                                                                           
1107.30 D run_job: git --no-pager --no-optional-locks --literal-pathspecs -c gc.auto=0 --git-dir /tmp/gitsigns/gitsigns_issue/gitsigns/.git blame --contents - --incremental -- /tmp/gitsigns/gitsigns_issue/gitsigns/doc/gitsigns.txt                                                                                          
1122.14 D incremental_iter: Unknown tag: ''                                                                                                                                                                                                                                                                                     
15805.24 D cli.run: Running action 'debug_messages' with arguments {}

Gitsigns cache

No response

@xz47sv xz47sv added the bug Something isn't working label Jul 12, 2024
@lewis6991
Copy link
Owner

The reason, I believe, is that process_incremental in git/blame.lua does not do any buffering, it will just split data on newlines and pass it to incremental_iter. If it is incomplete and doesn't contain a line starting with filename an error will occur.

I'm fairly sure (but not 100%) that the output from git-blame will always end in a newline (this can be tested by adding an assert), thus we don't need to buffer for this. But yes, that doesn't mean the full blame entry will be present in the output of each callback.

@xz47sv
Copy link
Author

xz47sv commented Jul 12, 2024

The reason, I believe, is that process_incremental in git/blame.lua does not do any buffering, it will just split data on newlines and pass it to incremental_iter. If it is incomplete and doesn't contain a line starting with filename an error will occur.

I'm fairly sure (but not 100%) that the output from git-blame will always end in a newline (this can be tested by adding an assert), thus we don't need to buffer for this. But yes, that doesn't mean the full blame entry will be present in the output of each callback.

If I add print(vim.inspect(data_lines)) on top of incremental_iter I can get this output in neovim:

{ "7178d1a430dcfff8a4c92d78b9e39e0297a779c0 1 1 1216", "" }
Error executing luv callback:                                                                                                                                                                                                                                                                                                   
...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:131: attempt to index a nil value                                                                                                                                                                                                                                   
stack traceback:                                                                                                                                                                                                                                                                                                                
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:131: in function 'incremental_iter'                                                                                                                                                                                                                         
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:171: in function 'process_incremental'                                                                                                                                                                                                                      
        ...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:229: in function <...igns/gitsigns_issue/gitsigns//lua/gitsigns/git/blame.lua:228>                                                                                                                                                                          
Press ENTER or type command to continue                                                                                                                                                                                                                                                                                         
{ "author Lewis Russell", "author-mail <lewis6991@gmail.com>", "author-time 1720692260", "author-tz +0100", "committer Lewis Russell", "committer-mail <lewis6991@gmail.com>", "committer-time 1720692260", "committer-tz +0100", "summary fix: add nil check", "boundary", "filename doc/gitsigns.txt", "" } 

So yes, it does end on a newline, just not the right one.

@lewis6991
Copy link
Owner

Can you test #1087?

@xz47sv
Copy link
Author

xz47sv commented Jul 12, 2024

Can you test #1087?

works like charm, cannot reproduce the issue anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants