Re: [PATCH] log: use true parents for diff even when rewriting
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:14
Thomas Rast [off-list ref] 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 <redacted> Signed-off-by: Thomas Rast <redacted> --- 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).
quoted hunk
@@ -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.