All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Clemens Buchacher <drizzd@aon.at>
Cc: git@vger.kernel.org, johannes.schindelin@gmx.de, raa.lkml@gmail.com
Subject: Re: [PATCH] modify/delete conflict resolution overwrites untracked file
Date: Sun, 28 Dec 2008 14:21:40 -0800	[thread overview]
Message-ID: <7v3ag89c0b.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20081228114445.GA8511@localhost> (Clemens Buchacher's message of "Sun, 28 Dec 2008 12:44:45 +0100")

Clemens Buchacher <drizzd@aon.at> writes:

> I've been giving this some thought and I would like to propose the following
> solution:
>
> 1. Save index state.
> 2. Merge using whichever strategy, in index only, ignoring work tree.
>    This step includes rename detection.
> 3. Check that work tree files match original index, if index does not match
>    HEAD. Otherwise abort, restore index and leave everything unchanged.
> 4. Checkout index, overwriting work tree files, and removing files which are
>    in HEAD, but not in the index.
> 5. If the merge was clean, commit.
>
> AFAICS, this is largely the behavior right now, except that steps 3 and 4
> are intermingled with step 2, which makes it impossible to abort the merge
> if an untracked file is in the way after rename detection.

The description of step 3 above may be a bit too sketchy and simplistic,
but I agree, provided if you are talking about what merge-recursive
backend does, that is how it ought to do things.  The strategy should
instead:

 - have trivial merge done inside the index, likely leveraging
   unpack_trees() three-way merge mechanism we already have, and it will
   allow you to notice and abort upon seeing local modifications in the
   work tree at this stage, except for renamed paths;

 - notice renames for the unmerged ones and deal with them inside the
   index, still without resolving.  E.g. in a merge between A and B, if
   side A deleted "path1" and created a "path2" that is similar to deleted
   "path1", have the ancestor version at stage #1 of "path2", hoist
   "path2" of side A to stage #2, and move the version of "path1" from
   side B and have it at stage #3 of "path2", which would result in an
   index without "path1" in any stage, and "path2" in three stages.

   If the other side's rename causes a path to be moved, you can/should
   check and notice local modifications to it at this stage.

   You can also notice D/F conflict at this phase (e.g. side B in the
   above example may have created a new path "path2/file1" and the index
   may have stage #3 entry without any other stages for "path2/file1") and
   abort.

 - try to resolve unmerged paths inside the index.

   Because you matched up possible renames and and adjusted the entries,
   you can resolve aggressively, e.g. a path with identical stage #1 and
   #3 but missing stage #2 results in delete, a path with missing stage #1
   and #2 but with stage #3 results in create, etc.

 - notice the paths that need to be written out to the work tree.  Paths
   with local changes should have been noticed already in the above, but
   while debugging your new algorithm, you may want to have a logic to
   double check so that you won't miss any.  Abort if you find any local
   change that can be clobbered.

 - Then, finally, you commit to writing the results out, both the index
   and the work tree.  The changes to the work tree may be cleanly merged,
   or an unmerged results with three-way conflict markers.

This way, you will never have "merge stops in halfway because it notices
it will clobber a local change too late after it has already written a
partial merge results to the work tree" problem, I think.

> git merge <branch>
> # Conflicts? I don't have time for that now.
> git reset --hard HEAD
>
> under all circumstances, without touching any untracked files.

Even though I agree the above without the second line is a good thing to
have, I think your description is a wrong way to present the desirable
goal.

"Conflicts?" is an indication that you had a lot of other things going on
inside your work tree as set of local changes that you "don't have time
for that now", and "reset --hard" is a sure way to lose them.  You
shouldn't train your users to say "--hard" lightly.

The issue of unreliable D/F conflict detection and local change detection
the current merge-recursive has at its corner case is not about
"Conflicts? I don't have time for that now" at all.  It is not about " I
had something precious in my work tree, but the merge turned out to be
unmanageable for me", either.

The issue is about "Crap, if merge has to abort in the middle because it
has to refrain from writing out a half-merged state out, not to overwrite
my local changes, it shouldn't have written out anything else to the work
tree at all.  Don't abort in the middle but abort upfront without touching
my work tree!".

I think the correct statement of the goal is:

	$ git merge ..
        # Ah, I have "path2" locally modified so it cannot proceed?
        # I did not lose anything, nor it did anything to my index nor
        # work tree.  Good.

By this, you realize that you should wrap up what you have been doing
first, because the above is an indication that other end has made some
overlapping changes.  You can either (1) decide not to merge at this point
because you are not ready, keep working on what you were doing first and
later merge, or (2) stash away what you have been doing and do the merge
first, and then unstash.

  reply	other threads:[~2008-12-28 22:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-10 20:12 [PATCH] modify/delete conflict resolution overwrites untracked file Clemens Buchacher
2008-12-10 20:51 ` Junio C Hamano
2008-12-10 21:11   ` Clemens Buchacher
2008-12-10 23:36     ` Junio C Hamano
2008-12-11  8:07       ` Clemens Buchacher
2008-12-11  8:13         ` Junio C Hamano
2008-12-15  0:46 ` Clemens Buchacher
2008-12-15  1:03   ` Junio C Hamano
2008-12-15  3:34     ` Junio C Hamano
2008-12-15  9:34       ` Johannes Schindelin
2008-12-15 10:35         ` Junio C Hamano
2008-12-15 11:03           ` Johannes Schindelin
2008-12-15  9:59       ` Clemens Buchacher
2008-12-15 10:22         ` Junio C Hamano
2008-12-15 10:50           ` Mike Ralphson
2008-12-15 11:09             ` Johannes Schindelin
2008-12-15 11:45               ` Mike Ralphson
2008-12-15 22:13           ` Junio C Hamano
2008-12-15 23:02             ` Clemens Buchacher
2008-12-16  0:16               ` Junio C Hamano
2008-12-16  1:09                 ` Jakub Narebski
2008-12-28 11:44           ` Clemens Buchacher
2008-12-28 22:21             ` Junio C Hamano [this message]
2008-12-28 23:53               ` Clemens Buchacher

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=7v3ag89c0b.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=raa.lkml@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.