{"thread":{"id":"52043","subject":"[PATCH 1/2] t1400: wrap setup code in test case","startedAt":"2019-10-16T07:07:23Z","lastAt":"2019-10-29T03:39:48Z","messageCount":31,"participants":["Johannes Schindelin via GitGitGadget","SZEDER Gábor","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"384166","messageId":"cf97c5182eb98cc0ae72f94d4331abcb4486ca83.1571209637.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.git.1571209637.gitgitgadget@gmail.com","subject":"[PATCH 1/2] t1400: wrap setup code in test case","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-16T07:07:16Z","receivedAt":"2019-10-16T07:07:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWithout this, you cannot use `--run=<...>` to skip that part, and a run\nwith `--run=0` (which is a common way to determine the test case number\ncorresponding to a given test case title).\n\nSigned-off-by: Johannes Schindelin \n---\n t/t1400-update-ref.sh | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 1fbd940408..69a7f27311 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -344,14 +344,16 @@ test_expect_success \"verifying $m's log (logged by config)\" '\n \ttest_cmp expect .git/logs/$m\n '\n \n-git update-ref $m $D\n-cat >.git/logs/$m < 1117150320 -0500\n-$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n-$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n-$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n-$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n-EOF\n+test_expect_success 'set up for querying the reflog' '\n+\tgit update-ref $m $D &&\n+\tcat >.git/logs/$m <<-EOF\n+\t$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n+\t$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n+\t$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n+\t$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n+\t$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n+\tEOF\n+'\n \n ed=\"Thu, 26 May 2005 18:32:00 -0500\"\n gd=\"Thu, 26 May 2005 18:33:00 -0500\"\n-- \ngitgitgadget\n\n"},{"id":"384168","messageId":"pull.401.git.1571209637.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":null,"subject":"[PATCH 0/2] Handle git_path() with lock files correctly in worktrees","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-16T07:07:15Z","receivedAt":"2019-10-16T07:07:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I stumbled over this during my recent work in Git GUI\n[https://github.com/gitgitgadget/git/pull/361] that was originally really\nonly intended to use the correct hooks directory.\n\nTechnically, the first patch is not needed (because I decided against adding\na test to t1400 in the end, in favor of t1500), but it shouldn't hurt,\neither.\n\nJohannes Schindelin (2):\n t1400: wrap setup code in test case\n git_path(): handle `.lock` files correctly\n\n path.c | 4 ++--\n t/t1400-update-ref.sh | 18 ++++++++++--------\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 3 files changed, 27 insertions(+), 10 deletions(-)\n\n\nbase-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-401/dscho/lock-files-in-worktrees-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/401\n-- \ngitgitgadget\n"},{"id":"384167","messageId":"f08c90ea02f6811b310a01301acf46da133f38a9.1571209637.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.git.1571209637.gitgitgadget@gmail.com","subject":"[PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-16T07:07:17Z","receivedAt":"2019-10-16T07:07:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since worktrees were introduced, the `git_path()` function _really_\nneeded to be called e.g. to get at the `index`. However, the wrong path\nis returned for `index.lock`.\n\nThis does not matter as long as the Git executable is doing the asking,\nas the path for that `index.lock` file is constructed from\n`git_path(\"index\")` by appending the `.lock` suffix.\n\nHowever, Git GUI just learned to use `--git-path` instead of appending\nrelative paths to what `git rev-parse --git-dir` returns (and as a\nconsequence not only using the correct hooks directory, but also using\nthe correct paths in worktrees other than the main one). And one of the\npaths it is looking for is... you guessed it... `index.lock`.\n\nSo let's make that work as script writers would expect it to.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 4 ++--\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e..ff85692b45 100644\n--- a/path.c\n+++ b/path.c\n@@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n \tint result;\n \tstruct trie *child;\n \n-\tif (!*key) {\n+\tif (!*key || !strcmp(key, \".lock\")) {\n \t\t/* we have reached the end of the key */\n \t\tif (root->value && !root->len)\n \t\t\treturn fn(key, root->value, baton);\n@@ -288,7 +288,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n \n \t/* Matched the entire compressed section */\n \tkey += i;\n-\tif (!*key)\n+\tif (!*key || !strcmp(key, \".lock\"))\n \t\t/* End of key */\n \t\treturn fn(key, root->value, baton);\n \ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 01abee533d..d318a1eeef 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git-path in worktree' '\n+\ttest_tick &&\n+\tgit commit --allow-empty -m empty &&\n+\tgit worktree add --detach wt &&\n+\ttest_write_lines >expect \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n+\tgit -C wt rev-parse >actual \\\n+\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n+\t\t--git-path index --git-path index.lock &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n \ttest_commit test_commit &&\n \techo true >expect &&\n-- \ngitgitgadget\n"},{"id":"384173","messageId":"20191016110440.GV29845@szeder.dev","threadId":"52043","inReplyTo":"f08c90ea02f6811b310a01301acf46da133f38a9.1571209637.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-16T11:04:40Z","receivedAt":"2019-10-16T11:04:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Oct 16, 2019 at 07:07:17AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Ever since worktrees were introduced, the `git_path()` function _really_\n> needed to be called e.g. to get at the `index`. However, the wrong path\n> is returned for `index.lock`.\n\nCould you give an example where it returns the wrong path for\n'index.lock'? I tried to reproduce this issue in a working tree, but\nno matter what I've tried, 'git rev-parse --git-dir index.lock' always\nreturned the right path.\n\n> This does not matter as long as the Git executable is doing the asking,\n> as the path for that `index.lock` file is constructed from\n> `git_path(\"index\")` by appending the `.lock` suffix.\n> \n> However, Git GUI just learned to use `--git-path` instead of appending\n> relative paths to what `git rev-parse --git-dir` returns (and as a\n> consequence not only using the correct hooks directory, but also using\n> the correct paths in worktrees other than the main one). And one of the\n> paths it is looking for is... you guessed it... `index.lock`.\n> \n> So let's make that work as script writers would expect it to.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> path.c | 4 ++--\n> t/t1500-rev-parse.sh | 15 +++++++++++++++\n> 2 files changed, 17 insertions(+), 2 deletions(-)\n> \n> diff --git a/path.c b/path.c\n> index e3da1f3c4e..ff85692b45 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> \tint result;\n> \tstruct trie *child;\n> \n> -\tif (!*key) {\n> +\tif (!*key || !strcmp(key, \".lock\")) {\n> \t\t/* we have reached the end of the key */\n> \t\tif (root->value && !root->len)\n> \t\t\treturn fn(key, root->value, baton);\n> @@ -288,7 +288,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> \n> \t/* Matched the entire compressed section */\n> \tkey += i;\n> -\tif (!*key)\n> +\tif (!*key || !strcmp(key, \".lock\"))\n> \t\t/* End of key */\n> \t\treturn fn(key, root->value, baton);\n> \n> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> index 01abee533d..d318a1eeef 100755\n> --- a/t/t1500-rev-parse.sh\n> +++ b/t/t1500-rev-parse.sh\n> @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git-path in worktree' '\n> +\ttest_tick &&\n> +\tgit commit --allow-empty -m empty &&\n> +\tgit worktree add --detach wt &&\n> +\ttest_write_lines >expect \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n> +\tgit -C wt rev-parse >actual \\\n> +\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n> +\t\t--git-path index --git-path index.lock &&\n> +\ttest_cmp expect actual\n\nWithout the fix applied this test fails with:\n\n + test_cmp expect actual\n --- expect 2019-10-16 10:20:31.047229423 +0000\n +++ actual 2019-10-16 10:20:31.051229519 +0000\n @@ -1,4 +1,4 @@\n /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD\n -/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD.lock\n +/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/logs/HEAD.lock\n /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index\n /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index.lock\n error: last command exited with $?=1\n\nSo the path of 'index.lock' seems to be fine already, it's the path of\nthe lockfile for HEAD's reflog that's indeed wrong and makes the test\nfail.\n\nOn a related note, I'm not sure whether the path of the reflogs\ndirectory is right while in a different working tree... Both with and\nwithout this patch I get a path pointing to the main working tree:\n\n $ ./git -C WT/ rev-parse --git-path logs\n /home/szeder/src/git/.git/logs\n\nHowever, I'm not sure what the right path should be in the first\nplace, given that each working tree has its own 'logs' directory, but\nonly for HEAD's reflog, while everything else goes to the main working\ntree's 'logs' directory.\n\n> +'\n> +\n> test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n> \ttest_commit test_commit &&\n> \techo true >expect &&\n> -- \n> gitgitgadget\n"},{"id":"384245","messageId":"xmqqzhhz7sh8.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"20191016110440.GV29845@szeder.dev","subject":"Re: [PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-17T07:15:31Z","receivedAt":"2019-10-17T07:17:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> However, I'm not sure what the right path should be in the first\n> place, given that each working tree has its own 'logs' directory, but\n> only for HEAD's reflog, while everything else goes to the main working\n> tree's 'logs' directory.\n\nAs all worktrees should share the same view of where 'master' (for\nexample) branch points at, what commit it was pointing at before,\netc., the reflogs should also be shared for refs. The exception is\nthe HEAD where each worktree can point at its own commit (when detached)\nor a branch (when not).\n\nThe above \"should\" does not mean \"I know the code works that way so\nif your git behaves differently your compiler is wrong\"; it just\nmeans \"that's the logical conclusion of the basic design, and if\nyour git does not behave that way, we have a bug (or two)\".\n\nThanks.\n"},{"id":"384297","messageId":"nycvar.QRO.7.76.6.1910172333360.46@tvgsbejvaqbjf.bet","threadId":"52043","inReplyTo":"20191016110440.GV29845@szeder.dev","subject":"Re: [PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-17T22:05:20Z","receivedAt":"2019-10-17T22:05:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Wed, 16 Oct 2019, SZEDER Gábor wrote:\n\n> On Wed, Oct 16, 2019 at 07:07:17AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > Ever since worktrees were introduced, the `git_path()` function _really_\n> > needed to be called e.g. to get at the `index`. However, the wrong path\n> > is returned for `index.lock`.\n>\n> Could you give an example where it returns the wrong path for\n> 'index.lock'?\n\nOh wow, this was a left-over from an early draft, before I got the\nregression test to work... What I meant was of course logs/HEAD.lock.\nWill fix.\n\n> I tried to reproduce this issue in a working tree, but\n> no matter what I've tried, 'git rev-parse --git-dir index.lock' always\n> returned the right path.\n\nWith `s/--git-dir/--git-path/`, I agree.\n\n> > This does not matter as long as the Git executable is doing the asking,\n> > as the path for that `index.lock` file is constructed from\n> > `git_path(\"index\")` by appending the `.lock` suffix.\n> >\n> > However, Git GUI just learned to use `--git-path` instead of appending\n> > relative paths to what `git rev-parse --git-dir` returns (and as a\n> > consequence not only using the correct hooks directory, but also using\n> > the correct paths in worktrees other than the main one). And one of the\n> > paths it is looking for is... you guessed it... `index.lock`.\n> >\n> > So let's make that work as script writers would expect it to.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > path.c | 4 ++--\n> > t/t1500-rev-parse.sh | 15 +++++++++++++++\n> > 2 files changed, 17 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/path.c b/path.c\n> > index e3da1f3c4e..ff85692b45 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> > \tint result;\n> > \tstruct trie *child;\n> >\n> > -\tif (!*key) {\n> > +\tif (!*key || !strcmp(key, \".lock\")) {\n> > \t\t/* we have reached the end of the key */\n> > \t\tif (root->value && !root->len)\n> > \t\t\treturn fn(key, root->value, baton);\n> > @@ -288,7 +288,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> >\n> > \t/* Matched the entire compressed section */\n> > \tkey += i;\n> > -\tif (!*key)\n> > +\tif (!*key || !strcmp(key, \".lock\"))\n> > \t\t/* End of key */\n> > \t\treturn fn(key, root->value, baton);\n> >\n> > diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> > index 01abee533d..d318a1eeef 100755\n> > --- a/t/t1500-rev-parse.sh\n> > +++ b/t/t1500-rev-parse.sh\n> > @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n> > \ttest_cmp expect actual\n> > '\n> >\n> > +test_expect_success 'git-path in worktree' '\n> > +\ttest_tick &&\n> > +\tgit commit --allow-empty -m empty &&\n> > +\tgit worktree add --detach wt &&\n> > +\ttest_write_lines >expect \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n> > +\tgit -C wt rev-parse >actual \\\n> > +\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n> > +\t\t--git-path index --git-path index.lock &&\n> > +\ttest_cmp expect actual\n>\n> Without the fix applied this test fails with:\n>\n> + test_cmp expect actual\n> --- expect 2019-10-16 10:20:31.047229423 +0000\n> +++ actual 2019-10-16 10:20:31.051229519 +0000\n> @@ -1,4 +1,4 @@\n> /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD\n> -/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/logs/HEAD.lock\n> +/home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/logs/HEAD.lock\n> /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index\n> /home/szeder/src/git/t/trash directory.t1500-rev-parse/.git/worktrees/wt/index.lock\n> error: last command exited with $?=1\n>\n> So the path of 'index.lock' seems to be fine already, it's the path of\n> the lockfile for HEAD's reflog that's indeed wrong and makes the test\n> fail.\n\nIndeed, and this makes this patch much less important than I previosly\nthought. It's not like it would break Git GUI in worktrees, which is\nwhat I thought, which in turn is the reason I sent this so close to\n-rc0.\n\n> On a related note, I'm not sure whether the path of the reflogs\n> directory is right while in a different working tree... Both with and\n> without this patch I get a path pointing to the main working tree:\n>\n> $ ./git -C WT/ rev-parse --git-path logs\n> /home/szeder/src/git/.git/logs\n>\n> However, I'm not sure what the right path should be in the first\n> place, given that each working tree has its own 'logs' directory, but\n> only for HEAD's reflog, while everything else goes to the main working\n> tree's 'logs' directory.\n\nIt's like Junio said, the reflog for `HEAD` is special because `HEAD` is\nspecial. Look for `common_list` in `path.c` (it is a bit confusing, I\nadmit, you have to look for the 3rd column of numbers: if it is a `1`,\nthen it is a worktree-specific path, if it is `0`, it is supposed to\nlive in the \"commondir\", i.e. in the gitdir of the main worktree).\n\nThanks,\nDscho\n\n>\n> > +'\n> > +\n> > test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n> > \ttest_commit test_commit &&\n> > \techo true >expect &&\n> > --\n> > gitgitgadget\n>\n"},{"id":"384298","messageId":"pull.401.v2.git.1571350077.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.git.1571209637.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Handle git_path() with lock files correctly in worktrees","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-17T22:07:55Z","receivedAt":"2019-10-17T22:08:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I stumbled over this during my recent work in Git GUI\n[https://github.com/gitgitgadget/git/pull/361] that was originally really\nonly intended to use the correct hooks directory.\n\nIt turns out that my fears that index.lock was mishandled were unfounded,\nhence this patch series has a lot lower priority for me than \"OMG we must\npush this into v2.24.0!\".\n\nTechnically, the first patch is not needed (because I decided against adding\na test to t1400 in the end, in favor of t1500), but it shouldn't hurt,\neither.\n\nChanges since v1:\n\n * Clarified the commit message to state that index.lock is fine, it is \n logs/HEAD.lock that isn't.\n\nJohannes Schindelin (2):\n t1400: wrap setup code in test case\n git_path(): handle `.lock` files correctly\n\n path.c | 4 ++--\n t/t1400-update-ref.sh | 18 ++++++++++--------\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 3 files changed, 27 insertions(+), 10 deletions(-)\n\n\nbase-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-401/dscho/lock-files-in-worktrees-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/401\n\nRange-diff vs v1:\n\n 1: cf97c5182e = 1: cf97c5182e t1400: wrap setup code in test case\n 2: f08c90ea02 ! 2: 93dba5a3a3 git_path(): handle `.lock` files correctly\n @@ -3,8 +3,9 @@\n git_path(): handle `.lock` files correctly\n \n Ever since worktrees were introduced, the `git_path()` function _really_\n - needed to be called e.g. to get at the `index`. However, the wrong path\n - is returned for `index.lock`.\n + needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n + specific to the worktree). However, the wrong path is returned for\n + `logs/HEAD.lock`.\n \n This does not matter as long as the Git executable is doing the asking,\n as the path for that `index.lock` file is constructed from\n @@ -13,10 +14,12 @@\n However, Git GUI just learned to use `--git-path` instead of appending\n relative paths to what `git rev-parse --git-dir` returns (and as a\n consequence not only using the correct hooks directory, but also using\n - the correct paths in worktrees other than the main one). And one of the\n - paths it is looking for is... you guessed it... `index.lock`.\n + the correct paths in worktrees other than the main one). While it does\n + not seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\n + let's be safe rather than sorry.\n \n - So let's make that work as script writers would expect it to.\n + Side note: Git GUI _does_ ask for `index.lock`, but that is already\n + resolved correctly.\n \n Signed-off-by: Johannes Schindelin \n \n\n-- \ngitgitgadget\n"},{"id":"384299","messageId":"cf97c5182eb98cc0ae72f94d4331abcb4486ca83.1571350077.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v2.git.1571350077.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] t1400: wrap setup code in test case","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-17T22:07:56Z","receivedAt":"2019-10-17T22:08:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWithout this, you cannot use `--run=<...>` to skip that part, and a run\nwith `--run=0` (which is a common way to determine the test case number\ncorresponding to a given test case title).\n\nSigned-off-by: Johannes Schindelin \n---\n t/t1400-update-ref.sh | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 1fbd940408..69a7f27311 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -344,14 +344,16 @@ test_expect_success \"verifying $m's log (logged by config)\" '\n \ttest_cmp expect .git/logs/$m\n '\n \n-git update-ref $m $D\n-cat >.git/logs/$m < 1117150320 -0500\n-$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n-$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n-$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n-$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n-EOF\n+test_expect_success 'set up for querying the reflog' '\n+\tgit update-ref $m $D &&\n+\tcat >.git/logs/$m <<-EOF\n+\t$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n+\t$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n+\t$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n+\t$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n+\t$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n+\tEOF\n+'\n \n ed=\"Thu, 26 May 2005 18:32:00 -0500\"\n gd=\"Thu, 26 May 2005 18:33:00 -0500\"\n-- \ngitgitgadget\n\n"},{"id":"384300","messageId":"93dba5a3a38d75ba79329383a1d50419a1990c8d.1571350077.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v2.git.1571350077.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-17T22:07:57Z","receivedAt":"2019-10-17T22:08:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since worktrees were introduced, the `git_path()` function _really_\nneeded to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\nspecific to the worktree). However, the wrong path is returned for\n`logs/HEAD.lock`.\n\nThis does not matter as long as the Git executable is doing the asking,\nas the path for that `index.lock` file is constructed from\n`git_path(\"index\")` by appending the `.lock` suffix.\n\nHowever, Git GUI just learned to use `--git-path` instead of appending\nrelative paths to what `git rev-parse --git-dir` returns (and as a\nconsequence not only using the correct hooks directory, but also using\nthe correct paths in worktrees other than the main one). While it does\nnot seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\nlet's be safe rather than sorry.\n\nSide note: Git GUI _does_ ask for `index.lock`, but that is already\nresolved correctly.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 4 ++--\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e..ff85692b45 100644\n--- a/path.c\n+++ b/path.c\n@@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n \tint result;\n \tstruct trie *child;\n \n-\tif (!*key) {\n+\tif (!*key || !strcmp(key, \".lock\")) {\n \t\t/* we have reached the end of the key */\n \t\tif (root->value && !root->len)\n \t\t\treturn fn(key, root->value, baton);\n@@ -288,7 +288,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n \n \t/* Matched the entire compressed section */\n \tkey += i;\n-\tif (!*key)\n+\tif (!*key || !strcmp(key, \".lock\"))\n \t\t/* End of key */\n \t\treturn fn(key, root->value, baton);\n \ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 01abee533d..d318a1eeef 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git-path in worktree' '\n+\ttest_tick &&\n+\tgit commit --allow-empty -m empty &&\n+\tgit worktree add --detach wt &&\n+\ttest_write_lines >expect \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n+\tgit -C wt rev-parse >actual \\\n+\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n+\t\t--git-path index --git-path index.lock &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n \ttest_commit test_commit &&\n \techo true >expect &&\n-- \ngitgitgadget\n"},{"id":"384332","messageId":"xmqq1rva7sp7.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"93dba5a3a38d75ba79329383a1d50419a1990c8d.1571350077.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] git_path(): handle `.lock` files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-18T01:23:00Z","receivedAt":"2019-10-18T01:23:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Ever since worktrees were introduced, the `git_path()` function _really_\n> needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n> specific to the worktree). However, the wrong path is returned for\n> `logs/HEAD.lock`.\n>\n> This does not matter as long as the Git executable is doing the asking,\n> as the path for that `index.lock` file is constructed from\n> `git_path(\"index\")` by appending the `.lock` suffix.\n\nIs this still git_path(\"index\") or is it now HEAD?\n\n> Side note: Git GUI _does_ ask for `index.lock`, but that is already\n> resolved correctly.\n\nIs that s/but/and/?\n\n> diff --git a/path.c b/path.c\n> index e3da1f3c4e..ff85692b45 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> \tint result;\n> \tstruct trie *child;\n> \n> -\tif (!*key) {\n> +\tif (!*key || !strcmp(key, \".lock\")) {\n\nWe only do strcmp for the tail part at the end of the path, so this\nshould probably OK from performance point of view but semantically\nit is not very satisfying to see a special case for a single .suffix\nthis deep in the callchain. I wonder if it is nicer to have the\nhigher level callers notice \".lock\" or whatever other suffixes they\ncare about and ask the lower layer for a key with the suffix\nstripped?\n\nWill queue.\n\nThanks.\n"},{"id":"384373","messageId":"20191018110618.GZ29845@szeder.dev","threadId":"52043","inReplyTo":"nycvar.QRO.7.76.6.1910172333360.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T11:06:18Z","receivedAt":"2019-10-18T11:06:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 18, 2019 at 12:05:20AM +0200, Johannes Schindelin wrote:\n> > I tried to reproduce this issue in a working tree, but\n> > no matter what I've tried, 'git rev-parse --git-dir index.lock' always\n> > returned the right path.\n> \n> With `s/--git-dir/--git-path/`, I agree.\n\nRight. I mistyped it a few times on the command line as well, but\nthen the command's output reminded me that I messed up. Alas, no such\nreminder when writing the email...\n\n> > On a related note, I'm not sure whether the path of the reflogs\n> > directory is right while in a different working tree... Both with and\n> > without this patch I get a path pointing to the main working tree:\n> >\n> > $ ./git -C WT/ rev-parse --git-path logs\n> > /home/szeder/src/git/.git/logs\n> >\n> > However, I'm not sure what the right path should be in the first\n> > place, given that each working tree has its own 'logs' directory, but\n> > only for HEAD's reflog, while everything else goes to the main working\n> > tree's 'logs' directory.\n> \n> It's like Junio said, the reflog for `HEAD` is special because `HEAD` is\n> special. Look for `common_list` in `path.c` (it is a bit confusing, I\n> admit, you have to look for the 3rd column of numbers: if it is a `1`,\n> then it is a worktree-specific path, if it is `0`, it is supposed to\n> live in the \"commondir\", i.e. in the gitdir of the main worktree).\n\nOK, got it.\n\nI didn't look yesterday at all, but now I did, and, unfortunately, see\ntwo more bugs, and one of them is a \"proper\" bug leading to bogus\noutput:\n\n $ git -C WT/ rev-parse --git-path logs/refs --git-path logs/refs/\n /home/szeder/src/git/.git/logs/refs\n /home/szeder/src/git/.git/worktrees/WT/logs/refs/\n\n"},{"id":"384374","messageId":"20191018113557.GA29845@szeder.dev","threadId":"52043","inReplyTo":"20191018110618.GZ29845@szeder.dev","subject":"Re: [PATCH 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T11:35:57Z","receivedAt":"2019-10-18T11:36:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 18, 2019 at 01:06:18PM +0200, SZEDER Gábor wrote:\n> > > On a related note, I'm not sure whether the path of the reflogs\n> > > directory is right while in a different working tree... Both with and\n> > > without this patch I get a path pointing to the main working tree:\n> > >\n> > > $ ./git -C WT/ rev-parse --git-path logs\n> > > /home/szeder/src/git/.git/logs\n> > >\n> > > However, I'm not sure what the right path should be in the first\n> > > place, given that each working tree has its own 'logs' directory, but\n> > > only for HEAD's reflog, while everything else goes to the main working\n> > > tree's 'logs' directory.\n> > \n> > It's like Junio said, the reflog for `HEAD` is special because `HEAD` is\n> > special. Look for `common_list` in `path.c` (it is a bit confusing, I\n> > admit, you have to look for the 3rd column of numbers: if it is a `1`,\n> > then it is a worktree-specific path, if it is `0`, it is supposed to\n> > live in the \"commondir\", i.e. in the gitdir of the main worktree).\n> \n> OK, got it.\n> \n> I didn't look yesterday at all, but now I did, and, unfortunately, see\n> two more bugs, and one of them is a \"proper\" bug leading to bogus\n> output:\n> \n> $ git -C WT/ rev-parse --git-path logs/refs --git-path logs/refs/\n> /home/szeder/src/git/.git/logs/refs\n> /home/szeder/src/git/.git/worktrees/WT/logs/refs/\n\nThis one-liner below fixes it, but I haven't yet made up my mind about\nwhether this is the right fix or whether there could be any fallout\n(at least the test suite doesn't show any).\n\n $ ./git -C WT/ rev-parse --git-path logs/refs --git-path logs/refs/\n /home/szeder/src/git/.git/logs/refs\n /home/szeder/src/git/.git/logs/refs/\n\n\ndiff --git a/path.c b/path.c\nindex 04b69b9feb..9019169418 100644\n--- a/path.c\n+++ b/path.c\n@@ -335,7 +335,7 @@ static int check_common(const char *unmatched, void *value, void *baton)\n \tstruct common_dir *dir = value;\n \n \tif (!dir)\n-\t\treturn 0;\n+\t\treturn -1;\n \n \tif (dir->is_dir && (unmatched[0] == 0 || unmatched[0] == '/'))\n \t\treturn !dir->exclude;\n\n"},{"id":"384375","messageId":"20191018114202.5452-2-szeder.dev@gmail.com","threadId":"52043","inReplyTo":"20191018114202.5452-1-szeder.dev@gmail.com","subject":"[PATCH 1/2] path.c: fix field name in 'struct common_dir'","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T11:42:01Z","receivedAt":"2019-10-18T11:42:11Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"An array of 'struct common_dir' instances is used to specify whether\nvarious paths in the '.git' directory are worktree-specific or belong\nto the main worktree. Confusingly, the path is recorded in the\nstruct's 'dirname' field, even though it can be a regular file, e.g.\n'gc.pid'.\n\nRename this 'dirname' field to 'path' to avoid future confusion.\n\nSigned-off-by: SZEDER Gábor \n---\n path.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e..eeb43e1d25 100644\n--- a/path.c\n+++ b/path.c\n@@ -103,7 +103,7 @@ struct common_dir {\n \tunsigned is_dir:1;\n \t/* Not common even though its parent is */\n \tunsigned exclude:1;\n-\tconst char *dirname;\n+\tconst char *path;\n };\n \n static struct common_dir common_list[] = {\n@@ -320,8 +320,8 @@ static void init_common_trie(void)\n \tif (common_trie_done_setup)\n \t\treturn;\n \n-\tfor (p = common_list; p->dirname; p++)\n-\t\tadd_to_trie(&common_trie, p->dirname, p);\n+\tfor (p = common_list; p->path; p++)\n+\t\tadd_to_trie(&common_trie, p->path, p);\n \n \tcommon_trie_done_setup = 1;\n }\n@@ -365,8 +365,8 @@ void report_linked_checkout_garbage(void)\n \t\treturn;\n \tstrbuf_addf(&sb, \"%s/\", get_git_dir());\n \tlen = sb.len;\n-\tfor (p = common_list; p->dirname; p++) {\n-\t\tconst char *path = p->dirname;\n+\tfor (p = common_list; p->path; p++) {\n+\t\tconst char *path = p->path;\n \t\tif (p->ignore_garbage)\n \t\t\tcontinue;\n \t\tstrbuf_setlen(&sb, len);\n-- \n2.23.0.1084.gae250eaa40\n\n"},{"id":"384376","messageId":"20191018114202.5452-3-szeder.dev@gmail.com","threadId":"52043","inReplyTo":"20191018114202.5452-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] path.c: mark 'logs/HEAD' in 'common_list' as file","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T11:42:02Z","receivedAt":"2019-10-18T11:42:13Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"'logs/HEAD', i.e. HEAD's reflog, is a file, but its entry in\n'common_list' has the 'is_dir' bit set.\n\nUnset that bit to make it consistent with what 'logs/HEAD' is supposed\nto be.\n\nThis doesn't make a difference in behavior: check_common() is the only\nfunction that looks at the 'is_dir' bit, and that function either\nreturns 0, or '!exclude', which for 'logs/HEAD' results in 0 as well.\n\nSigned-off-by: SZEDER Gábor \n---\n path.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/path.c b/path.c\nindex eeb43e1d25..04b69b9feb 100644\n--- a/path.c\n+++ b/path.c\n@@ -113,7 +113,7 @@ static struct common_dir common_list[] = {\n \t{ 0, 1, 0, \"info\" },\n \t{ 0, 0, 1, \"info/sparse-checkout\" },\n \t{ 1, 1, 0, \"logs\" },\n-\t{ 1, 1, 1, \"logs/HEAD\" },\n+\t{ 1, 0, 1, \"logs/HEAD\" },\n \t{ 0, 1, 1, \"logs/refs/bisect\" },\n \t{ 0, 1, 1, \"logs/refs/rewritten\" },\n \t{ 0, 1, 1, \"logs/refs/worktree\" },\n-- \n2.23.0.1084.gae250eaa40\n\n"},{"id":"384377","messageId":"20191018114202.5452-1-szeder.dev@gmail.com","threadId":"52043","inReplyTo":"20191018110618.GZ29845@szeder.dev","subject":"[PATCH 0/2] path.c: minor common_list fixes","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T11:42:00Z","receivedAt":"2019-10-18T11:42:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 18, 2019 at 01:06:18PM +0200, SZEDER Gábor wrote:\n> I didn't look yesterday at all, but now I did, and, unfortunately, see\n> two more bugs\n\nThe second patch is kind of a bugfix, though luckily the bug doesn't\nactually manifest in undesired behavior.\nThe first is a minor cleanup, so future readers won't have a 'Huh?'\nmoment like I just did.\n\nSZEDER Gábor (2):\n path.c: fix field name in 'struct common_dir'\n path.c: mark 'logs/HEAD' in 'common_list' as file\n\n path.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\n-- \n2.23.0.1084.gae250eaa40\n\n"},{"id":"384378","messageId":"20191018123504.GB29845@szeder.dev","threadId":"52043","inReplyTo":"xmqq1rva7sp7.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-18T12:35:04Z","receivedAt":"2019-10-18T12:35:12Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 18, 2019 at 10:23:00AM +0900, Junio C Hamano wrote:\n> > diff --git a/path.c b/path.c\n> > index e3da1f3c4e..ff85692b45 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> > \tint result;\n> > \tstruct trie *child;\n> > \n> > -\tif (!*key) {\n> > +\tif (!*key || !strcmp(key, \".lock\")) {\n> \n> We only do strcmp for the tail part at the end of the path, so this\n> should probably OK from performance point of view but semantically\n> it is not very satisfying to see a special case for a single .suffix\n> this deep in the callchain. I wonder if it is nicer to have the\n> higher level callers notice \".lock\" or whatever other suffixes they\n> care about and ask the lower layer for a key with the suffix\n> stripped?\n\nI was wondering the same. Despite all of its functions being file\nstatic, this is a fairly general trie implementation for paths, which\ncould potentially be useful in other parts of Git as well. Special\ncasing any suffix within its implementation makes it less (re)usable\nin other cases.\n\n"},{"id":"384556","messageId":"nycvar.QRO.7.76.6.1910212133540.46@tvgsbejvaqbjf.bet","threadId":"52043","inReplyTo":"20191018114202.5452-1-szeder.dev@gmail.com","subject":"Re: [PATCH 0/2] path.c: minor common_list fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-21T19:35:00Z","receivedAt":"2019-10-21T19:35:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Fri, 18 Oct 2019, SZEDER Gábor wrote:\n\n> On Fri, Oct 18, 2019 at 01:06:18PM +0200, SZEDER Gábor wrote:\n> > I didn't look yesterday at all, but now I did, and, unfortunately, see\n> > two more bugs\n>\n> The second patch is kind of a bugfix, though luckily the bug doesn't\n> actually manifest in undesired behavior.\n> The first is a minor cleanup, so future readers won't have a 'Huh?'\n> moment like I just did.\n>\n> SZEDER Gábor (2):\n> path.c: fix field name in 'struct common_dir'\n> path.c: mark 'logs/HEAD' in 'common_list' as file\n\nThose patches look very reasonable to me.\n\nThanks,\nDscho\n\n>\n> path.c | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> --\n> 2.23.0.1084.gae250eaa40\n>\n>\n"},{"id":"384562","messageId":"nycvar.QRO.7.76.6.1910212157350.46@tvgsbejvaqbjf.bet","threadId":"52043","inReplyTo":"xmqq1rva7sp7.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-21T20:26:26Z","receivedAt":"2019-10-21T20:26:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 18 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > Ever since worktrees were introduced, the `git_path()` function _really_\n> > needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n> > specific to the worktree). However, the wrong path is returned for\n> > `logs/HEAD.lock`.\n> >\n> > This does not matter as long as the Git executable is doing the asking,\n> > as the path for that `index.lock` file is constructed from\n> > `git_path(\"index\")` by appending the `.lock` suffix.\n>\n> Is this still git_path(\"index\") or is it now HEAD?\n\nDarn. It is \"logs/HEAD\", of course.\n\n> > Side note: Git GUI _does_ ask for `index.lock`, but that is already\n> > resolved correctly.\n>\n> Is that s/but/and/?\n\nIt sounds better with an \"and\", you're right.\n\n> > diff --git a/path.c b/path.c\n> > index e3da1f3c4e..ff85692b45 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -268,7 +268,7 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n> > \tint result;\n> > \tstruct trie *child;\n> >\n> > -\tif (!*key) {\n> > +\tif (!*key || !strcmp(key, \".lock\")) {\n>\n> We only do strcmp for the tail part at the end of the path, so this\n> should probably OK from performance point of view but semantically\n> it is not very satisfying to see a special case for a single .suffix\n> this deep in the callchain. I wonder if it is nicer to have the\n> higher level callers notice \".lock\" or whatever other suffixes they\n> care about and ask the lower layer for a key with the suffix\n> stripped?\n\nHmm. The parameter is provided as a `const char *`, so I cannot just\nedit it. My first attempt at fixing this resulted in this commit:\n\n-- snip --\n path: switch `trie_find()` to a counted string\n\n Before this patch, the `trie_find()` function would be fed a regular,\n NUL-terminated string.\n\n However, in the next commit, we want to strip any `.lock` suffix from\n the argument, and the argument is provided as a `const char *` (i.e. we\n do not want to modify it).\n\n Let's use a string + length pair of parameters instead of a single\n NUL-terminated string to open the door for this kind of manipulation.\n\n Signed-off-by: Johannes Schindelin \n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e2..b18d552c890 100644\n--- a/path.c\n+++ b/path.c\n@@ -236,7 +236,8 @@ static void *add_to_trie(struct trie *root, const char *key, void *value)\n \treturn old;\n }\n\n-typedef int (*match_fn)(const char *unmatched, void *data, void *baton);\n+typedef int (*match_fn)(const char *unmatched, size_t unmatched_len,\n+\t\t\tvoid *data, void *baton);\n\n /*\n * Search a trie for some key. Find the longest /-or-\\0-terminated\n@@ -261,22 +262,22 @@ typedef int (*match_fn)(const char *unmatched, void *data, void *baton);\n * |-----------------|------------|---------------|--------------|\n *\n */\n-static int trie_find(struct trie *root, const char *key, match_fn fn,\n-\t\t void *baton)\n+static int trie_find(struct trie *root, const char *key, size_t key_len,\n+\t\t match_fn fn, void *baton)\n {\n \tint i;\n \tint result;\n \tstruct trie *child;\n\n-\tif (!*key) {\n+\tif (!key_len) {\n \t\t/* we have reached the end of the key */\n \t\tif (root->value && !root->len)\n-\t\t\treturn fn(key, root->value, baton);\n+\t\t\treturn fn(key, key_len, root->value, baton);\n \t\telse\n \t\t\treturn -1;\n \t}\n\n-\tfor (i = 0; i < root->len; i++) {\n+\tfor (i = 0; i < key_len && i < root->len; i++) {\n \t\t/* Partial path normalization: skip consecutive slashes. */\n \t\tif (key[i] == '/' && key[i+1] == '/') {\n \t\t\tkey++;\n@@ -288,24 +289,26 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,\n\n \t/* Matched the entire compressed section */\n \tkey += i;\n-\tif (!*key)\n+\tkey_len -= i;\n+\n+\tif (!key_len)\n \t\t/* End of key */\n-\t\treturn fn(key, root->value, baton);\n+\t\treturn fn(key, key_len, root->value, baton);\n\n \t/* Partial path normalization: skip consecutive slashes */\n-\twhile (key[0] == '/' && key[1] == '/')\n-\t\tkey++;\n+\twhile (key_len > 0 && key[0] == '/' && key[1] == '/')\n+\t\tkey++, key_len--;\n\n-\tchild = root->children[(unsigned char)*key];\n+\tchild = root->children[!key_len ? '0' : (unsigned char)*key];\n \tif (child)\n-\t\tresult = trie_find(child, key + 1, fn, baton);\n+\t\tresult = trie_find(child, key + 1, key_len - 1, fn, baton);\n \telse\n \t\tresult = -1;\n\n-\tif (result >= 0 || (*key != '/' && *key != 0))\n+\tif (result >= 0 || (key_len && *key != '/'))\n \t\treturn result;\n \tif (root->value)\n-\t\treturn fn(key, root->value, baton);\n+\t\treturn fn(key, key_len, root->value, baton);\n \telse\n \t\treturn -1;\n }\n@@ -330,17 +333,18 @@ static void init_common_trie(void)\n * Helper function for update_common_dir: returns 1 if the dir\n * prefix is common.\n */\n-static int check_common(const char *unmatched, void *value, void *baton)\n+static int check_common(const char *unmatched, size_t unmatched_len,\n+\t\t\tvoid *value, void *baton)\n {\n \tstruct common_dir *dir = value;\n\n \tif (!dir)\n \t\treturn 0;\n\n-\tif (dir->is_dir && (unmatched[0] == 0 || unmatched[0] == '/'))\n+\tif (dir->is_dir && (!unmatched_len || unmatched[0] == '/'))\n \t\treturn !dir->exclude;\n\n-\tif (!dir->is_dir && unmatched[0] == 0)\n+\tif (!dir->is_dir && !unmatched_len)\n \t\treturn !dir->exclude;\n\n \treturn 0;\n@@ -350,8 +354,10 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len,\n \t\t\t const char *common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n+\tsize_t len = strlen(base);\n+\n \tinit_common_trie();\n-\tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n+\tif (trie_find(&common_trie, base, len, check_common, NULL) > 0)\n \t\treplace_dir(buf, git_dir_len, common_dir);\n }\n-- snap --\n\nHowever, I think this is _really_ ugly and intrusive. The opposite of\nwhat my goals were.\n\nSo I think I'll just bite the bullet and use a temporary copy if the\nargument ends in `.lock`.\n\nCiao,\nDscho\n"},{"id":"384567","messageId":"cf97c5182eb98cc0ae72f94d4331abcb4486ca83.1571694882.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v3.git.1571694882.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] t1400: wrap setup code in test case","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T21:54:41Z","receivedAt":"2019-10-21T21:54:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWithout this, you cannot use `--run=<...>` to skip that part, and a run\nwith `--run=0` (which is a common way to determine the test case number\ncorresponding to a given test case title).\n\nSigned-off-by: Johannes Schindelin \n---\n t/t1400-update-ref.sh | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 1fbd940408..69a7f27311 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -344,14 +344,16 @@ test_expect_success \"verifying $m's log (logged by config)\" '\n \ttest_cmp expect .git/logs/$m\n '\n \n-git update-ref $m $D\n-cat >.git/logs/$m < 1117150320 -0500\n-$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n-$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n-$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n-$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n-EOF\n+test_expect_success 'set up for querying the reflog' '\n+\tgit update-ref $m $D &&\n+\tcat >.git/logs/$m <<-EOF\n+\t$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n+\t$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n+\t$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n+\t$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n+\t$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n+\tEOF\n+'\n \n ed=\"Thu, 26 May 2005 18:32:00 -0500\"\n gd=\"Thu, 26 May 2005 18:33:00 -0500\"\n-- \ngitgitgadget\n\n"},{"id":"384568","messageId":"pull.401.v3.git.1571694882.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v2.git.1571350077.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] Handle git_path() with lock files correctly in worktrees","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T21:54:40Z","receivedAt":"2019-10-21T21:54:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I stumbled over this during my recent work in Git GUI\n[https://github.com/gitgitgadget/git/pull/361] that was originally really\nonly intended to use the correct hooks directory.\n\nIt turns out that my fears that index.lock was mishandled were unfounded,\nhence this patch series has a lot lower priority for me than \"OMG we must\npush this into v2.24.0!\".\n\nTechnically, the first patch is not needed (because I decided against adding\na test to t1400 in the end, in favor of t1500), but it shouldn't hurt,\neither.\n\nChanges since v2:\n\n * Adjusted the commit message to really not talk about index.lock.\n * Instead of modifying the code inside trie_find() to special-case .lock, I\n now copy the string without the suffix .lock (if any) before handing it\n off to trie_find().\n\nChanges since v1:\n\n * Clarified the commit message to state that index.lock is fine, it is \n logs/HEAD.lock that isn't.\n\nJohannes Schindelin (2):\n t1400: wrap setup code in test case\n git_path(): handle `.lock` files correctly\n\n path.c | 8 +++++++-\n t/t1400-update-ref.sh | 18 ++++++++++--------\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 3 files changed, 32 insertions(+), 9 deletions(-)\n\n\nbase-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-401/dscho/lock-files-in-worktrees-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/401\n\nRange-diff vs v2:\n\n 1: cf97c5182e = 1: cf97c5182e t1400: wrap setup code in test case\n 2: 93dba5a3a3 ! 2: 8b1f4f089a git_path(): handle `.lock` files correctly\n @@ -4,12 +4,12 @@\n \n Ever since worktrees were introduced, the `git_path()` function _really_\n needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n - specific to the worktree). However, the wrong path is returned for\n - `logs/HEAD.lock`.\n + specific to the worktree, and therefore so is its reflog). However, the\n + wrong path is returned for `logs/HEAD.lock`.\n \n This does not matter as long as the Git executable is doing the asking,\n - as the path for that `index.lock` file is constructed from\n - `git_path(\"index\")` by appending the `.lock` suffix.\n + as the path for that `logs/HEAD.lock` file is constructed from\n + `git_path(\"logs/HEAD\")` by appending the `.lock` suffix.\n \n However, Git GUI just learned to use `--git-path` instead of appending\n relative paths to what `git rev-parse --git-dir` returns (and as a\n @@ -19,7 +19,8 @@\n let's be safe rather than sorry.\n \n Side note: Git GUI _does_ ask for `index.lock`, but that is already\n - resolved correctly.\n + resolved correctly, due to `update_common_dir()` preferring to leave\n + unknown paths in the (worktree-specific) git directory.\n \n Signed-off-by: Johannes Schindelin \n \n @@ -27,23 +28,23 @@\n --- a/path.c\n +++ b/path.c\n @@\n - \tint result;\n - \tstruct trie *child;\n - \n --\tif (!*key) {\n -+\tif (!*key || !strcmp(key, \".lock\")) {\n - \t\t/* we have reached the end of the key */\n - \t\tif (root->value && !root->len)\n - \t\t\treturn fn(key, root->value, baton);\n -@@\n - \n - \t/* Matched the entire compressed section */\n - \tkey += i;\n --\tif (!*key)\n -+\tif (!*key || !strcmp(key, \".lock\"))\n - \t\t/* End of key */\n - \t\treturn fn(key, root->value, baton);\n + static void update_common_dir(struct strbuf *buf, int git_dir_len,\n + \t\t\t const char *common_dir)\n + {\n +-\tchar *base = buf->buf + git_dir_len;\n ++\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n ++\n ++\tif (ends_with(base, \".lock\"))\n ++\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n ++\n + \tinit_common_trie();\n + \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n + \t\treplace_dir(buf, git_dir_len, common_dir);\n ++\n ++\tfree(base2);\n + }\n \n + void report_linked_checkout_garbage(void)\n \n diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n --- a/t/t1500-rev-parse.sh\n\n-- \ngitgitgadget\n"},{"id":"384569","messageId":"8b1f4f089a6d4a2293507a1a77668c828598e84f.1571694882.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v3.git.1571694882.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T21:54:42Z","receivedAt":"2019-10-21T21:54:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since worktrees were introduced, the `git_path()` function _really_\nneeded to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\nspecific to the worktree, and therefore so is its reflog). However, the\nwrong path is returned for `logs/HEAD.lock`.\n\nThis does not matter as long as the Git executable is doing the asking,\nas the path for that `logs/HEAD.lock` file is constructed from\n`git_path(\"logs/HEAD\")` by appending the `.lock` suffix.\n\nHowever, Git GUI just learned to use `--git-path` instead of appending\nrelative paths to what `git rev-parse --git-dir` returns (and as a\nconsequence not only using the correct hooks directory, but also using\nthe correct paths in worktrees other than the main one). While it does\nnot seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\nlet's be safe rather than sorry.\n\nSide note: Git GUI _does_ ask for `index.lock`, but that is already\nresolved correctly, due to `update_common_dir()` preferring to leave\nunknown paths in the (worktree-specific) git directory.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 8 +++++++-\n t/t1500-rev-parse.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e..5ff64e7a8a 100644\n--- a/path.c\n+++ b/path.c\n@@ -349,10 +349,16 @@ static int check_common(const char *unmatched, void *value, void *baton)\n static void update_common_dir(struct strbuf *buf, int git_dir_len,\n \t\t\t const char *common_dir)\n {\n-\tchar *base = buf->buf + git_dir_len;\n+\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n+\n+\tif (ends_with(base, \".lock\"))\n+\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n+\n \tinit_common_trie();\n \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n \t\treplace_dir(buf, git_dir_len, common_dir);\n+\n+\tfree(base2);\n }\n \n void report_linked_checkout_garbage(void)\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 01abee533d..d318a1eeef 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git-path in worktree' '\n+\ttest_tick &&\n+\tgit commit --allow-empty -m empty &&\n+\tgit worktree add --detach wt &&\n+\ttest_write_lines >expect \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n+\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n+\tgit -C wt rev-parse >actual \\\n+\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n+\t\t--git-path index --git-path index.lock &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n \ttest_commit test_commit &&\n \techo true >expect &&\n-- \ngitgitgadget\n"},{"id":"384610","messageId":"20191022160105.GE4348@szeder.dev","threadId":"52043","inReplyTo":"8b1f4f089a6d4a2293507a1a77668c828598e84f.1571694882.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-22T16:01:05Z","receivedAt":"2019-10-22T16:01:14Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 21, 2019 at 09:54:42PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Ever since worktrees were introduced, the `git_path()` function _really_\n> needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n> specific to the worktree, and therefore so is its reflog). However, the\n> wrong path is returned for `logs/HEAD.lock`.\n> \n> This does not matter as long as the Git executable is doing the asking,\n> as the path for that `logs/HEAD.lock` file is constructed from\n> `git_path(\"logs/HEAD\")` by appending the `.lock` suffix.\n> \n> However, Git GUI just learned to use `--git-path` instead of appending\n> relative paths to what `git rev-parse --git-dir` returns (and as a\n> consequence not only using the correct hooks directory, but also using\n> the correct paths in worktrees other than the main one). While it does\n> not seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\n> let's be safe rather than sorry.\n> \n> Side note: Git GUI _does_ ask for `index.lock`, but that is already\n> resolved correctly, due to `update_common_dir()` preferring to leave\n> unknown paths in the (worktree-specific) git directory.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> path.c | 8 +++++++-\n> t/t1500-rev-parse.sh | 15 +++++++++++++++\n> 2 files changed, 22 insertions(+), 1 deletion(-)\n> \n> diff --git a/path.c b/path.c\n> index e3da1f3c4e..5ff64e7a8a 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -349,10 +349,16 @@ static int check_common(const char *unmatched, void *value, void *baton)\n> static void update_common_dir(struct strbuf *buf, int git_dir_len,\n> \t\t\t const char *common_dir)\n> {\n> -\tchar *base = buf->buf + git_dir_len;\n> +\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n> +\n> +\tif (ends_with(base, \".lock\"))\n> +\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n\nHm, this adds the magic number 5 and calls strlen(base) twice, because\nends_with() calls strip_suffix(), which calls strlen(). Calling\nstrip_suffix() directly would allow us to avoid the magic number and\nthe second strlen():\n\n size_t len;\n if (strip_suffix(base, \".lock\", &len))\n base = base2 = xstrndup(base, len);\n\n> \tinit_common_trie();\n> \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n> \t\treplace_dir(buf, git_dir_len, common_dir);\n> +\n> +\tfree(base2);\n> }\n> \n> void report_linked_checkout_garbage(void)\n> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> index 01abee533d..d318a1eeef 100755\n> --- a/t/t1500-rev-parse.sh\n> +++ b/t/t1500-rev-parse.sh\n> @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git-path in worktree' '\n> +\ttest_tick &&\n> +\tgit commit --allow-empty -m empty &&\n> +\tgit worktree add --detach wt &&\n> +\ttest_write_lines >expect \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n> +\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n> +\tgit -C wt rev-parse >actual \\\n> +\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n> +\t\t--git-path index --git-path index.lock &&\n> +\ttest_cmp expect actual\n> +'\n\nI think this test better fits into 't0060-path-utils.sh': it already\nchecks 'logs/HEAD' and 'index' in a different working tree (well, with\nGIT_COMMON_DIR set, but that's the same), and it has a helper function\nto make each of the two new .lock tests a one-liner.\n\n> test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n> \ttest_commit test_commit &&\n> \techo true >expect &&\n> -- \n> gitgitgadget\n"},{"id":"384662","messageId":"xmqqd0eojjll.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"nycvar.QRO.7.76.6.1910212157350.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 2/2] git_path(): handle `.lock` files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-23T02:12:22Z","receivedAt":"2019-10-23T02:12:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> However, I think this is _really_ ugly and intrusive. The opposite of\n> what my goals were.\n>\n> So I think I'll just bite the bullet and use a temporary copy if the\n> argument ends in `.lock`.\n\nSounds like a quite sensible design decision to me.\n"},{"id":"384668","messageId":"xmqqmudsi10s.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"20191022160105.GE4348@szeder.dev","subject":"Re: [PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-23T03:38:59Z","receivedAt":"2019-10-23T03:39:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> +\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n>> +\n>> +\tif (ends_with(base, \".lock\"))\n>> +\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n>\n> Hm, this adds the magic number 5 and calls strlen(base) twice, because\n> ends_with() calls strip_suffix(), which calls strlen(). Calling\n> strip_suffix() directly would allow us to avoid the magic number and\n> the second strlen():\n>\n> size_t len;\n> if (strip_suffix(base, \".lock\", &len))\n> base = base2 = xstrndup(base, len);\n\nMakes sense, and is easy to squash in.\n"},{"id":"384982","messageId":"nycvar.QRO.7.76.6.1910281225260.46@tvgsbejvaqbjf.bet","threadId":"52043","inReplyTo":"20191022160105.GE4348@szeder.dev","subject":"Re: [PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-28T12:01:49Z","receivedAt":"2019-10-28T12:02:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 22 Oct 2019, SZEDER Gábor wrote:\n\n> On Mon, Oct 21, 2019 at 09:54:42PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > Ever since worktrees were introduced, the `git_path()` function _really_\n> > needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\n> > specific to the worktree, and therefore so is its reflog). However, the\n> > wrong path is returned for `logs/HEAD.lock`.\n> >\n> > This does not matter as long as the Git executable is doing the asking,\n> > as the path for that `logs/HEAD.lock` file is constructed from\n> > `git_path(\"logs/HEAD\")` by appending the `.lock` suffix.\n> >\n> > However, Git GUI just learned to use `--git-path` instead of appending\n> > relative paths to what `git rev-parse --git-dir` returns (and as a\n> > consequence not only using the correct hooks directory, but also using\n> > the correct paths in worktrees other than the main one). While it does\n> > not seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\n> > let's be safe rather than sorry.\n> >\n> > Side note: Git GUI _does_ ask for `index.lock`, but that is already\n> > resolved correctly, due to `update_common_dir()` preferring to leave\n> > unknown paths in the (worktree-specific) git directory.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > path.c | 8 +++++++-\n> > t/t1500-rev-parse.sh | 15 +++++++++++++++\n> > 2 files changed, 22 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/path.c b/path.c\n> > index e3da1f3c4e..5ff64e7a8a 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -349,10 +349,16 @@ static int check_common(const char *unmatched, void *value, void *baton)\n> > static void update_common_dir(struct strbuf *buf, int git_dir_len,\n> > \t\t\t const char *common_dir)\n> > {\n> > -\tchar *base = buf->buf + git_dir_len;\n> > +\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n> > +\n> > +\tif (ends_with(base, \".lock\"))\n> > +\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n>\n> Hm, this adds the magic number 5 and calls strlen(base) twice, because\n> ends_with() calls strip_suffix(), which calls strlen(). Calling\n> strip_suffix() directly would allow us to avoid the magic number and\n> the second strlen():\n>\n> size_t len;\n> if (strip_suffix(base, \".lock\", &len))\n> base = base2 = xstrndup(base, len);\n\nActually, we should probably avoid the extra allocation. Earlier, I was\nconcerned about multi-threading issues when attempting to modify the\n`strbuf`. But then, we modify that `strbuf` a couple of lines later\nanyway, so my fears were totally unfounded. Therefore, my current\nversion reads like this:\n\n-- snip --\n\tint has_lock_suffix = strbuf_strip_suffix(buf, LOCK_SUFFIX);\n\n\tinit_common_trie();\n\tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n\t\treplace_dir(buf, git_dir_len, common_dir);\n\n\tif (has_lock_suffix)\n\t\tstrbuf_addstr(buf, LOCK_SUFFIX);\n-- snap --\n\n\n>\n> > \tinit_common_trie();\n> > \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n> > \t\treplace_dir(buf, git_dir_len, common_dir);\n> > +\n> > +\tfree(base2);\n> > }\n> >\n> > void report_linked_checkout_garbage(void)\n> > diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> > index 01abee533d..d318a1eeef 100755\n> > --- a/t/t1500-rev-parse.sh\n> > +++ b/t/t1500-rev-parse.sh\n> > @@ -116,6 +116,21 @@ test_expect_success 'git-path inside sub-dir' '\n> > \ttest_cmp expect actual\n> > '\n> >\n> > +test_expect_success 'git-path in worktree' '\n> > +\ttest_tick &&\n> > +\tgit commit --allow-empty -m empty &&\n> > +\tgit worktree add --detach wt &&\n> > +\ttest_write_lines >expect \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n> > +\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n> > +\tgit -C wt rev-parse >actual \\\n> > +\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n> > +\t\t--git-path index --git-path index.lock &&\n> > +\ttest_cmp expect actual\n> > +'\n>\n> I think this test better fits into 't0060-path-utils.sh': it already\n> checks 'logs/HEAD' and 'index' in a different working tree (well, with\n> GIT_COMMON_DIR set, but that's the same), and it has a helper function\n> to make each of the two new .lock tests a one-liner.\n\nExcellent point. Thank you for helping me improve the patch!\n\nCiao,\nDscho\n\n>\n> > test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n> > \ttest_commit test_commit &&\n> > \techo true >expect &&\n> > --\n> > gitgitgadget\n>\n"},{"id":"384983","messageId":"20191028123214.GT4348@szeder.dev","threadId":"52043","inReplyTo":"nycvar.QRO.7.76.6.1910281225260.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-28T12:32:14Z","receivedAt":"2019-10-28T12:32:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Oct 28, 2019 at 01:01:49PM +0100, Johannes Schindelin wrote:\n> > > @@ -349,10 +349,16 @@ static int check_common(const char *unmatched, void *value, void *baton)\n> > > static void update_common_dir(struct strbuf *buf, int git_dir_len,\n> > > \t\t\t const char *common_dir)\n> > > {\n> > > -\tchar *base = buf->buf + git_dir_len;\n> > > +\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n> > > +\n> > > +\tif (ends_with(base, \".lock\"))\n> > > +\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n> >\n> > Hm, this adds the magic number 5 and calls strlen(base) twice, because\n> > ends_with() calls strip_suffix(), which calls strlen(). Calling\n> > strip_suffix() directly would allow us to avoid the magic number and\n> > the second strlen():\n> >\n> > size_t len;\n> > if (strip_suffix(base, \".lock\", &len))\n> > base = base2 = xstrndup(base, len);\n> \n> Actually, we should probably avoid the extra allocation. Earlier, I was\n> concerned about multi-threading issues when attempting to modify the\n> `strbuf`. But then, we modify that `strbuf` a couple of lines later\n> anyway,\n\nDo we? You're right, we do indeed, because in replace_dir(buf, ...)\nwe call strbuf_splice(buf, ...).\nOK, then your suggestion below is even better.\n\n> so my fears were totally unfounded. Therefore, my current\n> version reads like this:\n> \n> -- snip --\n> \tint has_lock_suffix = strbuf_strip_suffix(buf, LOCK_SUFFIX);\n> \n> \tinit_common_trie();\n> \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n> \t\treplace_dir(buf, git_dir_len, common_dir);\n> \n> \tif (has_lock_suffix)\n> \t\tstrbuf_addstr(buf, LOCK_SUFFIX);\n> -- snap --\n"},{"id":"384984","messageId":"pull.401.v4.git.1572267438.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v3.git.1571694882.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] Handle git_path() with lock files correctly in worktrees","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T12:57:16Z","receivedAt":"2019-10-28T12:57:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I stumbled over this during my recent work in Git GUI\n[https://github.com/gitgitgadget/git/pull/361] that was originally really\nonly intended to use the correct hooks directory.\n\nIt turns out that my fears that index.lock was mishandled were unfounded,\nhence this patch series has a lot lower priority for me than \"OMG we must\npush this into v2.24.0!\".\n\nTechnically, the first patch is not needed (because I decided against adding\na test to t1400 in the end, in favor of t1500), but it shouldn't hurt,\neither.\n\nI verified locally that this patch pair does not conflict with \nsg/dir-trie-fixes.\n\nChanges since v3:\n\n * Instead of duplicating the path when it has a .lock suffix, we now use \n strbuf manipulation to strip the suffix temporarily.\n * The test case in t1500 was scrapped in favor of a much simpler pair of\n test cases in t0060.\n\nChanges since v2:\n\n * Adjusted the commit message to really not talk about index.lock.\n * Instead of modifying the code inside trie_find() to special-case .lock, I\n now copy the string without the suffix .lock (if any) before handing it\n off to trie_find().\n\nChanges since v1:\n\n * Clarified the commit message to state that index.lock is fine, it is \n logs/HEAD.lock that isn't.\n\nJohannes Schindelin (2):\n t1400: wrap setup code in test case\n git_path(): handle `.lock` files correctly\n\n path.c | 6 ++++++\n t/t0060-path-utils.sh | 2 ++\n t/t1400-update-ref.sh | 18 ++++++++++--------\n 3 files changed, 18 insertions(+), 8 deletions(-)\n\n\nbase-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-401/dscho/lock-files-in-worktrees-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/401\n\nRange-diff vs v3:\n\n 1: cf97c5182e = 1: cf97c5182e t1400: wrap setup code in test case\n 2: 8b1f4f089a ! 2: d98810875e git_path(): handle `.lock` files correctly\n @@ -28,46 +28,40 @@\n --- a/path.c\n +++ b/path.c\n @@\n - static void update_common_dir(struct strbuf *buf, int git_dir_len,\n + #include \"path.h\"\n + #include \"packfile.h\"\n + #include \"object-store.h\"\n ++#include \"lockfile.h\"\n + \n + static int get_st_mode_bits(const char *path, int *mode)\n + {\n +@@\n \t\t\t const char *common_dir)\n {\n --\tchar *base = buf->buf + git_dir_len;\n -+\tchar *base = buf->buf + git_dir_len, *base2 = NULL;\n -+\n -+\tif (ends_with(base, \".lock\"))\n -+\t\tbase = base2 = xstrndup(base, strlen(base) - 5);\n + \tchar *base = buf->buf + git_dir_len;\n ++\tint has_lock_suffix = strbuf_strip_suffix(buf, LOCK_SUFFIX);\n +\n \tinit_common_trie();\n \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n \t\treplace_dir(buf, git_dir_len, common_dir);\n +\n -+\tfree(base2);\n ++\tif (has_lock_suffix)\n ++\t\tstrbuf_addstr(buf, LOCK_SUFFIX);\n }\n \n void report_linked_checkout_garbage(void)\n \n - diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n - --- a/t/t1500-rev-parse.sh\n - +++ b/t/t1500-rev-parse.sh\n + diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n + --- a/t/t0060-path-utils.sh\n + +++ b/t/t0060-path-utils.sh\n @@\n - \ttest_cmp expect actual\n - '\n - \n -+test_expect_success 'git-path in worktree' '\n -+\ttest_tick &&\n -+\tgit commit --allow-empty -m empty &&\n -+\tgit worktree add --detach wt &&\n -+\ttest_write_lines >expect \\\n -+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD\" \\\n -+\t\t\"$(pwd)/.git/worktrees/wt/logs/HEAD.lock\" \\\n -+\t\t\"$(pwd)/.git/worktrees/wt/index\" \\\n -+\t\t\"$(pwd)/.git/worktrees/wt/index.lock\" &&\n -+\tgit -C wt rev-parse >actual \\\n -+\t\t--git-path logs/HEAD --git-path logs/HEAD.lock \\\n -+\t\t--git-path index --git-path index.lock &&\n -+\ttest_cmp expect actual\n -+'\n -+\n - test_expect_success 'rev-parse --is-shallow-repository in shallow repo' '\n - \ttest_commit test_commit &&\n - \techo true >expect &&\n + test_git_path GIT_OBJECT_DIRECTORY=foo objects2 .git/objects2\n + test_expect_success 'setup common repository' 'git --git-dir=bar init'\n + test_git_path GIT_COMMON_DIR=bar index .git/index\n ++test_git_path GIT_COMMON_DIR=bar index.lock .git/index.lock\n + test_git_path GIT_COMMON_DIR=bar HEAD .git/HEAD\n + test_git_path GIT_COMMON_DIR=bar logs/HEAD .git/logs/HEAD\n ++test_git_path GIT_COMMON_DIR=bar logs/HEAD.lock .git/logs/HEAD.lock\n + test_git_path GIT_COMMON_DIR=bar logs/refs/bisect/foo .git/logs/refs/bisect/foo\n + test_git_path GIT_COMMON_DIR=bar logs/refs/bisec/foo bar/logs/refs/bisec/foo\n + test_git_path GIT_COMMON_DIR=bar logs/refs/bisec bar/logs/refs/bisec\n\n-- \ngitgitgadget\n"},{"id":"384986","messageId":"cf97c5182eb98cc0ae72f94d4331abcb4486ca83.1572267438.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v4.git.1572267438.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] t1400: wrap setup code in test case","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T12:57:17Z","receivedAt":"2019-10-28T12:57:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWithout this, you cannot use `--run=<...>` to skip that part, and a run\nwith `--run=0` (which is a common way to determine the test case number\ncorresponding to a given test case title).\n\nSigned-off-by: Johannes Schindelin \n---\n t/t1400-update-ref.sh | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 1fbd940408..69a7f27311 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -344,14 +344,16 @@ test_expect_success \"verifying $m's log (logged by config)\" '\n \ttest_cmp expect .git/logs/$m\n '\n \n-git update-ref $m $D\n-cat >.git/logs/$m < 1117150320 -0500\n-$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n-$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n-$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n-$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n-EOF\n+test_expect_success 'set up for querying the reflog' '\n+\tgit update-ref $m $D &&\n+\tcat >.git/logs/$m <<-EOF\n+\t$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n+\t$C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n+\t$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n+\t$F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n+\t$Z $E $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 -0500\n+\tEOF\n+'\n \n ed=\"Thu, 26 May 2005 18:32:00 -0500\"\n gd=\"Thu, 26 May 2005 18:33:00 -0500\"\n-- \ngitgitgadget\n\n"},{"id":"384985","messageId":"d98810875ee4a085e84bf00836ce198a7d64d1a2.1572267438.git.gitgitgadget@gmail.com","threadId":"52043","inReplyTo":"pull.401.v4.git.1572267438.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] git_path(): handle `.lock` files correctly","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T12:57:18Z","receivedAt":"2019-10-28T12:57:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since worktrees were introduced, the `git_path()` function _really_\nneeded to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is\nspecific to the worktree, and therefore so is its reflog). However, the\nwrong path is returned for `logs/HEAD.lock`.\n\nThis does not matter as long as the Git executable is doing the asking,\nas the path for that `logs/HEAD.lock` file is constructed from\n`git_path(\"logs/HEAD\")` by appending the `.lock` suffix.\n\nHowever, Git GUI just learned to use `--git-path` instead of appending\nrelative paths to what `git rev-parse --git-dir` returns (and as a\nconsequence not only using the correct hooks directory, but also using\nthe correct paths in worktrees other than the main one). While it does\nnot seem as if Git GUI in particular is asking for `logs/HEAD.lock`,\nlet's be safe rather than sorry.\n\nSide note: Git GUI _does_ ask for `index.lock`, but that is already\nresolved correctly, due to `update_common_dir()` preferring to leave\nunknown paths in the (worktree-specific) git directory.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 6 ++++++\n t/t0060-path-utils.sh | 2 ++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/path.c b/path.c\nindex e3da1f3c4e..198a867017 100644\n--- a/path.c\n+++ b/path.c\n@@ -11,6 +11,7 @@\n #include \"path.h\"\n #include \"packfile.h\"\n #include \"object-store.h\"\n+#include \"lockfile.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -350,9 +351,14 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len,\n \t\t\t const char *common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n+\tint has_lock_suffix = strbuf_strip_suffix(buf, LOCK_SUFFIX);\n+\n \tinit_common_trie();\n \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n \t\treplace_dir(buf, git_dir_len, common_dir);\n+\n+\tif (has_lock_suffix)\n+\t\tstrbuf_addstr(buf, LOCK_SUFFIX);\n }\n \n void report_linked_checkout_garbage(void)\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex c7b53e494b..2aca8ccff9 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -285,8 +285,10 @@ test_git_path GIT_OBJECT_DIRECTORY=foo objects/foo foo/foo\n test_git_path GIT_OBJECT_DIRECTORY=foo objects2 .git/objects2\n test_expect_success 'setup common repository' 'git --git-dir=bar init'\n test_git_path GIT_COMMON_DIR=bar index .git/index\n+test_git_path GIT_COMMON_DIR=bar index.lock .git/index.lock\n test_git_path GIT_COMMON_DIR=bar HEAD .git/HEAD\n test_git_path GIT_COMMON_DIR=bar logs/HEAD .git/logs/HEAD\n+test_git_path GIT_COMMON_DIR=bar logs/HEAD.lock .git/logs/HEAD.lock\n test_git_path GIT_COMMON_DIR=bar logs/refs/bisect/foo .git/logs/refs/bisect/foo\n test_git_path GIT_COMMON_DIR=bar logs/refs/bisec/foo bar/logs/refs/bisec/foo\n test_git_path GIT_COMMON_DIR=bar logs/refs/bisec bar/logs/refs/bisec\n-- \ngitgitgadget\n"},{"id":"385009","messageId":"xmqqtv7s93rp.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"nycvar.QRO.7.76.6.1910281225260.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 2/2] git_path(): handle `.lock` files correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-28T17:30:18Z","receivedAt":"2019-10-28T17:34:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> size_t len;\n>> if (strip_suffix(base, \".lock\", &len))\n>> base = base2 = xstrndup(base, len);\n>\n> Actually, we should probably avoid the extra allocation. Earlier, I was\n> concerned about multi-threading issues when attempting to modify the\n> `strbuf`. But then, we modify that `strbuf` a couple of lines later\n> anyway, so my fears were totally unfounded. Therefore, my current\n> version reads like this:\n>\n> -- snip --\n> \tint has_lock_suffix = strbuf_strip_suffix(buf, LOCK_SUFFIX);\n>\n> \tinit_common_trie();\n> \tif (trie_find(&common_trie, base, check_common, NULL) > 0)\n> \t\treplace_dir(buf, git_dir_len, common_dir);\n>\n> \tif (has_lock_suffix)\n> \t\tstrbuf_addstr(buf, LOCK_SUFFIX);\n> -- snap --\n\nMakes sense. Thanks for thinking it through.\n"},{"id":"385045","messageId":"xmqqy2x45if3.fsf@gitster-ct.c.googlers.com","threadId":"52043","inReplyTo":"pull.401.v4.git.1572267438.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 0/2] Handle git_path() with lock files correctly in worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-29T03:39:44Z","receivedAt":"2019-10-29T03:39:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Changes since v3:\n>\n> * Instead of duplicating the path when it has a .lock suffix, we now use \n> strbuf manipulation to strip the suffix temporarily.\n> * The test case in t1500 was scrapped in favor of a much simpler pair of\n> test cases in t0060.\n\nMakes sense. Will replace.\n\nThanks.\n"}]}