git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "igor.mikushkin" <igor.mikushkin@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Clemens Buchacher <drizzd@aon.at>,
	git@vger.kernel.org
Subject: [PATCH 3/4] merge: merge unborn index before setting ref
Date: Fri, 25 Mar 2011 14:10:38 -0400	[thread overview]
Message-ID: <20110325181038.GC14898@sigill.intra.peff.net> (raw)
In-Reply-To: <20110325180644.GA30838@sigill.intra.peff.net>

When we merge into an unborn branch, there are basically two
steps:

  1. Write the sha1 of the new commit into the ref pointed
     to by HEAD.

  2. Update the index with the new content, and check it out
     to the working tree.

We currently do them in this order. However, (2) is the step
that is much more likely to fail, since it can be blocked by
things like untracked working tree files. When it does, the
merge fails and we are left with an empty index but an
updated HEAD.

This patch switches the order, so that a failure in updating
the index leaves us unchanged. Of course, a failure in
updating the ref now leaves us with an updated index and
mis-matched HEAD. That is arguably not much better, but it
is probably less likely to actually happen.

Signed-off-by: Jeff King <peff@peff.net>
---
I noticed this while diagnosing the pull problem fixed in 4/4. As
discused, this is just trading one set of error conditions for another.
The "right" thing to do is probably to rollback, but of course that can
fail, too, and it's more effort. I think in practice this is fine.

 builtin/merge.c            |    2 +-
 t/t7607-merge-overwrite.sh |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index aa3453c..c8d028c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1063,9 +1063,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
 		if (!remote_head)
 			die("%s - not something we can merge", argv[0]);
+		read_empty(remote_head->sha1, 0);
 		update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
 				DIE_ON_ERR);
-		read_empty(remote_head->sha1, 0);
 		return 0;
 	} else {
 		struct strbuf merge_names = STRBUF_INIT;
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index c86e298..b54e840 100755
--- a/t/t7607-merge-overwrite.sh
+++ b/t/t7607-merge-overwrite.sh
@@ -157,6 +157,10 @@ test_expect_success 'will not overwrite untracked file on unborn branch' '
 	test_cmp important c0.c
 '
 
+test_expect_success 'failed merge leaves unborn branch in the womb' '
+	test_must_fail git rev-parse --verify HEAD
+'
+
 test_expect_success 'set up unborn branch and content' '
 	git symbolic-ref HEAD refs/heads/unborn &&
 	rm -f .git/index &&
-- 
1.7.4.33.gb8855.dirty

  parent reply	other threads:[~2011-03-25 18:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-25 14:52 Why git silently replaces untracked files? igor.mikushkin
2011-03-25 16:58 ` Jeff King
2011-03-25 17:53   ` igor.mikushkin
2011-03-25 18:06     ` Jeff King
2011-03-25 18:08       ` [PATCH 1/4] t7607: mark known breakage in test 11 as fixed Jeff King
2011-03-25 18:09       ` [PATCH 2/4] t7607: clean up stray untracked file Jeff King
2011-03-25 18:10       ` Jeff King [this message]
2011-03-25 18:13       ` [PATCH 4/4] pull: do not clobber untracked files on initial pull Jeff King

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=20110325181038.GC14898@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=igor.mikushkin@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).