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
git: worktree, Fix file reported as Untracked
while it is committed
#1023
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't understand what you proposed. Can you explain it a little more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.Status
and 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
status
empty before making the diff operations. This causes an empty status object when no diff is encountered. Which lead us to getUntracked
status for committed files.I manage to solve this by adding an initialization method for our
status
that iterates over thestorer
index building an initial status with all the unmodified files included. This way, when we usestatus.File
it will find it on the map and will return correctly theUnmodified
status.Fix #119