git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: JuanLeon Lahoz <juanleon.lahoz@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: Inconsistent results obtained regarding how git decides what commits modifies a given path
Date: Fri, 7 Aug 2015 04:44:26 -0400	[thread overview]
Message-ID: <20150807084425.GA8232@sigill.intra.peff.net> (raw)
In-Reply-To: <CANYPdHA6a1Jg2NUJGcg5O3KBG2AB+AK3071ckDywGL=MHghEZA@mail.gmail.com>

On Fri, Aug 07, 2015 at 08:42:52AM +0200, JuanLeon Lahoz wrote:

> # This prints nothing on git < 1.8.4; prints a commit that corresponds with
> # "Merge branch 'b3' into b2_3" in git >= 1.8.4 (tested with 1.8.4 and 2.5.0)
> echo COMMITS checkpoint..b2_3: $(git rev-list checkpoint..b2_3 -- version)

I would expect the current behavior (to show the merge). The fix bisects
to d0af663 (revision.c: Make --full-history consider more merges,
2013-05-16). See that commit message, or the thread at:

  http://thread.gmane.org/gmane.comp.version-control.git/220624

for more detail. Though I'm not 100% it was intentional...

> # This prints nothing on any git version I tested.
> echo COMMITS checkpoint..b1_4: $(git rev-list checkpoint..b1_4 -- version)

This looks like a bug to me. It should be the exact same simplification
case as b2_3 (we have a merge whose outcome matches the second parent,
but not the first). One obvious difference, if you look at:

  git log --oneline --graph --decorate checkpoint b2_3 b1_4

is that in the b1_4 case, the immediate parent of the merge is marked as
UNINTERESTING|BOTTOM, because it is "checkpoint". Whereas in the b2_3
case, it is simply UNINTERESTING, as it is the _parent_ of checkpoint,
and we propagated the flag.

This difference matters in relevant_commit(). In the b2_3 case, we
consider the second parent irrelevant, because it is "just"
UNINTERESTING. But in the b1_4 case, we consider it relevant, due to the
BOTTOM flag. This code comes from 4d82660 (revision.c: discount side
branches when computing TREESAME, 2013-05-16).

And that commit claims that we exclude irrelevant parents when
determining TREESAME. Meaning the b2_3 case has only one relevant parent
(which is different, making the merge !TREESAME). Whereas b1_4 has two
relevant parents, we realize we are TREESAME to the second one, and
simplify away the merge.

At this point, I'm pretty confused (and reading the comments added by
4282660 only confused me further). I would have thought the TREESAME was
independent of the limiting of the traversal, but clearly it is not
intended to be. But this weird BOTTOM exception makes even less sense to
me.

So I'll give up for now. Maybe somebody else can pick it up from there,
or perhaps it will make more sense to me in the morning.

-Peff

      reply	other threads:[~2015-08-07  8:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07  6:42 Inconsistent results obtained regarding how git decides what commits modifies a given path JuanLeon Lahoz
2015-08-07  8:44 ` Jeff King [this message]

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=20150807084425.GA8232@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=juanleon.lahoz@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 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).