Git development
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: git@vger.kernel.org
Subject: "git checkout" branch switching safety broken in 'next'
Date: Mon, 10 Mar 2008 01:12:37 -0700	[thread overview]
Message-ID: <7vmyp7j8ui.fsf@gitster.siamese.dyndns.org> (raw)

Linus, please be careful when switching branches if you are using your
unpack_trees() patch (in 'next') for production.  There is a breakage that
makes switching branches with "git checkout" lose your work-in-progress.

Traditionally, when you have a change in the work tree, and switch to
another branch that has different contents in the modified path, we
errored out by saying:

    error: Entry 'foo' not uptodate. Cannot merge.

The updated unpack_trees() notices the condition, issues this message
correctly and tries to propagate the error up, but loses the error at a
few places in the call chain.  The end result is that the caller does not
notice the two-way merge failure, and after that all h*ll breaks loose.

I think this should fix it.

-- >8 --
traverse_trees_recursive(): propagate merge errors up

There were few places where merge errors detected deeper in the call chain
were ignored and not propagated up the callchain to the caller.

Most notably, this caused "git checkout" to switch branches to internally
notice that a path modified in a work tree are different between the HEAD
version and the commit being switched to, but ignore that and switch
branches nevertheless, resulting in an incorrect two-way merge and loss of
the change in the work tree.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 unpack-trees.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index da68557..5a0f038 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -144,8 +144,7 @@ int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conf
 			sha1 = names[i].sha1;
 		fill_tree_descriptor(t+i, sha1);
 	}
-	traverse_trees(n, t, &newinfo);
-	return 0;
+	return traverse_trees(n, t, &newinfo);
 }
 
 /*
@@ -306,7 +305,9 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
 			if (src[0])
 				conflicts |= 1;
 		}
-		traverse_trees_recursive(n, dirmask, conflicts, names, info);
+		if (traverse_trees_recursive(n, dirmask, conflicts,
+					     names, info) < 0)
+			return -1;
 		return mask;
 	}
 

             reply	other threads:[~2008-03-10  8:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-10  8:12 Junio C Hamano [this message]
2008-03-10  9:29 ` "git checkout" branch switching safety broken in 'next' Jakub Narebski
2008-03-10 13:56   ` Jay Soffian
2008-03-11  8:57     ` setting git-gui locale Maxim Fridental
2008-03-11 10:01       ` Johannes Schindelin
2008-03-10 14:38 ` "git checkout" branch switching safety broken in 'next' Linus Torvalds
2008-03-11  2:41   ` "git diff-index" " Junio C Hamano
2008-03-11  3:20     ` Linus Torvalds

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=7vmyp7j8ui.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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