git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
To: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Subject: [RFC PATCH v2] builtin/worktree: enhance worktree removal
Date: Mon, 27 Nov 2017 23:06:21 +0530	[thread overview]
Message-ID: <20171127173621.6707-1-kaartic.sivaraam@gmail.com> (raw)
In-Reply-To: <20883815-0d9b-a4d8-cc93-0eb8b918551f@gmail.com>

"git worktree remove" removes both the named worktree
directory and the administrative information for it after
checking that there is no local modifications that would be
lost (which is a handy safety measure). However, due to a
possible oversight, it aborts with an error if the worktree
directory is _already_ removed.

The user could use "git worktree prune" after seeing the
error and realizing the situation, but at that point, there
is nothing gained by leaving only the administrative data
behind. Teach "git worktree remove" to go ahead and remove
the trace of the worktree in such a case.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 Changes in v2:

  - incorporated the suggestion to avoid quieting `validate_worktree()`
    to detect inexistent directory (thanks, Eric!)

  - used the suggested (much better) commit message

 builtin/worktree.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index b5afba164..6eab91889 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -605,6 +605,23 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	return update_worktree_location(wt, dst.buf);
 }
 
+/* Removes the .git/worktrees/worktree_id directory for
+ * the given worktree_id
+ *
+ * Returns 0 on success and non-zero value in case of failure
+ */
+static int remove_worktree_entry(char *worktree_id) {
+	int ret = 0;
+	struct strbuf we_path = STRBUF_INIT;
+	strbuf_addstr(&we_path, git_common_path("worktrees/%s", worktree_id));
+	if (remove_dir_recursively(&we_path, 0)) {
+		error_errno(_("failed to delete '%s'"), we_path.buf);
+		ret = -1;
+	}
+	strbuf_release(&we_path);
+	return ret;
+}
+
 static int remove_worktree(int ac, const char **av, const char *prefix)
 {
 	int force = 0;
@@ -634,6 +651,16 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 			die(_("already locked, reason: %s"), reason);
 		die(_("already locked, no reason"));
 	}
+
+	if (!file_exists(wt->path)) {
+	/* There's a worktree entry but the worktree directory
+	 * doesn't exist. So, just remove the worktree entry.
+	 */
+		ret = remove_worktree_entry(wt->id);
+		free_worktrees(worktrees);
+		return ret;
+	}
+
 	if (validate_worktree(wt, 0))
 		return -1;
 
@@ -670,13 +697,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 		error_errno(_("failed to delete '%s'"), sb.buf);
 		ret = -1;
 	}
-	strbuf_reset(&sb);
-	strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
-	if (remove_dir_recursively(&sb, 0)) {
-		error_errno(_("failed to delete '%s'"), sb.buf);
-		ret = -1;
-	}
-	strbuf_release(&sb);
+	ret = remove_worktree_entry(wt->id);
 	free_worktrees(worktrees);
 	return ret;
 }
-- 
2.15.0.345.gf926f18f3


  reply	other threads:[~2017-11-27 17:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-21 15:09 [RFC PATCH] builtin/worktree: enhance worktree removal Kaartic Sivaraam
2017-11-21 21:37 ` Eric Sunshine
2017-11-22  2:12   ` Junio C Hamano
2017-11-22  3:14     ` Eric Sunshine
2017-11-22  3:23       ` Eric Sunshine
2017-11-22  3:55         ` Junio C Hamano
2017-11-22  4:37           ` Eric Sunshine
2017-11-22 17:51             ` Kaartic Sivaraam
2017-11-27 17:36               ` Kaartic Sivaraam [this message]
2017-11-28  3:02                 ` [RFC PATCH v2] " Junio C Hamano
2017-11-28  3:48                   ` Eric Sunshine
2017-11-28  4:04                     ` Junio C Hamano
2017-11-28  6:02                       ` Eric Sunshine
2017-11-28 16:46                       ` Kaartic Sivaraam
2017-11-22 17:13           ` [RFC PATCH] " Kaartic Sivaraam
2017-11-22 17:09   ` Kaartic Sivaraam
2017-11-22 18:05     ` Eric Sunshine

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20171127173621.6707-1-kaartic.sivaraam@gmail.com \
    --to=kaartic.sivaraam@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).