> -----Original Message----- > From: Eric DeCosta via GitGitGadget <gitgitgadget@xxxxxxxxx> > Sent: Monday, April 10, 2023 3:46 PM > To: git@xxxxxxxxxxxxxxx > Cc: Johannes Schindelin <Johannes.Schindelin@xxxxxx>; Jeff Hostetler > <git@xxxxxxxxxxxxxxxxx>; Eric DeCosta <edecosta@xxxxxxxxxxxxx>; Eric > DeCosta <edecosta@xxxxxxxxxxxxx> > Subject: [PATCH v2] fsmonitor: handle differences between Windows named > pipe functions > > From: Eric DeCosta <edecosta@xxxxxxxxxxxxx> > > The two functions involved with creating and checking for the existance of > the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW > appear to handle names with leading slashes or backslashes a bit differently. > > If a repo resolves to a remote file system with the UNC path of //some- > server/some-dir/some-repo, CreateNamedPipeW accepts this name and > creates this named pipe: \\.\pipe\some-server\some-dir\some-repo > > However, when the same UNC path is passed to WaitNamedPipeW, it fails > with ERROR_FILE_NOT_FOUND. > > Skipping the leading slash for UNC paths works for both CreateNamedPipeW > and WaitNamedPipeW. Resulting in a named pipe with the same name as > above that WaitNamedPipeW is able to correctly find. > > Signed-off-by: Eric DeCosta <edecosta@xxxxxxxxxxxxx> > --- > fsmonitor: handle differences between Windows named pipe functions > > The two functions involved with creating and checking for the existance of > the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW > appear to handle names with leading slashes or backslashes a bit differently. > > If a repo resolves to a remote file system with the UNC path of //some- > server/some-dir/some-repo, CreateNamedPipeW accepts this name and > creates this named pipe: .\pipe\some-server\some-dir\some-repo > > However, when the same UNC path is passed to WaitNamedPipeW, it fails > with ERROR_FILE_NOT_FOUND. > > Skipping the leading slash for UNC paths works for both CreateNamedPipeW > and WaitNamedPipeW. Resulting in a named pipe with the same name as > above that WaitNamedPipeW is able to correctly find. > > v1 differs from v0: > > * Abandon hashing repo path in favor of simpler solution where the leading > slash is simply dropped for UNC paths > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr- > 1503%2Fedecosta-mw%2Ffsmonitor_windows-v2 <https://protect- > us.mimecast.com/s/WUMKCqxoXGIDMzRYtE7lUM?domain=github.com> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git <https://protect- > us.mimecast.com/s/meLwCrkpNJSpB16QhjvzbL?domain=github.com> pr- > 1503/edecosta-mw/fsmonitor_windows-v2 > Pull-Request: https://github.com/gitgitgadget/git/pull/1503 <https://protect- > us.mimecast.com/s/0_e0Cv2w7NFGk24wH5_RA3?domain=github.com> > > Range-diff vs v1: > > 1: c5307928cd7 ! 1: b41f9bd2e96 fsmonitor: handle differences between > Windows named pipe functions @@ Metadata ## Commit message ## > fsmonitor: handle differences between Windows named pipe functions > > - CreateNamedPipeW is perfectly happy accepting pipe names with > seemingly > - embedded escape charcters (e.g. \b), WaitNamedPipeW is not and > incorrectly > - returns ERROR_FILE_NOT_FOUND when clearly a named pipe, succesfully > created > - with CreateNamedPipeW, exists. > + The two functions involved with creating and checking for the > + existance of the local fsmonitor named pipe, CratedNamedPipeW and > + WaitNamedPipeW appear to handle names with leading slashes or > backslashes a bit differently. > > - For example, this network path is problemmatic: > - \\batfs-sb29-cifs\vmgr\sbs29\my_git_repo > + If a repo resolves to a remote file system with the UNC path of > + //some-server/some-dir/some-repo, CreateNamedPipeW accepts this > name > + and creates this named pipe: \\.\pipe\some-server\some-dir\some-repo > > - In order to work around this issue, rather than using the path to the > - worktree directly as the name of the pipe, instead use the hash of the > - worktree path. > + However, when the same UNC path is passed to WaitNamedPipeW, it fails > + with ERROR_FILE_NOT_FOUND. > + > + Skipping the leading slash for UNC paths works for both > + CreateNamedPipeW and WaitNamedPipeW. Resulting in a named pipe with > + the same name as above that WaitNamedPipeW is able to correctly find. > > Signed-off-by: Eric DeCosta <edecosta@xxxxxxxxxxxxx> > > ## compat/simple-ipc/ipc-win32.c ## > -@@ > - #include "cache.h" > -+#include "hex.h" > - #include "simple-ipc.h" > - #include "strbuf.h" > - #include "pkt-line.h" > @@ > static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t > alloc) { int off = 0; > -- struct strbuf realpath = STRBUF_INIT; > -- > -- if (!strbuf_realpath(&realpath, path, 0)) > -- return -1; > -+ int ret = 0; > -+ git_SHA_CTX sha1ctx; > -+ struct strbuf real_path = STRBUF_INIT; struct strbuf pipe_name = > -+ STRBUF_INIT; unsigned char hash[GIT_MAX_RAWSZ]; > ++ int real_off = 0; > + struct strbuf realpath = STRBUF_INIT; > > -- off = swprintf(wpath, alloc, L"\\\\.\\pipe\\"); > -- if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0) > -+ if (!strbuf_realpath(&real_path, path, 0)) > + if (!strbuf_realpath(&realpath, path, 0)) > return -1; > > -- /* Handle drive prefix */ > -- if (wpath[off] && wpath[off + 1] == L':') { > -- wpath[off + 1] = L'_'; > -- off += 2; > -- } > -+ git_SHA1_Init(&sha1ctx); > -+ git_SHA1_Update(&sha1ctx, real_path.buf, real_path.len); > -+ git_SHA1_Final(hash, &sha1ctx); strbuf_release(&real_path); > - > -- for (; wpath[off]; off++) > -- if (wpath[off] == L'/') > -- wpath[off] = L'\\'; > -+ strbuf_addf(&pipe_name, "git-fsmonitor-%s", hash_to_hex(hash)); off = > -+ swprintf(wpath, alloc, L"\\\\.\\pipe\\"); if (xutftowcs(wpath + off, > -+ pipe_name.buf, alloc - off) < 0) ret = -1; > - > -- strbuf_release(&realpath); > -- return 0; > -+ strbuf_release(&pipe_name); > -+ return ret; > - } > ++ /* 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; > > - static enum ipc_active_state get_active_state(wchar_t *pipe_path) > + /* Handle drive prefix */ > > > compat/simple-ipc/ipc-win32.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c > index 997f6144344..632b12e1ab5 100644 > --- a/compat/simple-ipc/ipc-win32.c > +++ b/compat/simple-ipc/ipc-win32.c > @@ -19,13 +19,18 @@ > static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t > alloc) { int off = 0; > + int real_off = 0; > struct strbuf realpath = STRBUF_INIT; > > if (!strbuf_realpath(&realpath, path, 0)) return -1; > > + /* 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; > > /* 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
