Merge branch 'sh-use-3way-merge-for-squash' into 'master'

Use 3-way merge for squashing commits

See merge request gitlab-org/gitaly!1214

3 files changed, 85 insertions, 5 deletions

diff --git a/changelogs/unreleased/sh-use-3way-merge-for-squash.yml b/changelogs/unreleased/sh-use-3way-merge-for-squash.yml
new file mode 100644
index 000000000..f7a7b6642
--- /dev/null
+++ b/changelogs/unreleased/sh-use-3way-merge-for-squash.yml
@@ -0,0 +1,5 @@
+---
+title: Use 3-way merge for squashing commits
+merge_request: 1214
+author:
+type: fixed
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index 29d13183c..ca0767f02 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -1,11 +1,14 @@
 package operations
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
+	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc/codes"
 )
@@ -51,10 +54,82 @@ func TestSuccessfulUserSquashRequest(t *testing.T) {
 
 	commit, err := log.GetCommit(ctx, testRepo, response.SquashSha)
 	require.NoError(t, err)
-	require.Equal(t, commit.ParentIds, []string{startSha})
-	require.Equal(t, string(commit.Author.Email), "johndoe@gitlab.com")
-	require.Equal(t, string(commit.Committer.Email), "janedoe@gitlab.com")
-	require.Equal(t, commit.Subject, commitMessage)
+	require.Equal(t, []string{startSha}, 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 TestSuccessfulUserSquashRequestWith3wayMerge(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()
+
+	// The following patch inserts a single line so that we create a
+	// file that is slightly different from a branched version of this
+	// file.  A 3-way merge must be used in order to merge a branch that
+	// alters lines involved in this diff.
+	patch := `
+diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb
+index 5b812ff..ff85394 100644
+--- a/files/ruby/popen.rb
++++ b/files/ruby/popen.rb
+@@ -19,6 +19,7 @@ module Popen
+
+     @cmd_output = ""
+     @cmd_status = 0
++
+     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+       @cmd_output << stdout.read
+       @cmd_output << stderr.read
+`
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "squash-test")
+	require.NoError(t, err)
+
+	tmpFile.Write([]byte(patch))
+	tmpFile.Close()
+
+	defer os.Remove(tmpFile.Name())
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", "3way-test", "v1.0.0")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "apply", tmpFile.Name())
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-m", "test", "-a")
+	baseSha := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "HEAD"))
+
+	request := &gitalypb.UserSquashRequest{
+		Repository:    testRepo,
+		User:          user,
+		SquashId:      "1",
+		Branch:        []byte("3way-test"),
+		Author:        author,
+		CommitMessage: commitMessage,
+		// The diff between two of these commits results in some changes to files/ruby/popen.rb
+		StartSha: "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9",
+		EndSha:   "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
+	}
+
+	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{baseSha}, 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 TestFailedUserSquashRequestDueToGitError(t *testing.T) {
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 030bece0b..123540ae9 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -426,7 +426,7 @@ module Gitlab
         with_worktree(squash_path, branch, 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 --whitespace=nowarn], chdir: squash_path, env: env, include_stderr: true) do |stdin|
+          run_git!(%w[apply --index --3way --whitespace=nowarn], chdir: squash_path, env: env, include_stderr: true) do |stdin|
             stdin.binmode
             stdin.write(diff)
           end