RE: [PATCH v6 3/6] fsmonitor: relocate socket file if .git directory is remote
Show 19 quoted lines
> -----Original Message-----
> From: Junio C Hamano <gitster@pobox.com>
> Sent: Tuesday, September 13, 2022 8:48 PM
> To: Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com>
> Cc: git@vger.kernel.org; Jeff Hostetler <git@jeffhostetler.com>; Eric Sunshine
> <sunshine@sunshineco.com>; Torsten Bögershausen <tboegi@web.de>;
> Ævar Arnfjörð Bjarmason <avarab@gmail.com>; Ramsay Jones
> <ramsay@ramsayjones.plus.com>; Johannes Schindelin
> <Johannes.Schindelin@gmx.de>; Eric DeCosta <edecosta@mathworks.com>
> Subject: Re: [PATCH v6 3/6] fsmonitor: relocate socket file if .git directory is
> remote
>
> "Eric DeCosta via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +const char *fsmonitor_ipc__get_path(void) {
>
> Looks like a bit klunky API. I would have expected it to take at least the path
> to the worktree or a pointer to struct repository.
> OK, I'll change it to take a pointer to struct repository.
Show 29 quoted lines
> > + 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, the_repository->worktree,
> strlen(the_repository->worktree));
> > + SHA1_Final(hash, &sha1ctx);
>
> I would not worry about SHA-1 hash collision for this use case, but would
> worry more about .worktree possible being unique.
>
> Can the .worktree string be aliased for the same directory in some way (e.g.
> depending on the initialization sequence, can it be a full pathname, a relative
> pathname, or can a pathname that looks like a full-pathname have symbolic
> link component in it?) that ends up creating two or more socket for the same
> worktree?
>
.worktree is set to the real path and since hardlinks are not allowed across file systems, I think we are OK.
Show 18 quoted lines
> > + repo_config_get_string(the_repository, "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));
> > +
> > + ipc_path = interpolate_path(ipc_file.buf, 1);
> > + if (!ipc_path)
> > + die(_("Invalid path: %s"), ipc_file.buf);
> > +
> > + strbuf_release(&ipc_file);
> > + return ipc_path;
> > +}Eric DeCosta <edecosta@mathworks.com>