git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Oct 16, 2019, 11:04 UTC
Message-ID
<20191016110440.GV29845@szeder.dev>
In-Reply-To
<f08c90ea02f6811b310a01301acf46da133f38a9.1571209637.git.gitgitgadget@gmail.com>
On Wed, Oct 16, 2019 at 07:07:17AM +0000, Johannes Schindelin via GitGitGadget wrote:
Show 5 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 `index`. However, the wrong path
> is returned for `index.lock`.

Could you give an example where it returns the wrong path for 'index.lock'? I tried to reproduce this issue in a working tree, but no matter what I've tried, 'git rev-parse --git-dir index.lock' always returned the right path.

Show 61 quoted lines
> 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.
> 
> However, Git GUI just learned to use `--git-path` instead of appending
> relative paths to what `git rev-parse --git-dir` returns (and as a
> consequence not only using the correct hooks directory, but also using
> the correct paths in worktrees other than the main one). And one of the
> paths it is looking for is... you guessed it... `index.lock`.
> 
> So let's make that work as script writers would expect it to.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  path.c               |  4 ++--
>  t/t1500-rev-parse.sh | 15 +++++++++++++++
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> 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 have reached the end of the key */
>  		if (root->value && !root->len)
>  			return fn(key, root->value, baton);
> @@ -288,7 +288,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,
>  
>  	/* Matched the entire compressed section */
>  	key += i;
> -	if (!*key)
> +	if (!*key || !strcmp(key, ".lock"))
>  		/* End of key */
>  		return fn(key, root->value, baton);
>  
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 01abee533d..d318a1eeef 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'git-path in worktree' '
> +	test_tick &&
> +	git commit --allow-empty -m empty &&
> +	git worktree add --detach wt &&
> +	test_write_lines >expect \
> +		"$(pwd)/.git/worktrees/wt/logs/HEAD" \
> +		"$(pwd)/.git/worktrees/wt/logs/HEAD.lock" \
> +		"$(pwd)/.git/worktrees/wt/index" \
> +		"$(pwd)/.git/worktrees/wt/index.lock" &&
> +	git -C wt rev-parse >actual \
> +		--git-path logs/HEAD --git-path logs/HEAD.lock \
> +		--git-path index --git-path index.lock &&
> +	test_cmp expect actual
Without the fix applied this test fails with:
  + test_cmp expect actual
  --- expect      2019-10-16 10:20:31.047229423 +0000
  +++ actual      2019-10-16 10:20:31.051229519 +0000
  @@ -1,4 +1,4 @@
   /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD
  -/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD.lock
  +/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/logs/HEAD.lock
   /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index
   /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index.lock
  error: last command exited with $?=1

So the path of 'index.lock' seems to be fine already, it's the path of the lockfile for HEAD's reflog that's indeed wrong and makes the test fail.

On a related note, I'm not sure whether the path of the reflogs directory is right while in a different working tree... Both with and without this patch I get a path pointing to the main working tree:

  $ ./git -C WT/ rev-parse --git-path logs
  /home/szeder/src/git/.git/logs

However, I'm not sure what the right path should be in the first place, given that each working tree has its own 'logs' directory, but only for HEAD's reflog, while everything else goes to the main working tree's 'logs' directory.

Show 7 quoted lines
> +'
> +
>  test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '
>  	test_commit test_commit &&
>  	echo true >expect &&
> -- 
> gitgitgadget
Previous: Johannes Schindelin via GitGitGadgetNext: Junio C Hamano
Message 4 of 31 in “Handle git_path() with lock files correctly in worktrees”
  1. 0/2 Handle git_path() with lock files correctly in worktreesJohannes Schindelin via GitGitGadget, Oct 16, 2019
  2. 1/2 t1400: wrap setup code in test caseJohannes Schindelin via GitGitGadget, Oct 16, 2019
  3. 2/2 git_path(): handle `.lock` files correctlyJohannes Schindelin via GitGitGadget, Oct 16, 2019
  4. SZEDER GáborOct 16, 2019
  5. Junio C HamanoOct 17, 2019
  6. Johannes SchindelinOct 17, 2019
  7. SZEDER GáborOct 18, 2019
  8. SZEDER GáborOct 18, 2019
  9. 0/2 path.c: minor common_list fixesSZEDER Gábor, Oct 18, 2019
  10. 1/2 path.c: fix field name in 'struct common_dir'SZEDER Gábor, Oct 18, 2019
  11. 2/2 path.c: mark 'logs/HEAD' in 'common_list' as fileSZEDER Gábor, Oct 18, 2019
  12. Johannes SchindelinOct 21, 2019
  13. 0/2 Handle git_path() with lock files correctly in worktreesJohannes Schindelin via GitGitGadget, Oct 17, 2019
  14. 1/2 t1400: wrap setup code in test caseJohannes Schindelin via GitGitGadget, Oct 17, 2019
  15. 2/2 git_path(): handle `.lock` files correctlyJohannes Schindelin via GitGitGadget, Oct 17, 2019
  16. Junio C HamanoOct 18, 2019
  17. SZEDER GáborOct 18, 2019
  18. Johannes SchindelinOct 21, 2019
  19. Junio C HamanoOct 23, 2019
  20. 0/2 Handle git_path() with lock files correctly in worktreesJohannes Schindelin via GitGitGadget, Oct 21, 2019
  21. 1/2 t1400: wrap setup code in test caseJohannes Schindelin via GitGitGadget, Oct 21, 2019
  22. 2/2 git_path(): handle `.lock` files correctlyJohannes Schindelin via GitGitGadget, Oct 21, 2019
  23. SZEDER GáborOct 22, 2019
  24. Junio C HamanoOct 23, 2019
  25. Johannes SchindelinOct 28, 2019
  26. SZEDER GáborOct 28, 2019
  27. Junio C HamanoOct 28, 2019
  28. 0/2 Handle git_path() with lock files correctly in worktreesJohannes Schindelin via GitGitGadget, Oct 28, 2019
  29. 1/2 t1400: wrap setup code in test caseJohannes Schindelin via GitGitGadget, Oct 28, 2019
  30. 2/2 git_path(): handle `.lock` files correctlyJohannes Schindelin via GitGitGadget, Oct 28, 2019
  31. Junio C HamanoOct 29, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.