Thread (34 messages) 34 messages, 4 authors, 2019-10-28

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

From: SZEDER Gábor <hidden>
Date: 2019-10-16 11:04:47

On Wed, Oct 16, 2019 at 07:07:17AM +0000, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <redacted>

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.
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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.
+'
+
 test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '
 	test_commit test_commit &&
 	echo true >expect &&
-- 
gitgitgadget
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help