git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Sahil Dua <sahildua2305@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Date: Fri, 22 Sep 2017 12:39:46 +0900	[thread overview]
Message-ID: <xmqqlgl7l7ul.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqpoajmtu4.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Fri, 22 Sep 2017 09:59:31 +0900")

Junio C Hamano <gitster@pobox.com> writes:

> My understanding of the next step was for those who are interested
> in moving this topic forward to update these patches in that
> direction.

Well, I am one of those who are interested in moving this topic
forward, not because I'm likely to use it, but because the fewer
number of topics I have to keep in flight, the easier my work gets.

So here is such an update.  As the topic is not in 'next' yet, it
could also be implemented by replacing patch(es) in the series, but
doing it as a follow-up fix made it easier to see what got changed
(both in the code and in the tests), so that is how I decided to do
this patch.

-- >8 --
Subject: [PATCH] branch: fix "copy" to never touch HEAD

Probably because "git branch -c A B" piggybacked its implementation
on "git branch -m A B", when creating a new branch B by copying the
branch A that happens to be the current branch, it also updated HEAD
to point at the new branch.

This does not match the usual expectation.  If I were sitting on a
blue chair, and somebody comes and repaints it to red, I would
accept ending up sitting on a red chair, but if somebody creates a
new red chair, modelling it after the blue chair I am sitting on, I
do not expect to be booted off of the blue chair and ending up on
sitting on the red one.

Let's fix this strange behaviour before it hits 'next'.  Those who
want to create a new branch and switch to it can do "git checkout B"
after creating it by copying the current branch, or if that is so
useful to deserve a short-hand way to do so, perhaps extend "git
checkout -b B" to copy configurations while creating the new branch
B.  A "copy" should remain to be "copy", not "copy and sometimes
checkout".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/branch.c  |  9 +++------
 t/t3200-branch.sh | 10 +++++-----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 89f64f4123..e2e3692838 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -506,12 +506,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 				oldref.buf + 11);
 	}
 
-	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) {
-		if (copy)
-			die(_("Branch copied to %s, but HEAD is not updated!"), newname);
-		else
-			die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
-	}
+	if (!copy &&
+	    replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
+		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
 
 	strbuf_release(&logmsg);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5d03ad16f6..be9b3784c6 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -422,7 +422,7 @@ test_expect_success 'git branch --copy is a synonym for -c' '
 	test_cmp expect actual
 '
 
-test_expect_success 'git branch -c ee ef should copy and checkout branch ef' '
+test_expect_success 'git branch -c ee ef should copy to create branch ef' '
 	git checkout -b ee &&
 	git reflog exists refs/heads/ee &&
 	git config branch.ee.dummy Hello &&
@@ -431,7 +431,7 @@ test_expect_success 'git branch -c ee ef should copy and checkout branch ef' '
 	git reflog exists refs/heads/ef &&
 	test $(git config branch.ee.dummy) = Hello &&
 	test $(git config branch.ef.dummy) = Hello &&
-	test $(git rev-parse --abbrev-ref HEAD) = ef
+	test $(git rev-parse --abbrev-ref HEAD) = ee
 '
 
 test_expect_success 'git branch -c f/f g/g should work' '
@@ -494,12 +494,12 @@ test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out'
 	git checkout -b c1 &&
 	git branch c2 &&
 	git branch -C c1 c2 &&
-	test $(git rev-parse --abbrev-ref HEAD) = c2
+	test $(git rev-parse --abbrev-ref HEAD) = c1
 '
 
-test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' '
+test_expect_success 'git branch -C c1 c2 should never touch HEAD' '
 	msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
-	grep "$msg$" .git/logs/HEAD
+	! grep "$msg$" .git/logs/HEAD
 '
 
 test_expect_success 'git branch -C master should work when master is checked out' '
-- 
2.14.1-907-g5aa63875cf




  reply	other threads:[~2017-09-22  3:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-15  5:58 What's cooking in git.git (Sep 2017, #03; Fri, 15) Junio C Hamano
2017-09-15  7:50 ` Lars Schneider
2017-09-19  1:18   ` Junio C Hamano
2017-09-15 19:14 ` Johannes Schindelin
2017-09-15 20:33   ` Junio C Hamano
2017-09-15 19:23 ` Johannes Schindelin
2017-09-15 20:49   ` Junio C Hamano
2017-09-29 11:39     ` Johannes Schindelin
2017-09-21 21:14 ` Sahil Dua
2017-09-22  0:59   ` Junio C Hamano
2017-09-22  3:39     ` Junio C Hamano [this message]
2017-09-24 16:16       ` Sahil Dua
2017-09-25  0:40         ` 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=xmqqlgl7l7ul.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sahildua2305@gmail.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).