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

[Windows] "panic: Any" crash in repository with filenames exceeding MAX_PATH #176

Open
nbusseneau opened this issue Jul 6, 2020 · 24 comments · May be fixed by #689
Open

[Windows] "panic: Any" crash in repository with filenames exceeding MAX_PATH #176

nbusseneau opened this issue Jul 6, 2020 · 24 comments · May be fixed by #689
Labels
bug Something isn't working help wanted Extra attention is needed nostale immune to stale-bot windows

Comments

@nbusseneau
Copy link

nbusseneau commented Jul 6, 2020

Hello! Love this tool, thanks for your work 😄
I encountered and reproduced an issue with the Windows binary on repositories containing long filenames.

Bug description
On Windows, when running gitui.exe from the command line in repositories with filenames exceeding MAX_PATH (260 characters), it fails with the following error:

panic: Any
trace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: BaseThreadInitThunk
  10: RtlUserThreadStart

Running from Git Bash yields an additional bit of information about the crash origin:

5⣯: <unknown>>n>load: Any, message: Some(failed to fetch status: Git(Error { code: -1, klass: 30, message: "invalid path \'\\\\?\\C:\\U
   6: <unknown>\Workspace\\Git\\toto\\very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-d
   7: <unknown>orbi-dolor-ligula-tempus-dir\\very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faupanic: Anynown>uctor-morbi-dolor-ligula-tempus\' (path too long)" })), location: Location { file: "asyncgit\\src\\status.rs", line: 122trace:<unknown>
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>dInitThunk
   6: <unknown>readStart
   7: <unknown>
   8: <unknown>
   9: BaseThreadInitThunk
  10: RtlUserThreadStart

To reproduce
On Windows, from command line:

git init toto
cd toto
:: 136 characters long file name
echo . > very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus
:: 140 characters long directory name
mkdir very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir
:: At this point, gitui works
gitui
:: Move file inside directory, which will exceed MAX_PATH length
mv very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir\
:: At this point, gitui crashes
gitui

Expected behavior
gitui does not crash. Since Git allows working with long paths on Windows thanks to git config core.longpaths (usually set at the --system or --global level), this scenario is not an oddity.

Context

  • OS/Distro + Version: Windows 10
  • GitUI Version: 0.8
  • Rust version: unknown (bundled in .exe I guess?)
@extrawurst extrawurst added the bug Something isn't working label Jul 6, 2020
@extrawurst
Copy link
Owner

@Skymirrh thanks for reporting this, unfortunately I have no windows machine at my disposal, I would need support here.

@extrawurst extrawurst added the help wanted Extra attention is needed label Jul 6, 2020
@MCord
Copy link
Contributor

MCord commented Jul 6, 2020

I can look into this.

@laarmen
Copy link

laarmen commented Jul 6, 2020

I just had a quick look, and even if the gitui code were ready to handle long paths, in the end the backing libgit2 doesn't seem to cope with them just yet, see this issue and this recent pull request.

@extrawurst
Copy link
Owner

extrawurst commented Jul 6, 2020

@MCord thanks for volunteering here, but I think @laarmen is right and the only thing we can do is hope/wait for libgit2 to sort this limitation. BUT at least this issue should be resolved to gracefully have gitui exit with a compelling message and maybe even link the libgit issue so people can show demand :)

edit: funny how many tools suffer this: cargo, nodegit (used by many like GitKraken), gitup and even tortoiseGit

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Oct 4, 2020
@nbusseneau
Copy link
Author

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Perhaps this issue should be marked to be exempted, as it's not really getting "stale due to inactivity"?

@stale stale bot removed the wontfix This will not be worked on label Oct 4, 2020
@mdonoughe
Copy link

mdonoughe commented Oct 19, 2020

I cannot reproduce the crash. Instead of crashing, gitui just displays no unstaged files, even if there are unstaged files with short names.

As of the most recent code, even if you add the manifest to enable long filenames for gitui.exe, libgit2 checks if the path of the file it is trying to diff is longer than the old MAX_PATH constant before passing the path to the Win32 API. If the name is longer, diff generation ends (for the whole directory) and libgit2 returns code -1 and no results.

@nbusseneau
Copy link
Author

nbusseneau commented Oct 19, 2020

Still reproducing on my end with v0.10.1, with the steps given in OP. Perhaps your issue is something different?

@extrawurst
Copy link
Owner

@mdonoughe I guess you build yourself from master, right?

@mdonoughe
Copy link

Yeah I was using b21fad7 built locally.

@stale
Copy link

stale bot commented Jan 24, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jan 24, 2021
@nbusseneau
Copy link
Author

Same remark as last time: perhaps this issue should be marked to be exempted, as it's not really getting "stale due to inactivity"?

@stale stale bot removed the wontfix This will not be worked on label Jan 24, 2021
@extrawurst extrawurst added the nostale immune to stale-bot label Jan 24, 2021
@extrawurst
Copy link
Owner

extrawurst commented May 6, 2021

now that libgit2/libgit2#5857 was merged there is a light at the end of the tunnel for this. we now only need to update https://github.com/rust-lang/git2-rs to be using the newest libgit2 master and then we can see if this fixes the issue. any volunteers?

edit: upstream ticket rust-lang/git2-rs#703

@extrawurst
Copy link
Owner

rust-lang/git2-rs#703 is merged, just waiting for a git2-rs release to benefit from it

@extrawurst extrawurst linked a pull request May 8, 2021 that will close this issue
@extrawurst
Copy link
Owner

extrawurst commented May 8, 2021

@nbusseneau could you build gitui from source off of branch fix-176-win-maxpath and check if this fixes your problem?

Of course anyone willing to help with access to a windows machine and a repo that contains such long file names is welcome to help and check if #689 fixes the issue

@nbusseneau
Copy link
Author

could you build gitui from source off of branch fix-176-win-maxpath and check if this fixes your problem?

Not sure if that's the right way to do it (first time using Rust here) but I cross-compiled from WSL using the x86_64-pc-windows-gnu target. It seemed less of a hassle than installing MSVC build tools + Windows SDK.

I don't have access to the original repo causing this issue anymore as it was an internal repo at my previous company, but we can use the minimal reproduction steps outlines in OP to create a sample repo:

git init toto
cd toto
echo . > very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus
mkdir very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir
mv very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir\

First, I made sure git worked as expected:

  • When trying to git add . immediately after the steps above, git fails with:
error: open("very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir/very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus"): Filename too long
error: unable to index file 'very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir/very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus'
fatal: adding files failed
  • This is expected because we did not enable core.longpaths. After running git config core.longpaths true, git add . works:
> git status
On branch main

No commits yet

Changes to be committed:
  (use "git rm --cached <file>..." to unstage)
        new file:   very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir/very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus
  • Ran git rm --cached -r . to reset staged files.

Then, I reproduced the original issue using v0.10.1:

> gitui10 --version
gitui 0.10.1
> gitui10 # results in panic crash as described in OP

Then, I tried to reproduce using v0.15.0:

> gitui15 --version
gitui 0.15.0
> gitui15 # does not panic crash, but no files displayed

This is as noted above: it just displays nothing, even if unstaged files do exist. I think you know this considering your message above, but a change was introduced between v0.10.1 and v0.11.0 that made it so that gitui does not panic crash:

> gitui11 --version
gitui 0.11.0
> gitui11 # does not panic crash, but no files displayed

Then I tried with my cross-compiled build from fix-176-win-maxpath:

> gituiCustom --version # fix-176-win-maxpath cross-compiled from WSL using target x86_64-pc-windows-gnu
gitui 0.15.0
> gituiCustom
  • The files are displayed!
    image
  • However, trying to add them results in an error:
    image

Seems like we still have some work to do 😅

Funnily enough, if files are staged first from the command line, then gitui is able to unstage them without issue no matter the version, as long as it's not v0.10.1 since it still panic crashes.

The difference is obviously that they properly are visibly moved from staged to unstaged on fix-176-win-maxpath, whereas v0.11.0 and up can't show them in the unstaged area, so they seemingly "disappear".

@extrawurst
Copy link
Owner

can you do some more tests:

  • putting the big ass file In a subfolder tmp/<longfilename> and using stage on the folder
  • trying stage all

I will try writing a unittest (my only win machine is the CI)

@nbusseneau
Copy link
Author

  • Having an intermediate folder: we already have long_filename in long_dirname, but I did try with a small bogus tmp container just in case: it does not work either.
  • Stage all triggers the same error.

@extrawurst
Copy link
Owner

@nbusseneau thanks for testing 🤙

@extrawurst
Copy link
Owner

@nbusseneau if you have time to help me get to the bottom of this please join our discord, as you can see even creating a file with such a long name fails in my unittest (on windows): #689

@extrawurst
Copy link
Owner

@nbusseneau ok I got the test to cover adding a long filename just like it should happen in your case (in the root folder, not folder for now):
Screenshot 2021-05-08 at 19 16 43

now comes the kicker: this test works on the windows ci!
can you checkout the branch again and run the tests locally, something must go different on your machine.

run tests using cargo t

@nbusseneau
Copy link
Author

nbusseneau commented May 10, 2021

@nbusseneau if you have time to help me get to the bottom of this please join our discord, as you can see even creating a file with such a long name fails in my unittest (on windows): #689

I suspect the issue is you're trying to create a file with a long name right away, which will always fail. If you want to be able to go over the limit you need to use tricks ;)

One of them is creating a folder with a long-but-not-quite-at-the-limit name, a file with a long-but-not-quite-at-the-limit name, then move the file in the folder, as in the repro steps:

echo . > very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus
mkdir very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir
mv very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus very-long-path-lorem-ipsum-dolor-sit-amet-consectetur-adipiscing-elit-quisque-pharetra-faucibus-diam-ut-auctor-morbi-dolor-ligula-tempus-dir\

This works because the filename is 136 char long, the dir name is 140 char long, and thus both succeed on their own (unless they are created deep in an already lengthy path hierarchy), and then once combined we are sure to go over the 255 limit.

now comes the kicker: this test works on the windows ci!
can you checkout the branch again and run the tests locally, something must go different on your machine.

Sorry, can't do that right away :/
As said above I cross-compiled from WSL using the x86_64-pc-windows-gnu target as it seemed less of a hassle than installing MSVC build tools + Windows SDK. So I can't run cargo t from Windows because Rust is not installed over there.

I can look into installing Rust directly on Windows to do native compiling with MSVC but I have PTSD from the last time I tried to do stuff with third-party chains using MSVC tools: the Visual Studio installer that should be responsible for setting up everything was a complete mess, and even when I finally managed to set it all up after manually fixing registries, etc., it was flaky as hell 😅

@extrawurst
Copy link
Owner

extrawurst commented May 10, 2021

Interesting, I had a vm setup in under an hour. Regarding the repro: I gave up after wasting a few hours on this over the weekend. The branch is there to be picked up by someone who cares more :)

as soon as we have a reliable repro I can work on fixing this.

@nbusseneau
Copy link
Author

nbusseneau commented May 11, 2021

Oh well, perhaps they fixed the toolchain since that time! I may look into that when I have some free time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed nostale immune to stale-bot windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants