Fix squash when target repository has a renamed file

If a file were renamed on the target branch but the source branch still
contained the original file, previously squash would fail with `error:
Sparse checkout leaves no entry on working directory`. This happens
because of the way a worktree is created with squash:

1. `git diff --name-only <START_SHA>...<END_SHA>`, where START_SHA and
END_SHA are from the source branch.
2. This will populate `info/sparse-checkout` with the files changed on
the source branch.
3. We then checkout the target branch, which fails if all of the changed
files from the source branch do not exist on it.

A squash should not be confused with a squash and rebase. Squash should
simply take the existing commits on the branch and collapse them into
one. The target branch is irrelevant, and so we should ignore the branch
parameter entirely.

Closes https://gitlab.com/gitlab-org/gitaly/issues/2395

5 files changed, 73 insertions, 8 deletions

diff --git a/changelogs/unreleased/sh-fix-squash-renamed-files-target-branch.yml b/changelogs/unreleased/sh-fix-squash-renamed-files-target-branch.yml
new file mode 100644
index 000000000..74a0bbb62
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-squash-renamed-files-target-branch.yml
@@ -0,0 +1,5 @@
+---
+title: Fix squash when target repository has a renamed file
+merge_request: 1786
+author:
+type: fixed
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index 25e4936e0..f7ce71f41 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -108,7 +108,7 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) {
 
 	commit, err := log.GetCommit(ctx, testRepo, response.SquashSha)
 	require.NoError(t, err)
-	require.Equal(t, []string{"3d05a143ac193c1a6fe4d046a6e3fe71e825258a"}, commit.ParentIds)
+	require.Equal(t, []string{"6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"}, commit.ParentIds)
 	require.Equal(t, author.Name, commit.Author.Name)
 	require.Equal(t, author.Email, commit.Author.Email)
 	require.Equal(t, user.Name, commit.Committer.Name)
@@ -158,7 +158,70 @@ func TestSplitIndex(t *testing.T) {
 	require.False(t, ensureSplitIndexExists(t, testRepoPath))
 }
 
-func TestFailedUserSquashRequestDueToGitError(t *testing.T) {
+func TestSquashRequestWithRenamedFiles(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	server, serverSocketPath := runOperationServiceServer(t)
+	defer server.Stop()
+
+	client, conn := NewOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t)
+	defer cleanupFn()
+
+	originalFilename := "original-file.txt"
+	renamedFilename := "renamed-file.txt"
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.name", string(author.Name))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.email", string(author.Email))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", "squash-rename-test", "master")
+	require.NoError(t, ioutil.WriteFile(filepath.Join(testRepoPath, originalFilename), []byte("This is a test"), 0644))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "add", ".")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-m", "test file")
+
+	startCommitID := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "HEAD"))
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "mv", originalFilename, renamedFilename)
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "renamed test file")
+
+	// Modify the original file in another branch
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", "squash-rename-branch", startCommitID)
+	require.NoError(t, ioutil.WriteFile(filepath.Join(testRepoPath, originalFilename), []byte("This is a change"), 0644))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "test")
+
+	require.NoError(t, ioutil.WriteFile(filepath.Join(testRepoPath, originalFilename), []byte("This is another change"), 0644))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "test")
+
+	endCommitID := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "HEAD"))
+
+	request := &gitalypb.UserSquashRequest{
+		Repository:    testRepo,
+		User:          user,
+		SquashId:      "1",
+		Branch:        []byte("squash-rename-test"),
+		Author:        author,
+		CommitMessage: commitMessage,
+		StartSha:      startCommitID,
+		EndSha:        endCommitID,
+	}
+
+	response, err := client.UserSquash(ctx, request)
+	require.NoError(t, err)
+	require.Empty(t, response.GetGitError())
+
+	commit, err := log.GetCommit(ctx, testRepo, response.SquashSha)
+	require.NoError(t, err)
+	require.Equal(t, []string{startCommitID}, commit.ParentIds)
+	require.Equal(t, author.Name, commit.Author.Name)
+	require.Equal(t, author.Email, commit.Author.Email)
+	require.Equal(t, user.Name, commit.Committer.Name)
+	require.Equal(t, user.Email, commit.Committer.Email)
+	require.Equal(t, commitMessage, commit.Subject)
+}
+
+func TestSuccessfulUserSquashRequestWithMissingFileOnTargetBranch(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
@@ -187,7 +250,7 @@ func TestFailedUserSquashRequestDueToGitError(t *testing.T) {
 
 	response, err := client.UserSquash(ctx, request)
 	require.NoError(t, err)
-	require.Contains(t, response.GitError, "error: large_diff_old_name.md: does not exist in index")
+	require.Empty(t, response.GetGitError())
 }
 
 func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
diff --git a/ruby/lib/gitaly_server/operations_service.rb b/ruby/lib/gitaly_server/operations_service.rb
index 0d3cb43cd..2d26ef15c 100644
--- a/ruby/lib/gitaly_server/operations_service.rb
+++ b/ruby/lib/gitaly_server/operations_service.rb
@@ -299,7 +299,6 @@ module GitalyServer
       author = Gitlab::Git::User.from_gitaly(request.author)
 
       squash_sha = repo.squash(user, request.squash_id,
-                               branch: request.branch,
                                start_sha: request.start_sha,
                                end_sha: request.end_sha,
                                author: author,
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 2566ce4df..d443ceb77 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -383,7 +383,7 @@ module Gitlab
         end
       end
 
-      def squash(user, squash_id, branch:, start_sha:, end_sha:, author:, message:)
+      def squash(user, squash_id, start_sha:, end_sha:, author:, message:)
         worktree = Gitlab::Git::Worktree.new(path, SQUASH_WORKTREE_PREFIX, squash_id)
         env = git_env.merge(user.git_env).merge(
           'GIT_AUTHOR_NAME' => author.name,
@@ -394,7 +394,7 @@ module Gitlab
           %W[diff --name-only --diff-filter=ar --binary #{diff_range}]
         ).chomp
 
-        with_worktree(worktree, branch, sparse_checkout_files: diff_files, env: env) do
+        with_worktree(worktree, start_sha, sparse_checkout_files: diff_files, env: env) do
           # Apply diff of the `diff_range` to the worktree
           diff = run_git!(%W[diff --binary #{diff_range}])
           run_git!(%w[apply --index --3way --whitespace=nowarn], chdir: worktree.path, env: env, include_stderr: true) do |stdin|
diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb
index 417f60501..216666e60 100644
--- a/ruby/spec/lib/gitlab/git/repository_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_spec.rb
@@ -647,13 +647,11 @@ describe Gitlab::Git::Repository do # rubocop:disable Metrics/BlockLength
   describe '#squash' do
     let(:repository) { mutable_repository }
     let(:squash_id) { '1' }
-    let(:branch_name) { 'fix' }
     let(:start_sha) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' }
     let(:end_sha) { '12d65c8dd2b2676fa3ac47d955accc085a37a9c1' }
 
     subject do
       opts = {
-        branch: branch_name,
         start_sha: start_sha,
         end_sha: end_sha,
         author: user,