All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Stefan Beller <stefanbeller@gmail.com>,
	Johannes.Schindelin@gmx.de, barkalow@iabervon.org,
	git@vger.kernel.org
Subject: Re: [PATCH 3/3] checkout -m: attempt merge when deletion of path was staged
Date: Wed, 13 Aug 2014 11:59:18 -0700	[thread overview]
Message-ID: <xmqqvbpw2p2x.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqzjf82sc7.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Wed, 13 Aug 2014 10:48:56 -0700")

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

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> twoway_merge() is missing an o->gently check in the case where a file
>> that needs to be modified is missing from the index but present in the
>> old and new trees.  As a result, in this case 'git checkout -m' errors
>> out instead of trying to perform a merge.
>
> I see two hunks in threeway_merge(), so two existing callers there
> will not change their behaviour.  Two hunks in twoway_merge() means
> that among three existing callers in that function, this one at the
> end (not shown in your patch) changes its behaviour:
>
> 	else if (newtree) {
> 		if (oldtree && !o->initial_checkout) {
> 			/*
> 			 * deletion of the path was staged;
> 			 */
> 			if (same(oldtree, newtree))
> 				return 1;
> 			return reject_merge(oldtree, o);
> 		}
> 		return merged_entry(newtree, current, o);
> 	}
> 	return deleted_entry(oldtree, current, o);
>
>> This is the most iffy of the three patches, mostly because I was too
>> lazy to write a test.
>
> You would trigger this codepath by jumping from an old revision to a
> new revision after "git rm $path" any path that has been modified
> between the two.  The only behaviour difference is that it will stop
> issuing an error message---the "checkout -m" will successfully switch
> between the revs and leave the index in a "we modified, they removed"
> conflicting state with or without your patch.

IOW, something like this perhaps?

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0c9ec0a..cedbb6a 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -223,6 +223,23 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	test_cmp two expect
 '
 
+test_expect_success 'switch to another branch while carrying a deletion' '
+
+	git checkout -f master && git reset --hard && git clean -f &&
+	git rm two &&
+
+	test_must_fail git checkout simple 2>errs &&
+	test_i18ngrep overwritten errs &&
+
+	git checkout --merge simple 2>errs &&
+	! test_i18ngrep overwritten errs &&
+	git ls-files -u &&
+	test_must_fail git cat-file -t :0:two &&
+	test "$(git cat-file -t :1:two)" = blob &&
+	test "$(git cat-file -t :2:two)" = blob &&
+	test_must_fail git cat-file -t :3:two
+'
+
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 
 	git config advice.detachedHead false &&

  reply	other threads:[~2014-08-13 18:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-11 19:44 [PATCH] unpack-tree.c: remove dead code Stefan Beller
2014-08-12 18:13 ` Junio C Hamano
2014-08-12 21:15   ` Stefan Beller
2014-08-12 22:24     ` Junio C Hamano
2014-08-12 23:57     ` [PATCH 0/3] " Jonathan Nieder
2014-08-12 23:59       ` [PATCH 1/3] unpack-trees: simplify 'all other failures' case Jonathan Nieder
2014-08-13  0:00       ` [PATCH 2/3] unpack-trees: use 'cuddled' style for if-else cascade Jonathan Nieder
2014-08-13 14:52         ` Ronnie Sahlberg
2014-08-13  0:03       ` [PATCH 3/3] checkout -m: attempt merge when deletion of path was staged Jonathan Nieder
2014-08-13  0:38         ` Junio C Hamano
2014-08-13 17:48         ` Junio C Hamano
2014-08-13 18:59           ` Junio C Hamano [this message]
2014-08-13 19:30             ` Johannes Sixt
2014-08-13 20:02               ` Junio C Hamano
2014-08-13  6:41       ` [PATCH 0/3] Re: [PATCH] unpack-tree.c: remove dead code Stefan Beller

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=xmqqvbpw2p2x.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=barkalow@iabervon.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=stefanbeller@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.