git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <junkio@cox.net>
To: Fredrik Kuivinen <freku045@student.liu.se>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] A new merge algorithm, take 2
Date: Sat, 27 Aug 2005 22:23:23 -0700	[thread overview]
Message-ID: <7vmzn2prck.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: <20050827205135.GB16587@c165.ib.student.liu.se> (Fredrik Kuivinen's message of "Sat, 27 Aug 2005 22:51:35 +0200")

Fredrik Kuivinen <freku045@student.liu.se> writes:

> The main changes compared to the previous version are:
>
> * Lots of clean up.
> * Some of the scripts have been renamed to better match the naming
>   convention used in Git.
> * A new option ('-s') has been added to git-merge-cache
> * Unclean merges are detected and reported
> * Clean merges are committed

I have not looked deeply into your Graph thing, so I'd comment
only on a couple of points in the external interfaces area.

I am not sure why you need to be touching merge-cache.  I
suspect that reading directly from "ls-files --stage -z" might
be easier and certainly less intrusive.  I do not have immediate
objections to "read-tree -i", though.  I think it is useful.

When there is a merge conflict, the current code leaves the
index in unmerged state and intermediate merge result with
conflict marker is left in the working tree.  It appears that
your code changes this and puts the blob with conflict markers
in the index file.  Leaving things in unmerged state has two
advantages:

 - you _could_ run git-diff-stages to see what the changes in
   both sides are.

 - you cannot accidentally make a commit from such an index file
   state; in fact you cannot even write it out as a tree.

I understand why you do it this way, and I do not find your way
_too_ problematic, but we do need to realize that this is making
things somewhat more prone to human errors.

I have to admit that I find that "even when merging the
merge-base candidates results in a file with conflict markers in
it, the parts with the conflict markers often gets changed in
the heads being merged, and the conflict markers will be gone
from the result" trick very cute and interesting.  By itself it
has certain amusement value, and if it works well in practice
that is great.

> +print 'Merging', h1, 'with', h2
> +h1 = runProgram(['git-rev-parse', '--revs-only', h1]).rstrip()
> +h2 = runProgram(['git-rev-parse', '--revs-only', h2]).rstrip()

Here, '--verify" would be the right flag to give to these.  Perhaps:

    h1 = runProgram(['git-rev-parse', '--verify', ('%s^0' % h1)]).rstrip()

  reply	other threads:[~2005-08-28  5:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-27 20:51 [PATCH] A new merge algorithm, take 2 Fredrik Kuivinen
2005-08-28  5:23 ` Junio C Hamano [this message]
2005-08-28  8:03   ` Fredrik Kuivinen
2005-08-28  9:42     ` Junio C Hamano

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=7vmzn2prck.fsf@assigned-by-dhcp.cox.net \
    --to=junkio@cox.net \
    --cc=freku045@student.liu.se \
    --cc=git@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).