discard/reset on folder can cause errors

[benbfortis]

Describe the bug

This only seems to apply to YAML files (extensions .unity, .asset) created in Unity3D. To make sure there are no handles to those files, every related program was closed first.

Using a hard reset on a folder from the Files panel (i.e. D > h) causes a list of errors in the form:

error: unable to unlink old 'path/to/file1': Invalid argument
error: unable to unlink old 'path/to/file2': Invalid argument
fatal: Could not reset index file to revision 'HEAD'

Using a hard reset on the same folder via the Local Branches panel (g > h) or directly from the command line does not cause those errors.

Same applies to using Discard Changes on a folder as opposed to individual files.

Expected behavior Discard all the changes without issues

Version info: commit=367b0d331836c90c015bf0c45f88612f3d94d08a, build date=2022-07-20T09:27:56Z, build source=binaryRelease, version=0.35, os=windows, arch=amd64

git version 2.37.3.windows.1

[jesseduffield]

@mjarkk this is another argument in favour of changing how we discard folders (i.e. just using git checkout)

[mark2185]

I'm sorry, but I think you've been accidentally tagging mjarkk instead of mark2185 for the past couple of months.

[benbfortis]

I should clarify that using discard changes (git checkout) has a similar problem, and throw the same error after the first file with multiple changed files

For instance if there are three changed YAML files the the same folder, using discard on the folder will correctly reset the first file, but the second and third files will have the error. Running discard again will then reset the second file and the third will have the error.

[mark2185]

Yeah, that's what Jesse was referring to.

The current implementation walks the tree and discards each node individually, recently I proposed we just invoke git checkout -- on the entire directory for performance reasons, but now it turns out it also might fix this bug.

[benbfortis]

Ahh, I see. Makes sense now. If there's any early/preview build and you'd like it tested in this specific context, let me know

[mark2185]

Well, if you're happy to build from source, you can try building the branch develop from my fork.

[jesseduffield]

I'm sorry, but I think you've been accidentally tagging mjarkk instead of mark2185 for the past couple of months.

Oh my goodness, apologies to both of you! (and hi @mjarkk I hope you're doing well if you're reading this)

[benbfortis]

@mark2185 I tried using discard changes on a folder from your fork, but I got the same error for all the changed files. Funny thing is that it will discard changes to .cs files

[mark2185]

Are you using git-lfs by any chance?

[benbfortis]

Not generally for file types in this project. Only one attibutes file tracking a very specific file in a third party module.

[mark2185]

Just to clarify, you used d to discard a changes, not g for a reset? Did the log in the lower right corner say it tried executing git checkout -- <dir>?

[benbfortis]

Yes, I used discard, and the logs did show the checkout command in the form git checkout --\"dir\"

Another odd thing was I then tried using discard on single files, but it was erratic if it discard or not. For example, there were three files A,B,C. If I tried to discard A it would error repeatedly. I then went to C and it discarded. Then I went to B and it discarded. Then back to A and it too also then discarded. I checked the log and the checkout commands were there and all the same for A.

[mark2185]

Huh

And invoking the same thing directly from the cli works whichever order you try to checkout -- the A, B and C files?

Sorry for pestering you with so many questions, I'm just trying to narrow it down.

[benbfortis]

Yeah, cli checkouts work for both the folder and individual files.

However, at first I forgot to close lazygit, and the cli checkout failed with Unlink of file 'path/to/file' failed. Guess it's got a handle on the file, but maybe thats a clue to what's going wrong?

[mark2185]

I presume you wouldn't be willing to try and reproduce this on a unix machine?

git has always been funny on windows, with spurious locks and whatnot.

[benbfortis]

Unfortunately I don't have access to a unix machine without setting up and entire new system for it

[mark2185]

Is the repo in question public so we might try to reproduce it?

[benbfortis]

I'm afraid not. I'll try with a fresh git project and see if it happens there

[mark2185]

@mjarkk this is another argument in favour of changing how we discard folders (i.e. just using git checkout)

Adding a note for future implementers: test out discarding folders when there are conflicting files in a rebase scenario

[benbfortis]

I've added a test project here, which is a stripped down copy of what is causing issues. It's a Unity project, but I've added the local changed files so you don't have to go through the pain of installing it. https://github.com/benbfortis/TestProject

[mark2185]

Tried it in a windows 11 VM, no luck. Maybe @jesseduffield will have more luck in reproducing it.

[benbfortis]

I have another Windows PC, but with Windows 11 on it. I'll try it on that and at least see if it's machine/windows version related

[benbfortis]

I've tried it on my other Windows 11 PC, and I'm not getting the issue. I'll try and figure out if there are other differences beside OS version.

[mark2185]

The original OS was Windows 10?

[benbfortis]

Yes, Windows 10. There is no difference in global or project gitconfig between them, or anything else obvious I can see.

I've not idea if this means anything, but I had the Process Explorer utility open and noticed that git had handles to the problem files when the Files panel is selected in lazygit. Could it be an issue with these not being freed up when discard is selected? Google searches of that error mention stuff about another program having a handle to the problem file. Yes, I'm grasping at straws here.