Re: [PATCH] log: let --follow follow renames in merge commits
- From
Jeff King <peff@peff.net>
- Date
- May 22, 2026, 05:43 UTC
- Message-ID
- <20260522054312.GD861761@coredump.intra.peff.net>
- In-Reply-To
- <ag2265RJal-tJLoW@collabora.com>
On Wed, May 20, 2026 at 03:28:11PM +0200, Miklos Vajna wrote:
Show 20 quoted lines
> Hi Elijah, Jeff, > > On Tue, May 19, 2026 at 03:37:54PM +0900, Junio C Hamano <gitster@pobox.com> wrote: > > > :-) Should I just wait more or should I resend this? > > > > Rather, ask other reviewers > > I did a small improvement to how 'git log --follow' works, as in if the > rename happens inside the merge commit itself, then the rename was > detected "vs the first parent", but it wasn't detected "vs other > parents", which is painful with a "subtree" merge commit. > > I'm not sure if it adds value, but I can append a one-paragraph summary > of Junio's comment in this thread to the end the commit message, to be > more explicit that the inherent limitation of the current log follow > design (single path, once a rename is detected, we only care about the > new path) is not changed with the patch, this is just a fix patch so > 'git log' works better, similar to how 'git blame' already does. > > May I ask you to review the patch?
I saw Junio's comment. I was about to write something very similar before I saw that he had already done so. ;)
I think we can probably all agree that both before and after your patch, --follow is never going to do the _right_ thing, which is to follow paths independently down both sides of history.
I am OK conceptually with making the current broken behavior slightly more useful if it is easy to do. But I am not sure if we are making things more useful here or not. If we see a merge where the file "bar" was previous "foo" on one side and "bar" on the other, our broken follow is going to either pick "foo" or "bar" to continue with as we traverse. But which one is right? Whichever name we choose, we are potentially omitting results from the other side.
Right now we pick the first-parent name always. But it does not seem more correct to me to pick one from another parent. You'd be missing further commits using the original name along the first-parent track.
There might be a more useful rule like: if the path is untouched versus the merge result in all parents but one (i.e., TREESAME), then choose the parent where it was changed, including any --follow processing. But we already do something like that for history simplification. Which makes me wonder if you could get the results you want through some use of history-simplification flags.
Or maybe we already do that TREESAME check. Simplification kicks in when the traversal is limited by path, and --follow mode by definition has such a path. But I'm not sure if the --follow code would see the simplified parent list or not.
So I dunno. Probably some experimenting could yield more analysis there, but with the patch as-is I'm not convinced that it is not going to make some cases worse.
-Peff