On 24/05/2018 23:26, Kevin Bracey wrote:
On Wed, May 23, 2018 at 07:10:58PM +0200, SZEDER Gábor wrote:
$ git log --oneline master..ba95710a3b -- ci/
ea44c0a594 Merge branch 'bw/protocol-v2' into
jt/partial-clone-proto-v2
In this case, we're hitting a merge commit which is not on master, but
it has two parents which both are. Which, IIRC, means the merge commit
is INTERESTING with two UNINTERESTING parents; and we are TREESAME to
only one of them.
The commit changing the logic of TREESAME you identified believes that
those TREESAME changes for merges which were intended to improve
fuller history modes shouldn't affect the simple history "because
partially TREESAME merges are turned into normal commits". Clearly
that didn't happen here.
Haven't currently got a development environment set up here, but I've
been looking at the code.Here's a proposal, untested, as a potential
starting point if anyone wants to consider a proper patch.
The simplify_history first-scan logic never actually turned merges into
simple commits unless they were TREESAME to a relevant/interesting
parent. Anything where the TREESAME parent was UNINTERESTING was
retained as a merge, but had its TREESAME flag set, and that permitted
later simplification.
With the redefinition of the TREESAME flag, this merge commit is no
longer TREESAME, and as the decoration logic to refine TREESAME isn't
active for simplify_history, it doesn't get cleaned up (even if it would
be in full history?)
I think the answer may be to add an extra post-process step on the
initial loop to handle this special case. Something like:
case REV_TREE_SAME:
if (!revs->simplify_history || !relevant_commit(p)) {
/* 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.
*/
if (ts)
ts->treesame[nth_parent] = 1;
+ /* But we note it for potential later simplification */
+ if (!treesame_parent)
+ treesame_parent = p;
continue;
}
...
After loop:
+ if (relevant_parents == 0 && revs->simplify_history &&
treesame_parent) {
+ treesame_parent->next = NULL;// Repeats code from loop -
share somehow?
+ commit->parents = treesame_parent;
+ commit->object.flags |= TREESAME;
+ return;
+ }
/*
* TREESAME is straightforward for single-parent commits. For merge
The other option would be to take off the " || !relevant_commit(p)"
test, but I'm assuming that is still needed for other cases.
Kevin
