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

From: Jeff King <peff@peff.net>
Date: Jul 31, 2013, 22:55 UTC
Message-ID: <20130731225520.GB25882@sigill.intra.peff.net>
In-Reply-To: <e7f2ead2267ff78940aab00fe36c378a2ce5d85e.1375301293.git.trast@inf.ethz.ch>

On Wed, Jul 31, 2013 at 10:13:20PM +0200, Thomas Rast wrote:

Show 18 quoted lines

> 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:
> simplification reduces to one parent as long as the commit is TREESAME
> to it.  So the simplified parent of any given commit must have the
> same tree contents on the filtered paths as its true (unfiltered)
> parent.
> 
> 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).

Your description (and solution) make a lot of sense to me. Another code path that has a similar problem is the "-g" reflog walker. It rewrites the parents based on the reflog, and the diffs it produces are mostly useless (e.g., try "git stash list -p").

Should we be applying the same technique there?

I guess it might bother people who really want to see the diff between two lines in the reflog (e.g., comparing the results stored by "rebase" with the previous version). I think that is rare enough that "git diff" would be a better tool in that case, though.

And arguably, "log -g" should not be rewriting the parents at all, because it makes "--graph" pointless (and indeed, we disallow the combination). So potentially the solution is to stop the rewriting entirely, not mask it for the diffs. But doing so might be a good interim solution until somebody feels like rewriting the reflog walker.

-Peff