From: Jeff Hostetler Date: Thu, 27 Apr 2023 13:45:07 GMT Subject: Re: [PATCH v2] fsmonitor: handle differences between Windows named pipe functions Message-ID: In-Reply-To: <48d44c06-34ba-0a1f-dd0d-7d66bd8dfcb9@jeffhostetler.com> On 4/26/23 4:33 PM, Jeff Hostetler wrote: ... >>> >>> + /* UNC Path, skip leading slash */ >>> + if (realpath.buf[0] == '/' && realpath.buf[1] == '/') real_off = 1; >>> + >>> off = swprintf(wpath, alloc, L"\\\\.\\pipe\\"); >>> - if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0) >>> + if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0) >>> return -1; > > I haven't had a chance to test this, but this does look > like a minimal solution for the pathname confusion in the > MSFT APIs. > > Do you need to test for realpath.buf[0] and [1] being a forward OR > a backslash ? > > Should we set real_off to 2 rather than 1 because we already > appended a trailing backslash in the swprintf() ? On second thought, you might actually need the second slash in //./pipe//server/mount/whatever so that the GfW installer can find remote repo path to CWD and stop the daemon. Testing will tell here. Jeff > > You should run one of those NPFS directory listing tools to > confirm the exact spelling of the pipe matches your expectation > here.  Yes, if both functions now work, we should be good, but > it would be good to confirm your real_off choice, right? > > If would be good to (at least interactively) test that the > git-for-windows installer can find the path and stop the daemon > on an upgrade or uninstall.  See Johannes' earlier point. > > We should state somewhere that we are running the fsmonitor > daemon locally and it is watching a remote file system. > > You should run a few stress tests to ensure that the > MAX_RDCW_BUF_FALLBACK throttling works and that the daemon > doesn't fall behind on a very busy remote file system.  (There > are SMB/CIFS wire protocol limits.  See the source.)  (I did > test this between the combination of systems that I had, but > YMMV.) > > During the stress test, it would also be good to test that > IO generated by a client process on your local machine to the > remote file system is reported, but also that random IO from > remote processes on the remote system are seen in the event > stream.  Again, I tested the combinations of machines that I > had available at the time. > > Hope this helps, > Jeff > > >>> >>> /* Handle drive prefix */ >>> >>> base-commit: f285f68a132109c234d93490671c00218066ace9 >>> -- >>> gitgitgadget >> >> Are there any other thoughts about this? >> >> I believe that this is the simplest change possible that will ensure that >> fsmonitor correctly handles network repos. >> >> -Eric