Re: Can't find the revelant commit with git-log

> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Subject: pickaxe: don't simplify history too much
>>
>> If pickaxe is used, turn off history simplification and make sure to keep
>> merges with at least one interesting parent.
>>
>> If path specs are used, merges that have at least one parent whose files
>> match those in the specified subset are edited out.  This is good in
>> general, but leads to unexpectedly few results if used together with
>> pickaxe.  Merges that also have an interesting parent (in terms of -S or
>> -G) are dropped, too.
>>
>> This change makes sure pickaxe takes precedence over history
>> simplification.
> 
> Hmmm, I understand the _motivation_ behind the change in the second hunk,
> in that you _might_ want to dig the side branch that did not contribute
> anything to the end result when looking for a needle with either -S or -G,
> but doesn't the same logic apply to things like --grep?

> It feels as if you are fighting against what this part of the code does in
> try_to_simplify_commit():
> 
> 	switch (rev_compare_tree(revs, p, commit)) {
> 	case REV_TREE_SAME:
> 		tree_same = 1;
> 		if (!revs->simplify_history || (p->object.flags & UNINTERESTING)) {
> 			/* ... */
> 			pp = &parent->next;
> 			continue;
> 		}
> 		parent->next = NULL;
> 		commit->parents = parent;
> 		commit->object.flags |= TREESAME;
> 		return;
> 	...
> 
> When we see a _single_ parent that has the same tree, we set tree_same and
> also cull the parent list to contain only that parent.  When we are not
> simplifying the history, we do not cull the parent list and will inspect
> other parents as well, but still we set tree_same to 1 here.  When some
> other parent is found to be different, we set tree_changed to 1.  So we
> have four states (same = (0, 1) x changed = (0, 1)).
> 
> The code before your addition in the first hunk says that we keep the
> commit if there is no parent with the same contents (i.e. !tree_same) and
> there is at least one parent with different contents (i.e. tree_changed).
> I suspect that this logic may not be working well when we do not simplify
> the merge.
> 
> Let's look at the original code before your patch again.
> 
>  1. If all the parents of a commit are the same, we will see (tree_same &&
>     !tree_changed), so we get TREESAME.
> 
>  2. If some but not all of the parents are the same, we will see (tree_same
>     && tree_changed), and we end up getting TREESAME.
> 
>  3. If none of the parents is the same, (!tree_same && tree_changed) holds
>     true, and we do not get TREESAME.

> Perhaps the second condition needs to be tweaked for the "do not simplify
> merges" case?  That is, we split 2. into two cases:
> 
>  2a. When simplifying the merges, if any of the parents is the same as the
>      commit, we say TREESAME (the same as before);
> 
>  2b. When not simplifying, we say TREESAME only when all the parents are
>      the same as the commit.  Otherwise the merge commit itself is worth
>      showing, i.e. !TREESAME.
> 
> But I probably am missing some corner cases you saw in your analysis...
> 
> diff --git a/revision.c b/revision.c
> index 7b9eaef..0147124 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -439,7 +439,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
>  		}
>  		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
>  	}
> -	if (tree_changed && !tree_same)
> +	if ((!revs->simplify_history && tree_changed) || !tree_same)
>  		return;
>  	commit->object.flags |= TREESAME;
>  }