git: worktree, Fix file reported as Untracked while it is committed
#1023
Conversation
…lding it empty. Fixes go-git#119
| for len(nodes) > 0 { | ||
| var node noder.Noder | ||
| node, nodes = nodes[0], nodes[1:] | ||
| if node.IsDir() { | ||
| children, err := node.Children() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| nodes = append(nodes, children...) | ||
| continue | ||
| } | ||
| fs := status.File(node.Name()) | ||
| fs.Worktree = Unmodified | ||
| fs.Staging = Unmodified | ||
| } |
There was a problem hiding this comment.
This may have a performance impact, as we need to go through all nodes while growing nodes as part of that process.
What if instead, we amended File(path string) to validate the returning s[path] and force a default value of Unmodified if no value was currently set?
There was a problem hiding this comment.
Sorry, I didn't understand what you proposed. Can you explain it a little more?
There was a problem hiding this comment.
The issue here is that going through the entire worktree to create an initial state as unmodified can be quite costly - specially on large repositories. I tested this on a mid-sized repository and noticed that with these changes the .Status() call took ~30% longer than before.
My initial suggestion was to amend Status so that the File func would effectively do the same thing as the pre-init you created. However, this won't really work in some cases, as when there are no changes the Status returns an empty list. Besides, the existing logic is incorrect as whatever file we pass on to status.File(path) it returns Untracked even if that file does not exist across both the staging and the worktree.
In order to fix this, while keeping backwards compatibility and not introducing a performance regression, I think we have a few options:
- Introduce a new
Worktree.StatusWithOptions()func which returns a new type that is more flexible and calculates the status of a given file on demand. I suspect this will need to have access to the worktree fs, so that it can handle the scenario I mentioned above, without needing to front-load the entire worktree. It may also need access to the exclusion rules. - Introduce a new
Worktree.StatusWithOptions()func which returns a new type that is more flexible, but simply adds an option so users can opt-in to the front-loading this PR currently introduces. - Introduce a new
Worktree.FileStatus()func which calls.Statusand ensure the given file is correctly loaded to the Status map.
What are your thoughts on the above?
Overview
When using the lib I have also faced this problem #119. After looking into the code my understanding is that we were initializing the
statusempty before making the diff operations. This causes an empty status object when no diff is encountered. Which lead us to getUntrackedstatus for committed files.I manage to solve this by adding an initialization method for our
statusthat iterates over thestorerindex building an initial status with all the unmodified files included. This way, when we usestatus.Fileit will find it on the map and will return correctly theUnmodifiedstatus.Fix #119