Merge branch 'rebase-sparse-filelist' into 'master'

Fix sparse checkout file list for rebase onto remote branch

Closes #2759

See merge request gitlab-org/gitaly!2447

3 files changed, 91 insertions, 25 deletions

diff --git a/changelogs/unreleased/rebase-sparse-filelist.yml b/changelogs/unreleased/rebase-sparse-filelist.yml
new file mode 100644
index 000000000..1fe6d3944
--- /dev/null
+++ b/changelogs/unreleased/rebase-sparse-filelist.yml
@@ -0,0 +1,5 @@
+---
+title: Fix sparse checkout file list for rebase onto remote branch
+merge_request: 2447
+author:
+type: fixed
diff --git a/internal/service/operations/rebase_test.go b/internal/service/operations/rebase_test.go
index 9967a5ae9..c5d640624 100644
--- a/internal/service/operations/rebase_test.go
+++ b/internal/service/operations/rebase_test.go
@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
@@ -470,6 +471,71 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) {
 	require.True(t, secondResponse.GetRebaseApplied(), "the second rebase is applied")
 }
 
+func TestRebaseOntoRemoteBranch(t *testing.T) {
+	serverSocketPath, stop := runOperationServiceServer(t)
+	defer stop()
+
+	client, conn := newOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	localRepo, localRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	remoteRepo, remoteRepoPath, cleanup := testhelper.NewTestRepoWithWorktree(t)
+	defer cleanup()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	ctx = metadata.NewOutgoingContext(ctx, md)
+
+	localBranch := "master"
+	localBranchHash := getBranchSha(t, localRepoPath, localBranch)
+
+	remoteBranch := "remote-branch"
+	testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "config", "user.name", string(testhelper.TestUser.Name))
+	testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "config", "user.email", string(testhelper.TestUser.Email))
+	testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "checkout", "-b", remoteBranch, "master")
+	testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "rm", "README")
+	testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "commit", "-a", "-m", "remove README")
+	remoteBranchHash := getBranchSha(t, remoteRepoPath, remoteBranch)
+
+	rebaseStream, err := client.UserRebaseConfirmable(ctx)
+	require.NoError(t, err)
+
+	_, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash)
+	require.True(t, catfile.IsNotFound(err), "remote commit does not yet exist in local repository")
+
+	headerRequest := buildHeaderRequest(localRepo, testhelper.TestUser, "1", localBranch, localBranchHash, remoteRepo, remoteBranch)
+	require.NoError(t, rebaseStream.Send(headerRequest), "send header")
+
+	firstResponse, err := rebaseStream.Recv()
+	require.NoError(t, err, "receive first response")
+
+	_, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash)
+	require.NoError(t, err, "remote commit does now exist in local repository")
+
+	applyRequest := buildApplyRequest(true)
+	require.NoError(t, rebaseStream.Send(applyRequest), "apply rebase")
+
+	secondResponse, err := rebaseStream.Recv()
+	require.NoError(t, err, "receive second response")
+
+	err = testhelper.ReceiveEOFWithTimeout(func() error {
+		_, err = rebaseStream.Recv()
+		return err
+	})
+	require.NoError(t, err, "consume EOF")
+
+	rebasedBranchHash := getBranchSha(t, localRepoPath, localBranch)
+
+	require.NotEqual(t, rebasedBranchHash, localBranchHash)
+	require.Equal(t, rebasedBranchHash, firstResponse.GetRebaseSha())
+
+	require.True(t, secondResponse.GetRebaseApplied(), "the second rebase is applied")
+}
+
 func rebaseRecvTimeout(bidi gitalypb.OperationService_UserRebaseConfirmableClient, timeout time.Duration) (*gitalypb.UserRebaseConfirmableResponse, error) {
 	type responseError struct {
 		response *gitalypb.UserRebaseConfirmableResponse
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 43c0b2a85..e34683e12 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -372,35 +372,30 @@ module Gitlab
         worktree = Gitlab::Git::Worktree.new(path, REBASE_WORKTREE_PREFIX, rebase_id)
         env = git_env.merge(user.git_env)
 
-        if remote_repository.is_a?(RemoteRepository)
-          env.merge!(remote_repository.fetch_env)
-          remote_repo_path = GITALY_INTERNAL_URL
-        else
-          remote_repo_path = remote_repository.path
-        end
-
-        diff_range = "#{remote_branch}...#{branch}"
-        diff_files = begin
-                       run_git!(
-                         %W[diff --name-only #{diff_range}]
-                       ).chomp
-                     rescue GitError
-                       []
-                     end
-
-        with_worktree(worktree, branch, sparse_checkout_files: diff_files, env: env) do
-          run_git!(
-            %W[pull --rebase #{remote_repo_path} #{remote_branch}],
-            chdir: worktree.path, env: env, include_stderr: true
-          )
+        with_repo_branch_commit(remote_repository, remote_branch) do |commit|
+          diff_range = "#{commit.sha}...#{branch}"
+          diff_files = begin
+                         run_git!(
+                           %W[diff --name-only #{diff_range}]
+                         ).chomp
+                       rescue GitError
+                         []
+                       end
+
+          with_worktree(worktree, branch, sparse_checkout_files: diff_files, env: env) do
+            run_git!(
+              %W[rebase #{commit.sha}],
+              chdir: worktree.path, env: env, include_stderr: true
+            )
 
-          rebase_sha = run_git!(%w[rev-parse HEAD], chdir: worktree.path, env: env).strip
+            rebase_sha = run_git!(%w[rev-parse HEAD], chdir: worktree.path, env: env).strip
 
-          yield rebase_sha if block_given?
+            yield rebase_sha if block_given?
 
-          update_branch(branch, user: user, newrev: rebase_sha, oldrev: branch_sha, push_options: push_options)
+            update_branch(branch, user: user, newrev: rebase_sha, oldrev: branch_sha, push_options: push_options)
 
-          rebase_sha
+            rebase_sha
+          end
         end
       end