All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Miklos Vajna <vmiklos@collabora.com>
Cc: gitster@pobox.com, szeder.dev@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
Date: Fri, 6 Nov 2026 15:14:17 +0100	[thread overview]
Message-ID: <au3guSIBfekx7gNL@pks.im> (raw)
In-Reply-To: <7be8cfb3ba58a4d4dc040fb2f229524599524b78.1793298331.git.vmiklos@collabora.com>

On Fri, Oct 30, 2026 at 09:10:14PM +0100, Miklos Vajna wrote:
> Take a history where F is renamed to G on master, F is edited on a side
> branch, and the side branch is merged with '-s ours', so the merge keeps
> G and drops F. Here 'git log --follow G' doesn't show the edit of F on
> the side branch, while it did before the previous commit.
> 
> The problem is that follow_needs_renames() only searches for renames in
> a parent without the path if the merge base still has the path, which
> covers the case when the parent renamed the path. Here the other side
> of the merge did the rename, so the merge base only has the old name.
> The parent is then told to follow G, which it doesn't have, and the
> history of F on its side is lost. Searching for renames in each parent
> that lacks the path would fix this, but that is the expensive search
> that the previous commit avoids, e.g. 148 merges hit this case for
> include/linux/rsc_table.h in linux.git.
> 
> Fix the problem by also searching for renames when the merge base
> doesn't have the path, but the parent changed a file since the merge
> base that the merge no longer has: only such a file can be the old name
> of the path that still needs to be followed in the parent.

This isn't true though in the case where you copy a file though, is it?
E.g. the following test case is exactly the same as the one you add in
this commit, but instead of using a rename we use a copy:

diff --git a/t/t4219-log-follow-merge.sh b/t/t4219-log-follow-merge.sh
index a85fb78ca28..c67d9481ab5 100755
--- a/t/t4219-log-follow-merge.sh
+++ b/t/t4219-log-follow-merge.sh
@@ -297,4 +297,36 @@ test_expect_success '--follow follows an edit on the side the merge did not take
 	test_cmp expect actual
 '
 
+test_expect_success 'setup merge keeping the copied path, other side edited the original' '
+	git init keep-copied &&
+	printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
+	git -C keep-copied add F &&
+	test_tick &&
+	git -C keep-copied commit -m "O: add F" &&
+	git -C keep-copied branch B &&
+	cp keep-copied/F keep-copied/G &&
+	git -C keep-copied add G &&
+	test_tick &&
+	git -C keep-copied commit -m "A1: copy F to G on master" &&
+
+	git -C keep-copied checkout B &&
+	printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
+	test_tick &&
+	git -C keep-copied commit -am "B2: update F on B" &&
+
+	git -C keep-copied checkout master &&
+	test_tick &&
+	git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
+'
+
+test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
+	git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
+	cat >expect <<-\EOF &&
+	B2: update F on B
+	A1: copy F to G on master
+	O: add F
+	EOF
+	test_cmp expect actual
+'
+
 test_done

This test passes on master, but fails with your patch series.

Patrick

  reply	other threads:[~2026-11-06 14:14 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 13:55 Performance regression in `git log --follow` Patrick Steinhardt
2026-10-13 21:24 ` SZEDER Gábor
2026-10-15 14:48   ` [PATCH] log: speed up --follow following renames for non-linear history Miklos Vajna
2026-10-15 16:57     ` Junio C Hamano
2026-10-19  6:10       ` [PATCH v2] " Miklos Vajna
2026-10-22 20:45         ` [PATCH v3 0/4] " Miklos Vajna
2026-10-22 20:45           ` [PATCH v3 1/4] t4219: test more --follow cases across merges Miklos Vajna
2026-10-22 20:46           ` [PATCH v3 2/4] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-10-22 20:46           ` [PATCH v3 3/4] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-10-26 11:40             ` Patrick Steinhardt
2026-10-22 20:47           ` [PATCH v3 4/4] log: speed up " Miklos Vajna
2026-10-26 11:40             ` Patrick Steinhardt
2026-10-26 11:42           ` [PATCH v3 0/4] " Patrick Steinhardt
2026-10-30 20:07           ` [PATCH v4 0/6] " Miklos Vajna
2026-10-30 20:07             ` [PATCH v4 1/6] t4219: test more --follow cases across merges Miklos Vajna
2026-10-30 20:07             ` [PATCH v4 2/6] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-10-30 20:08             ` [PATCH v4 3/6] log: fix -m --follow losing the history of later parents Miklos Vajna
2026-10-30 20:08             ` [PATCH v4 4/6] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-10-30 20:09             ` [PATCH v4 5/6] log: speed up " Miklos Vajna
2026-11-06 14:14               ` Patrick Steinhardt
2026-11-09  8:02                 ` Miklos Vajna
2026-10-30 20:10             ` [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take Miklos Vajna
2026-11-06 14:14               ` Patrick Steinhardt [this message]
2026-11-09  8:03                 ` Miklos Vajna
2026-11-06 14:14             ` [PATCH v4 0/6] log: speed up --follow following renames for non-linear history Patrick Steinhardt
2026-11-09  8:01               ` Miklos Vajna
2026-11-09  8:03             ` [PATCH v5 0/5] " Miklos Vajna
2026-11-09  8:04               ` [PATCH v5 1/5] t4219: test more --follow cases across merges Miklos Vajna
2026-11-10 23:06                 ` Justin Tobler
2026-11-09  8:05               ` [PATCH v5 2/5] log: extract showing the remerge/combined diff of a merge commit Miklos Vajna
2026-11-09  8:05               ` [PATCH v5 3/5] log: fix -m --follow losing the history of later parents Miklos Vajna
2026-11-09  8:05               ` [PATCH v5 4/5] log: simplify --follow following renames for non-linear history Miklos Vajna
2026-11-10 16:45                 ` Toon Claes
2026-11-09  8:06               ` [PATCH v5 5/5] log: speed up " Miklos Vajna
2026-11-11  0:40               ` [PATCH v5 0/5] " Justin Tobler
2026-11-11 10:43                 ` Patrick Steinhardt
2026-11-11 12:41                   ` Miklos Vajna
2026-11-11 13:29                     ` Patrick Steinhardt
2026-11-12  9:05                       ` [PATCH] log: make --follow following renames for non-linear history optional Miklos Vajna
2026-11-12 19:09                         ` Junio C Hamano
2026-11-11 16:44                 ` [PATCH v5 0/5] log: speed up --follow following renames for non-linear history Junio C Hamano
2026-10-16 20:37     ` [PATCH] " Junio C Hamano
2026-10-14  6:16 ` Performance regression in `git log --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=au3guSIBfekx7gNL@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder.dev@gmail.com \
    --cc=vmiklos@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.