Re: [PATCH] builtin-commit: avoid using reduce_heads()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 26 Sep 2008, Miklos Vajna wrote:
> On Fri, Sep 26, 2008 at 09:17:39AM -0700, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> >
> > 1. As proposed above save git-merge options, for example in MERGE_MODE
> >    or MERGE_OPTS file, so git-commit knows what options to use if it
> >    was invoked to finish a merge.
> 
> First, thank you for the detailed description, I'm really bad in them.
> :)
> 
> ACK, that's why I implemented this one.

This is I think the fastest and seemingly simplest solution.

> > 2. git-merge saves _reduced_ heads in MERGE_HEAD, and git-commit
> >    reduces only HEAD, unless it is in MERGE_HEAD.  This means that
> >    git-commit uses the following pseudo-code
> > 
> >      if (resolve_ref(HEAD) == MERGE_HEAD[0]) {
> >         /* non fast-forward case */
> >         merge HEAD + MERGE_HEAD
> >      } else {
> >         reduce_HEAD_maybe()
> >         merge [HEAD] + MERGE_HEAD
> >      }
> > 
> >    This has the advantage that it doesn't change MERGE_HEAD semantic
> >    for simple merge which did not began as octopus
> 
> This is wrong IMHO. You can write the reduced heads to MERGE_HEAD but
> you can't know if you can omit the HEAD in case it is reachable already
> from one of the heads or not. That depends on if the user used --no-ff
> or not.

I have tried to explain in above pseudocode git-commit is to use
additional hack to decide whether to try to reduce HEAD wrt MERGE_HEAD
heads (normal case), or not (--no-ff case), namely if resolved HEAD
is first head in MERGE_HEAD that means --no-ff case.

It is "halfway" solution and bit hacky (and ugly).

> > 3. Remove reduce_heads() from git-commit entirely, and record in
> >    MERGE_HEAD (or rather now MERGE_HEADS) _all_ _reduced_ heads.
> >    _All_ means that HEAD is included in MERGE_HEAD if it is not
> >    reduced, _reduced_ means that only non-dependent heads are in
> >    MERGE_HEAD.  This for example means that for simple non-octopus
> >    merge case MERGE_HEAD/MERGE_HEADS now contain _all_ parents,
> >    and not only other side of merge.
> > 
> >    This solution has the advantage of being clear solution, clarifying
> >    semantic of MERGE_HEAD (currently HEAD is used both as target, i.e.
> >    where merge is to be recorded, and as one of heads to merge/to
> >    consider), and making it possible to separate layers: git-merge
> >    is about merging, git-commit doesn't need to know anything about
> >    merging.
> > 
> >    The disadvantage is that it changes format (well, semantic) of
> >    MERGE_HEAD, possibly breaking users' scripts; perhaps some of
> >    git commands like "git log --merge" or "git diff --merge" should
> >    be updated on such change.
> 
> Actually I think this would be ugly. MERGE_HEAD is about you can see
> what will be merged once you commit the merge, but once you include HEAD
> there, you can't easily check that. Maybe it's just me who sometimes
> have a look at that file myself directly... :-)

The new semantic would be very simple.  If there is no MERGE_HEAD, it
is an ordinary no-merge commit, and its only parent would be previous
(current) version of HEAD.  If there is MERGE_HEAD it contains _all_
parents in a merge commit, and only those heads which will be parents
(_reduced_ heads); if HEAD is symref, we modify given branch so it
points to newly created merge commit.

Currently parents of merge commits are 'reduce(HEAD + MERGE_HEAD)'
in symbolic equation; I propose they would be simply 'MERGE_HEAD'.
then we set this branch to new commit

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux