> -----Original Message-----
> From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Sent: Saturday, September 17, 2022 2:30 AM
> To: Eric DeCosta via GitGitGadget <gitgitgadget@xxxxxxxxx>
> Cc: Git List <git@xxxxxxxxxxxxxxx>; Jeff Hostetler <git@xxxxxxxxxxxxxxxxx>;
> Torsten Bögershausen <tboegi@xxxxxx>; Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx>; Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>;
> Johannes Schindelin <Johannes.Schindelin@xxxxxx>; Eric DeCosta
> <edecosta@xxxxxxxxxxxxx>
> Subject: Re: [PATCH v8 2/5] fsmonitor: relocate socket file if .git directory is
> remote
>
> On Fri, Sep 16, 2022 at 9:12 PM Eric DeCosta via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
> > If the .git directory is on a remote file system, create the socket
> > file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
> >
> > Signed-off-by: Eric DeCosta <edecosta@xxxxxxxxxxxxx>
> > ---
> > diff --git a/compat/fsmonitor/fsm-ipc-darwin.c
> > b/compat/fsmonitor/fsm-ipc-darwin.c
> > @@ -0,0 +1,46 @@
> > +static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path,
> > +"fsmonitor--daemon.ipc")
> > +
> > +const char *fsmonitor_ipc__get_path(struct repository *r) {
> > + static const char *ipc_path;
> > + SHA_CTX sha1ctx;
> > + char *sock_dir;
> > + struct strbuf ipc_file = STRBUF_INIT;
> > + unsigned char hash[SHA_DIGEST_LENGTH];
> > +
> > + if (ipc_path)
> > + return ipc_path;
> > +
> > + ipc_path = fsmonitor_ipc__get_default_path();
> > +
> > + /* By default the socket file is created in the .git directory */
> > + if (fsmonitor__is_fs_remote(ipc_path) < 1)
> > + return ipc_path;
> > +
> > + SHA1_Init(&sha1ctx);
> > + SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
> > + SHA1_Final(hash, &sha1ctx);
> > +
> > + repo_config_get_string(r, "fsmonitor.socketdir", &sock_dir);
> > +
> > + /* Create the socket file in either socketDir or $HOME */
> > + if (sock_dir && *sock_dir)
> > + strbuf_addf(&ipc_file, "%s/.git-fsmonitor-%s",
> > + sock_dir, hash_to_hex(hash));
> > + else
> > + strbuf_addf(&ipc_file, "~/.git-fsmonitor-%s",
> > + hash_to_hex(hash));
>
> A couple comments...
>
> In my mind, the directory specified by `fsmonitor.socketdir` is likely to be
> dedicated to this purpose (i.e. housing Git administrative junk). As such, it
> feels somewhat odd for the socket file to be hidden; I would instead expect
> the socket name to be non-hidden (say,
> "git-fsmonitor-daemon-{hash}.ipc") rather than hidden (".git-fsmonitor-*").
> The directory specified by `fsmonitor.socketdir` may or may not be hidden
> (i.e. start with a dot), but that's the user's decision. For the $HOME case, it
> almost feels cleaner to create a hidden directory (say, "$HOME/.git-
> fsmonitor") in which to house the socket files ("git-fsmonitor-daemon-
> {hash}.ipc"). Anyhow, this comment is quite subjective; perhaps not
> actionable.
> What happens if either $HOME or `fsmonitor.socketdir` are network-
> mounted? Should this code be checking for that case? If they are network-
> mounted, should it error out? At minimum, I would think a warning is
> warranted in order to save users the headache of wondering why fsmonitor
> isn't working correctly.
>
Ultimately, the UDS file location is checked by check_uds_volume in fsm-settings-darwin as part of the overall settings checks; it will error-out there if the path is on the network.
> > + ipc_path = interpolate_path(ipc_file.buf, 1);
> > + if (!ipc_path)
> > + die(_("Invalid path: %s"), ipc_file.buf);
> > +
> > + strbuf_release(&ipc_file);
>
> `sock_dir` is being leaked, isn't it?
>
Sure is, thanks.
> > + return ipc_path;
> > +}
> > diff --git a/compat/fsmonitor/fsm-ipc-win32.c
> > b/compat/fsmonitor/fsm-ipc-win32.c
> > @@ -0,0 +1,9 @@
> > +const char *fsmonitor_ipc__get_path(struct repository *r) {
> > + static char *ret;
> > + if (!ret)
> > + ret = git_pathdup("fsmonitor--daemon.ipc");
> > + return ret;
> > +}
> > \ No newline at end of file
>
> Mentioned already.
