From 75781f6f8fedf6de2de05aedd7415971e47819c4 Mon Sep 17 00:00:00 2001
From: Joe Woodward <jwoodward@gitlab.com>
Date: Tue, 2 Dec 2025 15:48:21 +0000
Subject: [PATCH] Follow merge commits when finding commits with follow

Fixes https://gitlab.com/gitlab-org/gitlab/-/work_items/421655+

When using subtrees the history cannot be found. This happens because
when you merge the upstream changes into the downstream repo, the files
are renamed.

This can also happen with merge commits that include a renamed file.

After this change the RPC will correctly find the history for these
files.
---
 .../gitaly/service/commit/find_commits.go     | 14 +++++-
 .../service/commit/find_commits_test.go       | 48 +++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go
index 92054c23b5..161276f4da 100644
--- a/internal/gitaly/service/commit/find_commits.go
+++ b/internal/gitaly/service/commit/find_commits.go
@@ -104,7 +104,11 @@ func (s *server) findCommits(ctx context.Context, req *gitalypb.FindCommitsReque
 		}
 	}
 
-	if err := streamCommits(getCommits, stream, req.GetTrailers(), req.GetIncludeShortstat(), len(req.GetIncludeReferencedBy()) > 0); err != nil {
+	// filter out merge commits when using --follow with paths which adds -m
+	// flag for locating subtree changes but are not desired in the results.
+	skipMerges := req.GetFollow() && len(req.GetPaths()) > 0
+
+	if err := streamCommits(getCommits, stream, req.GetTrailers(), req.GetIncludeShortstat(), len(req.GetIncludeReferencedBy()) > 0, skipMerges); err != nil {
 		return fmt.Errorf("error streaming commits: %w", err)
 	}
 
@@ -208,7 +212,7 @@ func (g *GetCommits) Commit(ctx context.Context, trailers, shortStat, refs bool)
 	return commit.GitCommit, nil
 }
 
-func streamCommits(getCommits *GetCommits, stream gitalypb.CommitService_FindCommitsServer, trailers, shortStat bool, refs bool) error {
+func streamCommits(getCommits *GetCommits, stream gitalypb.CommitService_FindCommitsServer, trailers, shortStat bool, refs bool, skipMerges bool) error {
 	ctx := stream.Context()
 
 	chunker := chunk.New(&commitsSender{
@@ -225,6 +229,11 @@ func streamCommits(getCommits *GetCommits, stream gitalypb.CommitService_FindCom
 			return err
 		}
 
+		// Skip merge commits if requested (has more than one parent)
+		if skipMerges && len(commit.GetParentIds()) > 1 {
+			continue
+		}
+
 		if err := chunker.Send(commit); err != nil {
 			return err
 		}
@@ -267,6 +276,7 @@ func getLogCommandSubCmd(req *gitalypb.FindCommitsRequest) gitcmd.Command {
 
 	if req.GetFollow() && len(req.GetPaths()) > 0 {
 		subCmd.Flags = append(subCmd.Flags, gitcmd.Flag{Name: "--follow"})
+		subCmd.Flags = append(subCmd.Flags, gitcmd.Flag{Name: "-m"})
 	}
 	if req.GetAuthor() != nil {
 		subCmd.Flags = append(subCmd.Flags, gitcmd.Flag{Name: fmt.Sprintf("--author=%s", string(req.GetAuthor()))})
diff --git a/internal/gitaly/service/commit/find_commits_test.go b/internal/gitaly/service/commit/find_commits_test.go
index 1e43c22f18..6dcb1f8d06 100644
--- a/internal/gitaly/service/commit/find_commits_test.go
+++ b/internal/gitaly/service/commit/find_commits_test.go
@@ -469,6 +469,54 @@ func TestFindCommits(t *testing.T) {
 				}
 			},
 		},
+		{
+			desc: "follow renames through merge commits (subtree)",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				// Create initial commit with a file
+				commitAID, commitA := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "testfile", Mode: "100644", Content: "line 1\n"},
+				))
+
+				// Create second commit modifying the file
+				commitBID, commitB := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "testfile", Mode: "100644", Content: "line 1\nline 2\n"},
+				), gittest.WithParents(commitAID))
+
+				// Create third commit modifying the file
+				commitCID, commitC := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "testfile", Mode: "100644", Content: "line 1\nline 2\nline 3\n"},
+				), gittest.WithParents(commitBID))
+
+				// Create a separate branch with different content
+				otherCommitID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "other", Mode: "100644", Content: "other\n"},
+				))
+
+				// Create merge commit (simulating subtree add/merge)
+				// This brings in the history from commitCID
+				mergeID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "testfile", Mode: "100644", Content: "line 1\nline 2\nline 3\n"},
+					gittest.TreeEntry{Path: "other", Mode: "100644", Content: "other\n"},
+				), gittest.WithParents(otherCommitID, commitCID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(mergeID),
+						Paths: [][]byte{
+							[]byte("testfile"),
+						},
+						Follow: true,
+						Limit:  9000,
+					},
+					// With -m flag and merge filtering: shows commits from merged branch
+					// but excludes the merge commit itself
+					expectedCommits: []*gitalypb.GitCommit{commitC, commitB, commitA},
+				}
+			},
+		},
 		{
 			desc: "all references",
 			setup: func(t *testing.T) setupData {
-- 
GitLab