git: worktree, Don't delete local untracked files when resetting worktree by k-anshul · Pull Request #1540 · go-git/go-git

k-anshul · 2025-05-02T09:25:33Z

Fixes #53

…tree. Fixes go-git#53

Copilot

Pull Request Overview

This PR refines the worktree reset functionality so that local untracked files are preserved when resetting the worktree, addressing issue #53.

Updates the test in worktree_test.go to verify that untracked files (e.g. "foo") remain unaltered.
Modifies the ResetSparsely, resetIndex, and resetWorktree functions in worktree.go to track removed files and adjust the file checkout logic based on untracked status.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
worktree_test.go	Replaces a simple IsClean assertion with a loop to validate specific file statuses.
worktree.go	Returns removed file information from resetIndex and uses it in resetWorktree to decide which files to check out.

worktree_test.go

worktree.go

…tree. Fixes go-git#53

onee-only · 2025-05-02T13:19:24Z

worktree.go

+		// only checkout an untracked file if it is in the list of files
+		// a reset should leave untracked files alone
+		file := nameFromAction(&ch)
+		if status.File(file).Worktree != Untracked || inFiles(files, file) {


Can we use ch instead of status? I think something like

action, err := ch.Action() if err != nil { return err } if action != merkletrie.Insert { // checkout the file }

Would work since "Untracked" would mean that the file appears in worktree but not in staging.

For reference, Status() also works this way internally.

go-git/worktree_status.go

Lines 93 to 118 in 93ed29d

right, err := w.diffStagingWithWorktree(false, true)

if err != nil {

return nil, err

}

for _, ch := range right {

a, err := ch.Action()

if err != nil {

return nil, err

}

fs := s.File(nameFromAction(&ch))

if fs.Staging == Untracked {

fs.Staging = Unmodified

}

switch a {

case merkletrie.Delete:

fs.Worktree = Deleted

case merkletrie.Insert:

fs.Worktree = Untracked

fs.Staging = Untracked

case merkletrie.Modify:

fs.Worktree = Modified

}

}

This way we can avoid re-doing computations that we already have done.

Hey @onee-only

Thanks for the reviews. I just realised that I don't need the check at all and I can directly do checkout.
The files being passed is always going to just include the files which need changes and no other files.

onee-only · 2025-05-02T13:33:06Z

worktree.go

+		// only checkout an untracked file if it is in the list of files
+		// a reset should leave untracked files alone
+		file := nameFromAction(&ch)
+		if status.File(file).Worktree != Untracked || inFiles(files, file) {


~~Is there any specific reason that we check inFiles here? I think it is already done in block above.~~
I wasn't paying attention to the comment. Sorry for the confusion.

Hey

You were right actually. I just removed the complete condition.

UnseenBook · 2025-05-07T08:40:02Z

I have been looking for a solution to the same problem. I have tried these changes and they work in most cases, but when checking out to a branch with the same commit, so no files change, it still deletes all untracked files (including files that are in the gitignore).
From brief manual testing I think it's because when no tracked files change the new list of removedFiles is empty, so it is not checked. If I understand correctly, the list of files passed to resetWorktree are supposed to be the only files that need to be reset in the worktree. (I have been trying to piece together the logic behind all this and find it very difficult to figure out why certain things are done the way they are. Not that there is anything wrong with that, just saying that I might be completely wrong.)
If my assumption is correct then the logic in resetWorktree has to change slightly. If the list of files passed to it is empty the for loop can be skipped, because there is no work to be done. I don't know if the whole function can be skipped, because there is also some index writing done, but I don't know if that needs to stay or not.
There might be other solutions, but from my brief testing this solved the issue of deleted untracked files. Tomorrow I will do some more testing.

k-anshul · 2025-05-08T10:45:42Z

Hey @UnseenBook

Thanks for looking into this and yes it does fail when reseting on to same commit. I am also discovering some more edge cases. I will take a look at it later again.

UnseenBook · 2025-05-08T11:30:57Z

@k-anshul I have created a PR to your branch with changes that I believe are what we need to solve this issue.
I realized that a hard reset does need to remove untracked and ignored files. So I made it only skip resetWorktree if the removedFiles slice is empty AND when mode is set to MergeReset, so that HardReset still uses the list of files provided in the ResetOptions.

The tests run successfully with this change. I hope this helps.

k-anshul · 2025-05-09T14:01:54Z

@k-anshul I have created a PR to your branch with changes that I believe are what we need to solve this issue. I realized that a hard reset does need to remove untracked and ignored files. So I made it only skip resetWorktree if the removedFiles slice is empty AND when mode is set to MergeReset, so that HardReset still uses the list of files provided in the ResetOptions.

The tests run successfully with this change. I hope this helps.

Sounds good @UnseenBook. Just merged your changes in my PR. Thanks for looking into this.

UnseenBook · 2025-05-12T07:26:22Z

Thanks @k-anshul !
What do you expect there is left to fully fix this issue? You mentioned you encountered more edge cases, might they be fixed with my changes, or are they very different? I would love to help :)

k-anshul · 2025-05-12T07:32:32Z

Thanks @k-anshul ! What do you expect there is left to fully fix this issue? You mentioned you encountered more edge cases, might they be fixed with my changes, or are they very different? I would love to help :)

I could not reproduce the issues after your fix :)

jacksoncalvert · 2025-05-19T00:26:06Z

@k-anshul can you adjust the commit message to get this merged please?

…tree. Fixes go-git#53

k-anshul · 2025-05-20T13:05:52Z

@k-anshul can you adjust the commit message to get this merged please?

Done @jacksoncalvert

jacksoncalvert · 2025-05-23T05:24:42Z

@onee-only we are seeing this problem in multiple instances. can we get this merged please

onee-only · 2025-05-23T05:39:52Z

@onee-only we are seeing this problem in multiple instances. can we get this merged please

Unfortunately I'm not the maintainer of this project. Let's wait for maintainers to review this PR, though they appear to be inactive on go-git at the moment.

jacksoncalvert · 2025-05-23T05:45:49Z

@pjbgf Hi Paulo, can we please get this merged in. My team is using this package and this issue is hugely blocking for some of our tasks.

Cheers

UnseenBook · 2025-05-25T09:36:01Z

If it's desired to have a test for this fix, I created a PR to add one: rilldata#4

@k-anshul If you merge the PR by using the rebase option, it should not create a merge commit that would fail the commit message check.

pjbgf

@k-anshul thanks for working on this. 🙇

Ch00k · 2025-07-11T20:51:43Z

Is there any chance this change could be incorporated in one of the upcoming v5.x.x releases?

git: worktree, Don't delete local untracked files when resetting worktree

pjbgf · 2025-12-12T20:24:46Z

@Ch00k feel free to create a back-port targeting the v5 release branch and tagging me for a review.

git: worktree, Don't delete local untracked files when resetting work…

a03b2cb

…tree. Fixes go-git#53

Copilot AI review requested due to automatic review settings May 2, 2025 09:25

k-anshul changed the title ~~git: worktree, Don't delete local untracked files when resetting work…~~ git: worktree, Don't delete local untracked files when resetting worktree May 2, 2025

Copilot AI reviewed May 2, 2025

View reviewed changes

worktree_test.go Outdated Show resolved Hide resolved

worktree.go Outdated Show resolved Hide resolved

git: worktree, Don't delete local untracked files when resetting work…

0e87c1f

…tree. Fixes go-git#53

k-anshul mentioned this pull request May 2, 2025

Don't delete untracked files when resetting worktree. #1235

Closed

onee-only reviewed May 2, 2025

View reviewed changes

k-anshul requested a review from onee-only May 5, 2025 06:42

git: worktree, Don't delete local untracked files when resetting work…

ad102d2

…tree. Fixes go-git#53

k-anshul force-pushed the reset_fix branch from 2823a26 to ad102d2 Compare May 20, 2025 13:02

git: test for branch checkout should not delete untracked file

bf0bb78

pjbgf approved these changes Jun 3, 2025

View reviewed changes

pjbgf merged commit 89fc507 into go-git:main Jun 3, 2025
16 checks passed

pjbgf mentioned this pull request Jun 4, 2025

[v6] Forward-port changes #1564

Merged

Ch00k pushed a commit to Ch00k/go-git that referenced this pull request Jul 12, 2025

Merge pull request go-git#1540 from rilldata/reset_fix

04c89af

git: worktree, Don't delete local untracked files when resetting worktree

Ch00k pushed a commit to Ch00k/go-git that referenced this pull request Sep 20, 2025

Merge pull request go-git#1540 from rilldata/reset_fix

028694b

git: worktree, Don't delete local untracked files when resetting worktree

Ch00k mentioned this pull request Dec 13, 2025

[v5] git: worktree, Don't delete local untracked files when resetting worktree #1800

Merged

	right, err := w.diffStagingWithWorktree(false, true)
	if err != nil {
	return nil, err
	}

	for _, ch := range right {
	a, err := ch.Action()
	if err != nil {
	return nil, err
	}

	fs := s.File(nameFromAction(&ch))
	if fs.Staging == Untracked {
	fs.Staging = Unmodified
	}

	switch a {
	case merkletrie.Delete:
	fs.Worktree = Deleted
	case merkletrie.Insert:
	fs.Worktree = Untracked
	fs.Staging = Untracked
	case merkletrie.Modify:
	fs.Worktree = Modified
	}
	}

Conversation

k-anshul commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

onee-only May 2, 2025

Choose a reason for hiding this comment

Uh oh!

k-anshul May 5, 2025

Choose a reason for hiding this comment

Uh oh!

onee-only May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-anshul May 5, 2025

Choose a reason for hiding this comment

Uh oh!

UnseenBook commented May 7, 2025

Uh oh!

k-anshul commented May 8, 2025

Uh oh!

UnseenBook commented May 8, 2025

Uh oh!

k-anshul commented May 9, 2025

Uh oh!

UnseenBook commented May 12, 2025

Uh oh!

k-anshul commented May 12, 2025

Uh oh!

jacksoncalvert commented May 19, 2025

Uh oh!

k-anshul commented May 20, 2025

Uh oh!

jacksoncalvert commented May 23, 2025

Uh oh!

onee-only commented May 23, 2025

Uh oh!

jacksoncalvert commented May 23, 2025

Uh oh!

UnseenBook commented May 25, 2025

Uh oh!

pjbgf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ch00k commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjbgf commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

k-anshul commented May 2, 2025 •

edited

Loading

onee-only May 2, 2025 •

edited

Loading

Ch00k commented Jul 11, 2025 •

edited

Loading