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

fsmonitor--daemon failed to start - using git on a network path #3335

Closed
JohannesBe opened this issue Jul 26, 2021 · 12 comments
Closed

fsmonitor--daemon failed to start - using git on a network path #3335

JohannesBe opened this issue Jul 26, 2021 · 12 comments

Comments

@JohannesBe
Copy link

JohannesBe commented Jul 26, 2021

I have read this issue with a similar problem, but I think I am receiving the same error, but for a different reason.

My git repository is located on a network drive (in fact a passthrough volume in virtualbox, i.e. host windows OS > child windows OS). I am running the git commands in the child VM.

I have created a perf.log file as described in the other issue, and I think the relevant part of it is the following:

12:32:13.400687 usage.c:74                   | d1 | th09:fsm-listen          | error        |     |           |           |              | ReadDirectoryChangedW failed on '//VBoxSvr/Code/projects/portal' [GLE 1]

For completeness, I have attached the rest of the file as well.
perf.log

@dscho
Copy link
Member

dscho commented Jul 26, 2021

I have read this issue with a similar problem, but I think I am receiving the same error, but for a different reason.

[...]

ReadDirectoryChangedW failed on '//VBoxSvr/Code/projects/portal'

Hmm. The documentation of the ReadDirectoryChangesW() function suggests that network filesystems are supported (it talks about SMB 3.0). Likewise, this code comment suggests the same.

The GLE value is the GetLastError() return value, and the value 1 corresponds to ERROR_INVALID_FUNCTION.

Since you mention VBoxSvr, I guess that this simply means that either VirtualBox' SMB implementation (if it even has one), or more likely, the client Operating System running inside your VirtualBox, lack support for a crucial functionality required by the ReadDirectoryChangesW() function.

@JohannesBe to unblock you, I would suggest to call git config core.useBuiltinFSMonitor false in that worktree. If you want to do that only when accessing the worktree from outside the VM, you might have to jump through the hoop where you call git config -f $HOME/.gitconfig.disable-fsmonitor core.useBuiltinFSMonitor false && MSYS_NO_PATHCONV=1 git config --global 'includeif.gitdir/i://VBoxSvr/Code/projects/portal/.git.path' $HOME/.gitconfig.disable-fsmonitor in Git Bash (the command-line might not be completely correct, I ask you to kindly fix that quietly if that is the case).

@jeffhostetler the bigger question here is: how can we deal with this situation in the best manner? Should we maybe modify the repo config upon first start, to avoid trying again? If so, there is the very real problem that we might still want to enable the FSMonitor under certain circumstances, e.g. when running inside a VM and having "local" access. We could of course introduce a new config variable (e.g. something like core.FSMonitor.excludeWorktree) and append that to the repo config when detecting that ReadDirectoryChangesW() failed, and heeding it when we determine whether FSMonitor is enabled at all. Something to mull about, I guess?

@jeffhostetler
Copy link

I'm going to investigate adding a call to GetDriveTypeW() near the checks for a bare repo and try to detect non-local repos and give a better error. I'm wondering if this only takes "C:/" paths or it also handles share paths.

The advantage with this approach is that we can test the drive type in client (like git status) calls and not waste time
trying to contact the daemon -- just like we shouldn't for bare repos. And on the daemon side, we get a better
error message.

@dscho
Copy link
Member

dscho commented Jul 30, 2021

I am slightly concerned that that call won't tell us whether the FSMonitor can work in this location or not. From what I understand, regular Windows file shares do support the ReadDirectoryChangesW() API... It's just that not all SMB/CIFS implementations seem to offer all the required functionality.

@JohannesBe
Copy link
Author

JohannesBe commented Jul 30, 2021

Thank you already for your efforts and quick response. Temporarily disabling the fsmonitor worked as a workaround. Appreciate it that you're looking into how to make the UX better upon this failure 🙂

@jeffhostetler
Copy link

Fun fact: when you call ReadDirectoryChangesW() with a very large buffer (in overlapped I/O mode) on a network share, it does not throw the error described in my code comment -- then. It waits until you call GetOverlappedResult() to give you the buffer too large error. I guess that makes sense (read: no, not really). Moving the buffer resize logic fixes the problem.

I was testing a remote repo from a shared folder on a windows file server and I was getting an GLE 87 and it now works. This should be in the next release.

However, the original problem described a virtualbox pass thru and I doubt that my fix will address that, so more digging is required here.

@Josephkagimu1
Copy link

i agree with you ... Jeffhostetler

@dscho
Copy link
Member

dscho commented Aug 8, 2021

@JohannesBe could you verify that the workaround in v2.33.0-rc1 works?

@jeffhostetler
Copy link

I added code in v2.33.0-rc1 that disables FSMonitor on network shares. We may want to relax this in the future, but for now during the initial rollout, let's restrict FSMonitor to working on a local working directory.

(I'm trying to avoid the problem where the remote OS does not support file/directory change notifications. This could cause the daemon to start, fail, and shutdown. And then every git status would try to restart it, wait for it to start or fail, and then fallback and do the work itself. And we'd have made the user experience worse. Granted, we could have the daemon set a config setting to say we tried and gave up -- or unset core.useBuiltinFSMonitor -- but if you're sharing the working directory and accessing it from both the host and guest systems, it is unclear where to store that information. So for now, let's avoid the issue and disable it.)

@dscho
Copy link
Member

dscho commented Aug 13, 2021

@JohannesBe could you verify that the workaround in v2.33.0-rc1 works?

Could you? -rc2 is out now... https://github.com/git-for-windows/git/releases/tag/v2.33.0-rc2.windows.1

@JohannesBe
Copy link
Author

JohannesBe commented Aug 14, 2021 via email

@JohannesBe
Copy link
Author

I confirmed that using version v2.33.0.windows.2 the workaround works.
In my configuration I have the following:

Johannes@**********  MINGW64 //10.0.2.2/share/portal2 (ops/storybook)
$ git config -l | grep core.use
core.usebuiltinfsmonitor=true

But a git status works as intended;

  • on first use an index refresh is performed
  • on subsequent calls, it takes a while (esp. in a large repo if I do e.g. mv large-folder large-folder2).

This tells me that indeed the file system monitor seems disabled and repository scanning is used as a fallback.

I think this issue thus can more or less be closed, although it might be worth it to create a follow-up issue to re-enable fsmonitor for network drives that support it. If you want, I can create that subsequent issue, but I will probably not be able to follow it up.

@JohannesBe
Copy link
Author

By the way, thanks for your fast co-operation and responses, I really appreciate that :)
Keep up the good work!

@dscho dscho closed this as completed Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants