All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/2] Handle git_path() with lock files correctly in worktrees
Date: Mon, 21 Oct 2019 21:54:40 +0000	[thread overview]
Message-ID: <pull.401.v3.git.1571694882.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.401.v2.git.1571350077.gitgitgadget@gmail.com>

I stumbled over this during my recent work in Git GUI
[https://github.com/gitgitgadget/git/pull/361] that was originally really
only intended to use the correct hooks directory.

It turns out that my fears that index.lock was mishandled were unfounded,
hence this patch series has a lot lower priority for me than "OMG we must
push this into v2.24.0!".

Technically, the first patch is not needed (because I decided against adding
a test to t1400 in the end, in favor of t1500), but it shouldn't hurt,
either.

Changes since v2:

 * Adjusted the commit message to really not talk about index.lock.
 * Instead of modifying the code inside trie_find() to special-case .lock, I
   now copy the string without the suffix .lock (if any) before handing it
   off to trie_find().

Changes since v1:

 * Clarified the commit message to state that index.lock is fine, it is 
   logs/HEAD.lock that isn't.

Johannes Schindelin (2):
  t1400: wrap setup code in test case
  git_path(): handle `.lock` files correctly

 path.c                |  8 +++++++-
 t/t1400-update-ref.sh | 18 ++++++++++--------
 t/t1500-rev-parse.sh  | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 9 deletions(-)


base-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-401/dscho/lock-files-in-worktrees-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/401

Range-diff vs v2:

 1:  cf97c5182e = 1:  cf97c5182e t1400: wrap setup code in test case
 2:  93dba5a3a3 ! 2:  8b1f4f089a git_path(): handle `.lock` files correctly
     @@ -4,12 +4,12 @@
      
          Ever since worktrees were introduced, the `git_path()` function _really_
          needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is
     -    specific to the worktree). However, the wrong path is returned for
     -    `logs/HEAD.lock`.
     +    specific to the worktree, and therefore so is its reflog). However, the
     +    wrong path is returned for `logs/HEAD.lock`.
      
          This does not matter as long as the Git executable is doing the asking,
     -    as the path for that `index.lock` file is constructed from
     -    `git_path("index")` by appending the `.lock` suffix.
     +    as the path for that `logs/HEAD.lock` file is constructed from
     +    `git_path("logs/HEAD")` 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
     @@ -19,7 +19,8 @@
          let's be safe rather than sorry.
      
          Side note: Git GUI _does_ ask for `index.lock`, but that is already
     -    resolved correctly.
     +    resolved correctly, due to `update_common_dir()` preferring to leave
     +    unknown paths in the (worktree-specific) git directory.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
     @@ -27,23 +28,23 @@
       --- a/path.c
       +++ b/path.c
      @@
     - 	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);
     -@@
     - 
     - 	/* Matched the entire compressed section */
     - 	key += i;
     --	if (!*key)
     -+	if (!*key || !strcmp(key, ".lock"))
     - 		/* End of key */
     - 		return fn(key, root->value, baton);
     + static void update_common_dir(struct strbuf *buf, int git_dir_len,
     + 			      const char *common_dir)
     + {
     +-	char *base = buf->buf + git_dir_len;
     ++	char *base = buf->buf + git_dir_len, *base2 = NULL;
     ++
     ++	if (ends_with(base, ".lock"))
     ++		base = base2 = xstrndup(base, strlen(base) - 5);
     ++
     + 	init_common_trie();
     + 	if (trie_find(&common_trie, base, check_common, NULL) > 0)
     + 		replace_dir(buf, git_dir_len, common_dir);
     ++
     ++	free(base2);
     + }
       
     + void report_linked_checkout_garbage(void)
      
       diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
       --- a/t/t1500-rev-parse.sh

-- 
gitgitgadget

  parent reply	other threads:[~2019-10-21 21:54 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-16  7:07 [PATCH 0/2] Handle git_path() with lock files correctly in worktrees Johannes Schindelin via GitGitGadget
2019-10-16  7:07 ` [PATCH 1/2] t1400: wrap setup code in test case Johannes Schindelin via GitGitGadget
2019-10-16  7:07 ` [PATCH 2/2] git_path(): handle `.lock` files correctly Johannes Schindelin via GitGitGadget
2019-10-16 11:04   ` SZEDER Gábor
2019-10-17  7:15     ` Junio C Hamano
2019-10-17 22:05     ` Johannes Schindelin
2019-10-18 11:06       ` SZEDER Gábor
2019-10-18 11:35         ` SZEDER Gábor
2019-10-21 16:00           ` [PATCH 0/5] path.c: a couple of common dir/trie fixes SZEDER Gábor
2019-10-21 16:00             ` [PATCH 1/5] Documentation: mention more worktree-specific exceptions SZEDER Gábor
2019-10-21 16:00             ` [PATCH 2/5] path.c: clarify trie_find()'s in-code comment SZEDER Gábor
2019-10-21 16:00             ` [PATCH 3/5] path.c: mark 'logs/HEAD' in 'common_list' as file SZEDER Gábor
2019-10-21 16:00             ` [PATCH 4/5] path.c: clarify two field names in 'struct common_dir' SZEDER Gábor
2019-10-21 16:00             ` [PATCH 5/5] path.c: don't call the match function without value in trie_find() SZEDER Gábor
2019-10-21 17:39               ` David Turner
2019-10-21 20:57               ` SZEDER Gábor
2019-10-23  4:01                 ` Junio C Hamano
2019-10-23 16:20                   ` SZEDER Gábor
2019-10-24  3:29                     ` Junio C Hamano
2019-10-28 10:57               ` Johannes Schindelin
2019-10-28 12:00                 ` SZEDER Gábor
2019-10-28 21:30                   ` Johannes Schindelin
2019-10-18 11:42         ` [PATCH 0/2] path.c: minor common_list fixes SZEDER Gábor
2019-10-18 11:42           ` [PATCH 1/2] path.c: fix field name in 'struct common_dir' SZEDER Gábor
2019-10-18 11:42           ` [PATCH 2/2] path.c: mark 'logs/HEAD' in 'common_list' as file SZEDER Gábor
2019-10-21 19:35           ` [PATCH 0/2] path.c: minor common_list fixes Johannes Schindelin
2019-10-17 22:07 ` [PATCH v2 0/2] Handle git_path() with lock files correctly in worktrees Johannes Schindelin via GitGitGadget
2019-10-17 22:07   ` [PATCH v2 1/2] t1400: wrap setup code in test case Johannes Schindelin via GitGitGadget
2019-10-17 22:07   ` [PATCH v2 2/2] git_path(): handle `.lock` files correctly Johannes Schindelin via GitGitGadget
2019-10-18  1:23     ` Junio C Hamano
2019-10-18 12:35       ` SZEDER Gábor
2019-10-21 20:26       ` Johannes Schindelin
2019-10-23  2:12         ` Junio C Hamano
2019-10-21 21:54   ` Johannes Schindelin via GitGitGadget [this message]
2019-10-21 21:54     ` [PATCH v3 1/2] t1400: wrap setup code in test case Johannes Schindelin via GitGitGadget
2019-10-21 21:54     ` [PATCH v3 2/2] git_path(): handle `.lock` files correctly Johannes Schindelin via GitGitGadget
2019-10-22 16:01       ` SZEDER Gábor
2019-10-23  3:38         ` Junio C Hamano
2019-10-28 12:01         ` Johannes Schindelin
2019-10-28 12:32           ` SZEDER Gábor
2019-10-28 17:30           ` Junio C Hamano
2019-10-28 12:57     ` [PATCH v4 0/2] Handle git_path() with lock files correctly in worktrees Johannes Schindelin via GitGitGadget
2019-10-28 12:57       ` [PATCH v4 1/2] t1400: wrap setup code in test case Johannes Schindelin via GitGitGadget
2019-10-28 12:57       ` [PATCH v4 2/2] git_path(): handle `.lock` files correctly Johannes Schindelin via GitGitGadget
2019-10-29  3:39       ` [PATCH v4 0/2] Handle git_path() with lock files correctly in worktrees Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.401.v3.git.1571694882.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.