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

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

 



Miklos Vajna <vmiklos@xxxxxxxxxxxxxx> writes:

> OK, here is the scenario.
> 
> The basic problemis that if it's git-commit that creates the merge
> commit and not git-merge, then git-commit does not "know" that git-merge
> was invoked using --no-ff.
> 
> --no-ff means that if reduce_heads() removes a parent, that will be a
> problem, since the resulting commit will no longer be a merge commit.
> 
> I think we can't avoid storing this info in a MERGE_MODE file, because
> we have to add HEAD to the list of parents in case --no-ff is used, but
> we should not do so in case it's reachable from existing heads and
> --no-ff is not used.
> 
> I'll send a patch that does this in a bit.

The following is summary of short dicussion about this issue on #git
see: http://colabti.org/irclogger/irclogger_log/git?date=2008-09-26,Fri#l1176

The problem is that sometimes git-commit finishes doing the merge, be
it use of --no-commit option, or a conflict occurred; depending on
whether git-merge was invoked with or without --no-ff (--ff=never)
option it should recurd reduced or non-reduced heads.

The problem for example occurs in the following situation:

  .---.---.---.                  <--- a <--- HEAD
              |\
              | \-1---2          <--- b
              \
               \--x---y          <--- c

$ git merge b c

                      /------------- b 
                      v
  .---.---.---.---1---2---M     <--- a <--- HEAD
              \          /
               \--x---y-/       <--- c

$ git merge --no-ff b c

  .---.---.---.---------M         <--- a <--- HEAD
              |\       /|
              | \-1---2 |         <--- b
              \         /
               \--x---y/          <--- c



We can select one of the following solutions:

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.

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

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.

-- 
Jakub Narebski
Poland
ShadeHawk on #git
--
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