All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: shaheedhaque@gmail.com
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH] worktree add: be tolerant of corrupt worktrees
Date: Mon, 13 May 2019 17:49:44 +0700	[thread overview]
Message-ID: <20190513104944.20367-1-pclouds@gmail.com> (raw)
In-Reply-To: <CAHAc2je-Yz4oej-sqvp+G+2Wv+eBABeJWUMm4scRwF2z_diUXw@mail.gmail.com>

find_worktree() can die() unexpectedly because it uses real_path()
instead of the gentler version. When it's used in 'git worktree add' [1]
and there's a bad worktree, this die() could prevent people from adding
new worktrees.

The "bad" condition to trigger this is when a parent of the worktree's
location is deleted. Then real_path() will complain.

Use the other version so that bad worktrees won't affect 'worktree
add'. The bad ones will eventually be pruned, we just have to tolerate
them for a bit.

[1] added in cb56f55c16 (worktree: disallow adding same path multiple
    times, 2018-08-28), or since v2.20.0. Though the real bug in
    find_worktree() is much older.

Reported-by: Shaheed Haque <shaheedhaque@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t2025-worktree-add.sh | 12 ++++++++++++
 worktree.c              |  7 +++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 286bba35d8..d83a9f0fdc 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -570,4 +570,16 @@ test_expect_success '"add" an existing locked but missing worktree' '
 	git worktree add --force --force --detach gnoo
 '
 
+test_expect_success '"add" should not fail because of another bad worktree' '
+	git init add-fail &&
+	(
+		cd add-fail &&
+		test_commit first &&
+		mkdir sub &&
+		git worktree add sub/to-be-deleted &&
+		rm -rf sub &&
+		git worktree add second
+	)
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index d6a0ee7f73..c79b3e42bb 100644
--- a/worktree.c
+++ b/worktree.c
@@ -222,9 +222,12 @@ struct worktree *find_worktree(struct worktree **list,
 		free(to_free);
 		return NULL;
 	}
-	for (; *list; list++)
-		if (!fspathcmp(path, real_path((*list)->path)))
+	for (; *list; list++) {
+		const char *wt_path = real_path_if_valid((*list)->path);
+
+		if (wt_path && !fspathcmp(path, wt_path))
 			break;
+	}
 	free(path);
 	free(to_free);
 	return *list;
-- 
2.21.0.1141.gd54ac2cb17


  parent reply	other threads:[~2019-05-13 10:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-12 10:12 "add worktree" fails with "fatal: Invalid path" error Shaheed Haque
2019-05-13  9:20 ` Duy Nguyen
2019-05-13 12:55   ` Shaheed Haque
2019-05-14 12:33     ` Duy Nguyen
2019-05-14 14:48       ` Shaheed Haque
2019-05-13 10:49 ` Nguyễn Thái Ngọc Duy [this message]
2019-05-13 12:42   ` [PATCH] worktree add: be tolerant of corrupt worktrees Shaheed Haque
2019-05-17  7:46   ` Eric Sunshine
2019-05-18 11:49     ` Duy Nguyen
2019-06-01 19:29       ` Shaheed Haque

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=20190513104944.20367-1-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=shaheedhaque@gmail.com \
    --cc=sunshine@sunshineco.com \
    /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.