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

Git Discard All Changes fails with "ENAMETOOLONG" #65693

Closed
kumarharsh opened this issue Dec 26, 2018 · 15 comments
Closed

Git Discard All Changes fails with "ENAMETOOLONG" #65693

kumarharsh opened this issue Dec 26, 2018 · 15 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities verified Verification succeeded
Milestone

Comments

@kumarharsh
Copy link
Contributor

kumarharsh commented Dec 26, 2018

  • VSCode Version: 1.30.1
  • OS Version: Windows 10 Pro 1809

Steps to Reproduce:

  1. Modify two thousand files in your working directory
  2. Open the Source Control tab
  3. Hover over the Changes sidebar group and click the Discard All Changes button.
  4. VSCode throws a ENAMETOOLONG error.

Reported earlier at #23943, but the issue was closed.

Does this issue occur when all extensions are disabled?: Yes

@vscodebot vscodebot bot added the git GIT issues label Dec 26, 2018
@kumarharsh
Copy link
Contributor Author

kumarharsh commented Dec 26, 2018

It's because vscode runs this command on clicking "Discard all changes":

git checkout -q -- file/path/1 file/path/2 file/path/3 file/path/4 file/path/5 ...

@kumarharsh
Copy link
Contributor Author

It seems like it's actually a shortcoming of git itself.
Using git checkout -q -- . can be a viable alternative, would you agree?

@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities good first issue Issues identified as good for first-time contributors labels Jan 3, 2019
@joaomoreno joaomoreno added this to the December/January 2019 milestone Jan 3, 2019
@rcbevans
Copy link
Contributor

rcbevans commented Jan 4, 2019

I have reproed this locally. In my test repo, I can see that discarding 2001 modified files creates a command line which is 78949 characters long, but the windows limit is 32767, assuming node is calling CreateProcess under the covers, and based on the info provided here: https://blogs.msdn.microsoft.com/oldnewthing/20031210-00/?p=41553/.

The problem is, the UI for discard all changes, in commands.ts:cleanAll, is translating the cleanAll into a repository.ts:clean with a list of all of the modified files. The repository itself doesn't know that this list of resources is all of the changes in the repository.

Furthermore, this issue doesn't affect only discard all changes. I was able to select the top change, and scroll down then shift click to select a subset of all of the changes, then right click, and select discard to see the same ENAMETOOLONG error message.

Simply replacing git checkout -q -- <files> with git checkout -q -- . won't fix the real issue, which is that the git operations aren't sanity checking their length prior to attempting to spawn the git command.

Looking in the code at repository.ts:clean, I suspect a similar issue may occur with git.ts:clean, and it may occur with other repository types, other that git, if they do anything similar in terms of concatenating the list of resources.

Two possible options for a fix:

  1. fix in respository.ts:clean, with the creation of the repository operations being split into multiple based on the length of the resource paths for toClean, and toCheckout, (and possibly even submodulesToUpdate?).
  2. fix in git.ts:clean/checkout/updateSubmodules.

Fixing in respository.ts means that any SCS underneath will benefit from being called with a list of files of a reasonable length.

The maximum allowable length will vary from system to system, with Windows having a much smaller maximum allowable size than others.

xargs --show-limits will show the maximum allowable command line argument length for a unix, and most unix-like systems, based on the hardware and environment variables. Maybe the code which decides how to break down the argument could/should be configurable based on the system? As a first attempt, the code could take 4096 as an upper limit, which is the minimum allowable max length of a unix system, as reported by xargs, and is the smallest common denominator?

@joaomoreno
Copy link
Member

Simply replacing git checkout -q -- <files> with git checkout -q -- . won't fix the real issue, which is that the git operations aren't sanity checking their length prior to attempting to spawn the git command.

Still, if this would solve the problem, I'd take it. I forgot why we had to list all files explicitly, but I'm assuming we'll remember as soon as we give it a try. We have to experiment with it.

@rcbevans
Copy link
Contributor

rcbevans commented Jan 4, 2019

But it won't fix the problem. As I mentioned, if I select a long subset of changes, not all, it still goes through the same code path and gives the same error. If git checkout -q -- . gets called there, it will incorrectly revert all files, not just the ones selected.

@kumarharsh
Copy link
Contributor Author

That's correct... Perhaps some sort of batching needs to be implemented to handle these type of cases?

@Herohtar
Copy link

The underlying issue is not specific to Discard All nor is it even specific to Discard in general. This affects any git command that has the potential of containing a long list of files -- if you select a large number of modified/added files and Stage/Unstage them, for example.

The problem is as @rcbevans stated: the command ends up being too long because it's just a concatenated list of files.

It looks like some commands (like git reset) support the --stdin flag which gets around the shell limits (see desktop/desktop#2877), but it's not official and not every command allows it. Probably the best solution at the time is to check the length and break the command into multiple calls if it's too big. The SmartGit client does this, and it works well.

@slim-hmidi
Copy link

I get the same issue. Is there any temporary solution to resolve that?

@joaomoreno joaomoreno modified the milestones: Backlog, August 2019 Aug 6, 2019
@kumarharsh
Copy link
Contributor Author

@joaomoreno is this fixed in a commit? or did you close it by mistake? or is there some other plan to tackle this?

@joaomoreno
Copy link
Member

@kumarharsh #66095

@RMacfarlane RMacfarlane added the verified Verification succeeded label Aug 28, 2019
@rsaritzky
Copy link

Is this released in the latest version of VSCode. I don't know by looking at #66095

@Herohtar
Copy link

Herohtar commented Sep 12, 2019

@rsaritzky As far as I can tell, it is. The PR was merged and some changes were made to clean it up and reorganize some things. The changes seem to be there in the 1.38.1 tag. Of course, the best way to verify is to try a command that used to fail and see if it works in the latest version.

@rsaritzky
Copy link

rsaritzky commented Sep 12, 2019 via email

@Herohtar
Copy link

@rsaritzky Go to Help -> About in VS Code and check which version you are running. I just tested on my installation which is running 1.38.0 and was able to do a Discard All with 1,130 modified files.

@rsaritzky
Copy link

rsaritzky commented Sep 12, 2019 via email

@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants
@joaomoreno @kumarharsh @rcbevans @RMacfarlane @Herohtar @slim-hmidi @rsaritzky and others