git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Bracey <kevin@bracey.fi>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: breakage in revision traversal with pathspec
Date: Wed, 11 Sep 2013 20:49:23 +0300	[thread overview]
Message-ID: <5230AD23.2050009@bracey.fi> (raw)
In-Reply-To: <xmqq38pcwc21.fsf@gitster.dls.corp.google.com>

On 11/09/2013 01:23, Junio C Hamano wrote:
> Kevin Bracey <kevin@bracey.fi> writes:
>
>> On 10/09/2013 20:19, Junio C Hamano wrote:
>>> This command
>>>
>>>       $ git log v1.8.3.1..v1.8.4 -- git-cvsserver.perl
>>>
>>> reports that a merge 766f0f8ef7 (which did not touch the specified
>>> path at all) touches it.
>>>
>>> Bisecting points at d0af663e (revision.c: Make --full-history
>>> consider more merges, 2013-05-16).
>>>
>> That merge appearing *with* --full-history would seem like correct
>> behaviour to me. Or at least it's what I intended.
> ... But it shouldn't
> appear if the user does not ask for "--full-history".

Well, there is a functioning semi-work-around for now: avoid difficult 
non-linear questions like "v1.8.3.1..v1.8.4". A question like 
"v1.8.3..v1.8.4" is a lot easier to visualise, and it does already omit 
the merge.

On reflection I'm not sure what we should for the "simple history" view 
of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a 
chance to reconsider the merge as being zero-parent, and we do have this 
little section of graph to traverse at the bottom:

           1.8.3
             o----x----x----x----x---x---     (x = included, o = 
excluded, *=!treesame)
                 /
                /*
   o--x--x--x--x

In effect, we do have a linear section of history to follow, and the 
file does change in the middle of that line. It may be quite hard to 
come up with a solid rule to hide the merge that doesn't go wrong 
somewhere else.

The current rules for this are

1) if identical to any on-graph parent, follow that one, and rewrite the 
merge as a non-merge. We currently do not follow to an identical 
off-graph parent. This long-standing comment in try_to_simplify_commit 
applies: "Even if a merge with an uninteresting side branch brought the 
entire change we are interested in, we do not want to lose the other 
branches of this merge, so we just keep going." For this query, the 
mainline link to 1.8.3 is the "uninteresting side branch"! If you do 
specify v1.8.3..v1.8.4, then v1.8.3 becomes "on-graph" thanks to other 
new rules, and this rule does kick in, hiding the merge.

2) If rule 1 doesn't activate, and it remains as a merge, hide it if 
treesame to all on-graph parents. Previously this rule was "hide if 
treesame to any parent", and so that would have hidden the merge.

Now, when I changed rule 2, I did not think this would affect the 
default log. See my commit message:

     "Now redefine a commit's TREESAME flag to be true only if a commit is
     TREESAME to _all_ of its [later: on-graph] parent. This doesn't 
affect ... the default
     simplify_history behaviour (because partially TREESAME merges are 
turned
     into normal commits)..."

Whoops - partially TREESAME merges are not always turned into normal 
commits.

Maybe the fix is to define TREESAME differently for simplify_history - 
to use the old definition of "identical to any parent" in that case. I'm 
not sure that's right though.

I currently feel instinctively more disposed to dropping the older 
"don't follow off-graph identical parents" rule. Let the default history 
go straight to v1.8.3 even though it goes off the graph, stopping us 
traversing the topic branch.

Kevin

  reply	other threads:[~2013-09-11 17:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-10 17:19 breakage in revision traversal with pathspec Junio C Hamano
2013-09-10 21:27 ` Kevin Bracey
2013-09-10 22:23   ` Junio C Hamano
2013-09-11 17:49     ` Kevin Bracey [this message]
2013-09-11 18:24       ` Jonathan Nieder
2013-09-11 19:21         ` Junio C Hamano
2013-09-11 19:39         ` Kevin Bracey
2013-09-11 21:15           ` Junio C Hamano
2013-09-19 21:35             ` Junio C Hamano
2013-09-20  3:35               ` Jeff King
2013-09-20  4:58                 ` Junio C Hamano
2013-09-20  5:11                   ` Jeff King
2013-09-20 17:51                     ` Junio C Hamano
2013-09-25  9:12                       ` Jeff King

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=5230AD23.2050009@bracey.fi \
    --to=kevin@bracey.fi \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).