Re: [PATCH v2 2/2] git_path(): handle `.lock` files correctly

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 18, 2019, 01:23 UTC
Message-ID: <xmqq1rva7sp7.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <93dba5a3a38d75ba79329383a1d50419a1990c8d.1571350077.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 10 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Ever since worktrees were introduced, the `git_path()` function _really_
> needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is
> specific to the worktree). However, the wrong path is returned for
> `logs/HEAD.lock`.
>
> This does not matter as long as the Git executable is doing the asking,
> as the path for that `index.lock` file is constructed from
> `git_path("index")` by appending the `.lock` suffix.

Is this still git_path("index") or is it now HEAD?

> Side note: Git GUI _does_ ask for `index.lock`, but that is already
> resolved correctly.

Is that s/but/and/?

Show 10 quoted lines

> diff --git a/path.c b/path.c
> index e3da1f3c4e..ff85692b45 100644
> --- a/path.c
> +++ b/path.c
> @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,
>  	int result;
>  	struct trie *child;
>  
> -	if (!*key) {
> +	if (!*key || !strcmp(key, ".lock")) {

We only do strcmp for the tail part at the end of the path, so this should probably OK from performance point of view but semantically it is not very satisfying to see a special case for a single .suffix this deep in the callchain. I wonder if it is nicer to have the higher level callers notice ".lock" or whatever other suffixes they care about and ask the lower layer for a key with the suffix stripped?

Will queue.

Thanks.