git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: [PATCH v5 1/5] branch: test for failures while renaming branches
Date: Mon, 27 Mar 2023 00:33:02 +0200	[thread overview]
Message-ID: <2c22e77e-314f-1025-3355-740e19b743fa@gmail.com> (raw)
In-Reply-To: <f8e6447e-5cd3-98fa-f567-39e1c60dacb0@gmail.com>

When we introduced replace_each_worktree_head_symref() in 70999e9cec
(branch -m: update all per-worktree HEADs, 2016-03-27), we implemented a
best effort approach.

If we are asked to rename a branch that is simultaneously checked out in
multiple worktrees, we try to update all of those worktrees.  If we fail
updating any of them, we die() as a signal that something has gone
wrong.  However, at this point, the branch ref has already been renamed
and also updated the HEADs of the successfully updated worktrees.
Despite returning an error, we do not try to rollback those changes.

Let's add a test to notice if we change this behavior in the future.

In next commits we will change replace_each_worktree_head_symref() to
work more closely with its only caller, copy_or_rename_branch().  Let's
move the former closer to its caller, to facilitate those changes.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 branch.c          | 27 ---------------------------
 branch.h          |  8 --------
 builtin/branch.c  | 32 ++++++++++++++++++++++++++++++++
 t/t3200-branch.sh | 15 +++++++++++++++
 4 files changed, 47 insertions(+), 35 deletions(-)

diff --git a/branch.c b/branch.c
index eacef62b7c..b45454593a 100644
--- a/branch.c
+++ b/branch.c
@@ -835,30 +835,3 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
 
 	free_worktrees(worktrees);
 }
-
-int replace_each_worktree_head_symref(const char *oldref, const char *newref,
-				      const char *logmsg)
-{
-	int ret = 0;
-	struct worktree **worktrees = get_worktrees();
-	int i;
-
-	for (i = 0; worktrees[i]; i++) {
-		struct ref_store *refs;
-
-		if (worktrees[i]->is_detached)
-			continue;
-		if (!worktrees[i]->head_ref)
-			continue;
-		if (strcmp(oldref, worktrees[i]->head_ref))
-			continue;
-
-		refs = get_worktree_ref_store(worktrees[i]);
-		if (refs_create_symref(refs, "HEAD", newref, logmsg))
-			ret = error(_("HEAD of working tree %s is not updated"),
-				    worktrees[i]->path);
-	}
-
-	free_worktrees(worktrees);
-	return ret;
-}
diff --git a/branch.h b/branch.h
index ef56103c05..30c01aed73 100644
--- a/branch.h
+++ b/branch.h
@@ -155,12 +155,4 @@ int read_branch_desc(struct strbuf *, const char *branch_name);
  */
 void die_if_checked_out(const char *branch, int ignore_current_worktree);
 
-/*
- * Update all per-worktree HEADs pointing at the old ref to point the new ref.
- * This will be used when renaming a branch. Returns 0 if successful, non-zero
- * otherwise.
- */
-int replace_each_worktree_head_symref(const char *oldref, const char *newref,
-				      const char *logmsg);
-
 #endif
diff --git a/builtin/branch.c b/builtin/branch.c
index f63fd45edb..c7ace2f2da 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -509,6 +509,38 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
+/*
+ * Update all per-worktree HEADs pointing at the old ref to point the new ref.
+ * This will be used when renaming a branch. Returns 0 if successful, non-zero
+ * otherwise.
+ */
+static int replace_each_worktree_head_symref(const char *oldref, const char *newref,
+					     const char *logmsg)
+{
+	int ret = 0;
+	struct worktree **worktrees = get_worktrees();
+	int i;
+
+	for (i = 0; worktrees[i]; i++) {
+		struct ref_store *refs;
+
+		if (worktrees[i]->is_detached)
+			continue;
+		if (!worktrees[i]->head_ref)
+			continue;
+		if (strcmp(oldref, worktrees[i]->head_ref))
+			continue;
+
+		refs = get_worktree_ref_store(worktrees[i]);
+		if (refs_create_symref(refs, "HEAD", newref, logmsg))
+			ret = error(_("HEAD of working tree %s is not updated"),
+				    worktrees[i]->path);
+	}
+
+	free_worktrees(worktrees);
+	return ret;
+}
+
 static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5a8a48287c..7abd938e15 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -239,6 +239,21 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
 	git worktree prune
 '
 
+test_expect_success 'git branch -M fails if updating any linked working tree fails' '
+	git worktree add -b baz bazdir1 &&
+	git worktree add -f bazdir2 baz &&
+	touch .git/worktrees/bazdir1/HEAD.lock &&
+	test_must_fail git branch -M baz bam &&
+	test $(git -C bazdir2 rev-parse --abbrev-ref HEAD) = bam &&
+	git branch -M bam baz &&
+	rm .git/worktrees/bazdir1/HEAD.lock &&
+	touch .git/worktrees/bazdir2/HEAD.lock &&
+	test_must_fail git branch -M baz bam &&
+	test $(git -C bazdir1 rev-parse --abbrev-ref HEAD) = bam &&
+	rm -rf bazdir1 bazdir2 &&
+	git worktree prune
+'
+
 test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
 	git checkout -b baz &&
 	git worktree add -f bazdir baz &&
-- 
2.39.2

  reply	other threads:[~2023-03-26 22:33 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-30 22:59 [PATCH 0/2] branch: operations on orphan branches Rubén Justo
2022-12-30 23:04 ` [PATCH 1/2] branch: description for orphan branch errors Rubén Justo
2023-01-01  3:45   ` Junio C Hamano
2023-01-03  1:15     ` Rubén Justo
2023-01-04  6:58       ` Junio C Hamano
2023-01-06 23:39         ` Rubén Justo
2023-01-06 23:59           ` Junio C Hamano
2023-01-07  0:35             ` Rubén Justo
2023-01-07  0:00           ` Junio C Hamano
2022-12-30 23:12 ` [PATCH 2/2] branch: rename orphan branches in any worktree Rubén Justo
2023-01-15 23:54 ` [PATCH v2 0/3] branch: operations on orphan branches Rubén Justo
2023-01-16  0:00   ` [PATCH v2 1/3] avoid unnecessary worktrees traversing Rubén Justo
2023-01-19 21:24     ` Junio C Hamano
2023-01-19 23:26       ` Rubén Justo
2023-01-16  0:02   ` [PATCH v2 2/3] branch: description for orphan branch errors Rubén Justo
2023-01-16  0:04   ` [PATCH 3/3] branch: rename orphan branches in any worktree Rubén Justo
2023-01-19 21:33     ` Junio C Hamano
2023-01-19 23:34       ` Rubén Justo
2023-01-16  0:06   ` [PATCH v2 " Rubén Justo
2023-02-06 23:01   ` [PATCH v3 0/3] branch: operations on orphan branches Rubén Justo
2023-02-06 23:06     ` [PATCH v3 1/3] branch: avoid unnecessary worktrees traversals Rubén Justo
2023-02-11  4:16       ` Jonathan Tan
2023-02-15 22:00         ` Rubén Justo
2023-02-06 23:06     ` [PATCH v3 2/3] branch: description for orphan branch errors Rubén Justo
2023-02-06 23:06     ` [PATCH v3 3/3] branch: rename orphan branches in any worktree Rubén Justo
2023-02-07  0:11     ` [PATCH v3 0/3] branch: operations on orphan branches Junio C Hamano
2023-02-07  8:33     ` Ævar Arnfjörð Bjarmason
2023-02-08  0:35       ` Rubén Justo
2023-02-08 18:37       ` Junio C Hamano
2023-02-22 22:50     ` [PATCH v4 " Rubén Justo
2023-02-22 22:52       ` [PATCH v4 1/3] branch: avoid unnecessary worktrees traversals Rubén Justo
2023-02-25 15:08         ` Rubén Justo
2023-02-27 19:30           ` Jonathan Tan
2023-02-28  0:11             ` Rubén Justo
2023-02-22 22:55       ` [PATCH v4 2/3] branch: description for orphan branch errors Rubén Justo
2023-02-27 19:38         ` Jonathan Tan
2023-02-27 21:56           ` Junio C Hamano
2023-02-28  0:22           ` Rubén Justo
2023-02-22 22:56       ` [PATCH v4 3/3] branch: rename orphan branches in any worktree Rubén Justo
2023-02-27 19:41         ` Jonathan Tan
2023-02-28  0:23           ` Rubén Justo
2023-03-26 22:19       ` [PATCH v5 0/5] branch: operations on orphan branches Rubén Justo
2023-03-26 22:33         ` Rubén Justo [this message]
2023-03-26 22:33         ` [PATCH v5 2/5] branch: use get_worktrees() in copy_or_rename_branch() Rubén Justo
2023-03-26 22:33         ` [PATCH v5 3/5] branch: description for orphan branch errors Rubén Justo
2023-03-26 22:33         ` [PATCH v5 4/5] branch: rename orphan branches in any worktree Rubén Justo
2023-03-26 22:33         ` [PATCH v5 5/5] branch: avoid unnecessary worktrees traversals Rubén Justo
2023-03-27 19:49         ` [PATCH v5 0/5] branch: operations on orphan branches Junio C Hamano
2023-05-01 22:19         ` 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=2c22e77e-314f-1025-3355-740e19b743fa@gmail.com \
    --to=rjusto@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.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 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 NNTP newsgroup(s).