Re: [PATCH] log: use true parents for diff even when rewriting

[Junio C Hamano <gitster@pobox.com>]

Thomas Rast <trast@inf.ethz.ch> writes:

> When using pathspec filtering in combination with diff-based log
> output, parent simplification happens before the diff is computed.
> The diff is therefore against the *simplified* parents.
>
> This works okay, arguably by accident, in the normal case: the pruned
> commits did not affect the paths being filtered, so the diff against
> the prune-result is the same as against the diff against the true
> parents.
>
> However, --full-diff breaks this guarantee, and indeed gives pretty
> spectacular results when comparing the output of
>
>   git log --graph --stat ...
>   git log --graph --full-diff --stat ...
>
> (--graph internally kicks in parent simplification, much like
> --parents).
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
> ---
>
> Perhaps like this.  It's getting a bit late, so I'm not sure if I'm
> missing another user of the "true" parent list, but it does fix the
> issue you reported.

Conceptually I can see how this will change the history
simplification in the vertical direction (skipping the ancestry
chain and jumping directly to the closest grandparent that touched
the specified path), but I am not sure how well this interacts with
history simplification in the horizontal direciton (culling
irrelevant side branches from the merge).

> @@ -2820,6 +2821,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
>  	if (action == commit_show &&
>  	    !revs->show_all &&
>  	    revs->prune && revs->dense && want_ancestry(revs)) {
> +		save_parents(commit);
>  		if (rewrite_parents(revs, commit, rewrite_one) < 0)
>  			return commit_error;
>  	}

After this, rewritten parent list may have shrunk by dropping
irrelevant side branches, but saved parents would have the full
parents list.  When we decide how to show diff depending on the
number of remaining parents, we would still use the rewritten
parents (which may have been reduced to a single strand of pearls)
and your change will use the original parents (which may be
multiple).

I also have to wonder if we always want to incur this save-parents
overhead, or we are better off limiting it to only when --full-diff
is in effect.

Thanks for getting the ball rolling, anyway.