From: Jeff Hostetler <git@jeffhostetler.com>
To: Eric DeCosta <edecosta@mathworks.com>,
Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com>,
"git@vger.kernel.org" <git@vger.kernel.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2] fsmonitor: handle differences between Windows named pipe functions
Date: Thu, 27 Apr 2023 09:45:07 -0400 [thread overview]
Message-ID: <dfd693ce-a9dc-6a8c-d16a-0573bc37b93e@jeffhostetler.com> (raw)
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
next prev parent reply other threads:[~2023-04-27 13:45 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-24 17:14 [PATCH] fsmonitor: handle differences between Windows named pipe functions Eric DeCosta via GitGitGadget
2023-03-27 11:37 ` Johannes Schindelin
2023-03-27 15:02 ` Jeff Hostetler
2023-03-27 16:08 ` Junio C Hamano
2023-04-06 19:08 ` Eric DeCosta
2023-04-07 20:55 ` Jeff Hostetler
2023-04-10 19:46 ` [PATCH v2] " Eric DeCosta via GitGitGadget
2023-04-22 20:00 ` Eric DeCosta
2023-04-26 20:33 ` Jeff Hostetler
2023-04-27 13:45 ` Jeff Hostetler [this message]
2023-05-08 20:30 ` Eric DeCosta
2023-05-15 21:50 ` Jeff Hostetler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dfd693ce-a9dc-6a8c-d16a-0573bc37b93e@jeffhostetler.com \
--to=git@jeffhostetler.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=edecosta@mathworks.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).