public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: Miklos Vajna <vmiklos@collabora•com>
Cc: Elijah Newren <newren@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH] log: let --follow follow renames in merge commits
Date: Fri, 22 May 2026 01:43:12 -0400	[thread overview]
Message-ID: <20260522054312.GD861761@coredump.intra.peff.net> (raw)
In-Reply-To: <ag2265RJal-tJLoW@collabora.com>

On Wed, May 20, 2026 at 03:28:11PM +0200, Miklos Vajna wrote:

> 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

  reply	other threads:[~2026-05-22  5:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12  7:21 [PATCH] log: let --follow follow renames in merge commits Miklos Vajna
2026-05-19  6:17 ` Miklos Vajna
2026-05-19  6:37   ` Junio C Hamano
2026-05-19  8:14     ` Junio C Hamano
2026-05-20 13:28     ` Miklos Vajna
2026-05-22  5:43       ` Jeff King [this message]
2026-05-23  6:04         ` [PATCH] log: improve --follow following " Miklos Vajna
2026-05-30  6:28           ` Miklos Vajna
2026-06-04 12:20             ` Miklos Vajna
  -- strict thread matches above, loose matches on Subject: below --
2026-05-05  7:42 [PATCH] log: let --follow follow " Miklos Vajna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260522054312.GD861761@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=newren@gmail$(echo .)com \
    --cc=vmiklos@collabora$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox