On Thu, Jun 20, 2013 at 03:33:34PM +0200, Thomas Rast wrote:

> >> I naively would have expected this to leave us in a conflicted state
> >> over "file".  But I guess read-tree just rejects it, because we are not
> >> doing a real three-way merge.  I'm not sure it is that big a deal; this
> >> is more about safety than about creating a conflicted/resolvable state.
> >
> > Note that the test_must_fail essentially tests that the merge is rejected.
> 
> Bah, no it doesn't, a conflicting merge also returns a nonzero status.
> Sigh.
> 
> If you meant we should actually conflict,

Yes, that's what I meant.

> I'm not sure what options there would be other than actually calling a
> merge driver.  And we could actually do this like so (it'll obviously
> break the tests):

I'd rather not invoke a merge driver directly if we can avoid it.  I
think you could get rid of this special code-path entirely if you just
lied to the "git-merge" and said "the ancestor and current tree are fake
commits with an empty tree", and then followed the usual path. But that
lying through git-merge is ugly and complicated (and is more or less
what you're doing with the merge-recursive patch here).

> diff --git i/git-pull.sh w/git-pull.sh
> index 1f84383..b3d36a8 100755
> --- i/git-pull.sh
> +++ w/git-pull.sh
> @@ -276,7 +276,7 @@ then
>       # lose index/worktree changes that the user already made on
>       # the unborn branch.
>       empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
> -     git read-tree -m -u $empty_tree HEAD || exit 1
> +     git merge-recursive $empty_tree -- $(git write-tree) HEAD || exit 1

I don't think there is any advantage to using merge-recursive over
read-tree here, in the sense that there cannot be any interesting
content-level merging going on (our ancestor is the empty tree, so we
know that differing content cannot be resolved).

So I think you could just use read-tree with a 3-way merge, but I cannot
seem to provoke it to leave a conflict. Hrm.

I also noticed that the procedure in this code path is:

  1. Update HEAD with the fetched ref.

  2. Checkout the contents with read-tree.

I wonder if it would make sense to update HEAD only _after_ we had
resolved successfully. As it is now, you are left in a weird state where
pull has reported failure, but we actually update the HEAD (and "git
status" afterwards reflects that you are building on top of the pulled
HEAD).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to