operations: Support worktreeless squashing for non-fast-forward merges

With 640b4de09 (operations: Implement squashing without worktrees,
2021-07-19), we have changed the implementation of UserSquash to support
worktreeless squashing. The way this is implemented is that we simply
take the tree of the end commit and the parents of the start commit,
where the result is then the squashed commit. The initial assumption was
that we're always going to have only fast-foward mergeable histories
where the end commit is a direct descendant of the start commit. As it
turned out, this assumption was wrong, which is why this implementation
was special-cased in 0cbd9335b (operations: Squash via patches if
commits are not directly related, 2021-07-20) to only apply for
fast-forward mergeable commits.

With this change in place, we now only use worktreeless squashes in a
subset of cases. We can generalize the current implementation though: if
start and end commit do not have a direct parent-child relationship,
then we must somehow update the child to be on top of the parent. This
is exactly what rebases do: by rebasing the end commit on top of the
start commit, we can use the above implementation for non-fast-forward
mergeable histories. And luckily, we already have in-memory-rebases
which do not require worktrees via git2go.

This commit thus generalizes worktreeless squashing by always doing a
rebase of the end commit on top of the start commit.

Changelog: performance

1 files changed, 29 insertions, 18 deletions

diff --git a/internal/gitaly/service/operations/squash.go b/internal/gitaly/service/operations/squash.go
index 1218fd52a..6b9470fb1 100644
--- a/internal/gitaly/service/operations/squash.go
+++ b/internal/gitaly/service/operations/squash.go
@@ -16,6 +16,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/alternates"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
@@ -25,6 +26,7 @@ import (
 const (
 	squashWorktreePrefix  = "squash"
 	gitlabWorktreesSubDir = "gitlab-worktree"
+	ambiguousArgumentFmt  = "fatal: ambiguous argument '%s...%s': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\n"
 )
 
 func (s *Server) UserSquash(ctx context.Context, req *gitalypb.UserSquashRequest) (*gitalypb.UserSquashResponse, error) {
@@ -120,22 +122,12 @@ func (er gitError) Error() string {
 }
 
 func (s *Server) userSquash(ctx context.Context, req *gitalypb.UserSquashRequest, env []string, repoPath string) (string, error) {
-	// In case there is an error, we silently ignore it and assume that the commits do not have
-	// a simple parent-child relationship. Ignoring this error is fine igven that we then fall
-	// back to doing a squash with a worktree.
-	err := s.localrepo(req.GetRepository()).ExecAndWait(ctx, git.SubCmd{
-		Name: "merge-base",
-		Flags: []git.Option{
-			git.Flag{Name: "--is-ancestor"},
-		},
-		Args: []string{
-			req.GetStartSha(),
-			req.GetEndSha(),
-		},
-	})
-
-	if err == nil && featureflag.UserSquashWithoutWorktree.IsEnabled(ctx) {
+	if featureflag.UserSquashWithoutWorktree.IsEnabled(ctx) {
 		repo := s.localrepo(req.GetRepository())
+		repoPath, err := s.locator.GetRepoPath(repo)
+		if err != nil {
+			return "", helper.ErrInternalf("cannot resolve repo path: %w", err)
+		}
 
 		// We need to retrieve the start commit such that we can create the new commit with
 		// all parents of the start commit.
@@ -145,18 +137,18 @@ func (s *Server) userSquash(ctx context.Context, req *gitalypb.UserSquashRequest
 				// This error is simply for backwards compatibility. We should just
 				// return the plain error eventually.
 				Err:    err,
-				ErrMsg: fmt.Sprintf("fatal: ambiguous argument '%s...%s'", req.GetStartSha(), req.GetEndSha()),
+				ErrMsg: fmt.Sprintf(ambiguousArgumentFmt, req.GetStartSha(), req.GetEndSha()),
 			})
 		}
 
 		// And we need to take the tree of the end commit. This tree already is the result
-		treeID, err := repo.ResolveRevision(ctx, git.Revision(req.GetEndSha()+"^{tree}"))
+		endCommit, err := repo.ResolveRevision(ctx, git.Revision(req.GetEndSha()+"^{commit}"))
 		if err != nil {
 			return "", fmt.Errorf("cannot resolve end commit's tree: %w", gitError{
 				// This error is simply for backwards compatibility. We should just
 				// return the plain error eventually.
 				Err:    err,
-				ErrMsg: fmt.Sprintf("fatal: ambiguous argument '%s...%s'", req.GetStartSha(), req.GetEndSha()),
+				ErrMsg: fmt.Sprintf(ambiguousArgumentFmt, req.GetStartSha(), req.GetEndSha()),
 			})
 		}
 
@@ -165,6 +157,25 @@ func (s *Server) userSquash(ctx context.Context, req *gitalypb.UserSquashRequest
 			return "", helper.ErrInvalidArgument(err)
 		}
 
+		// We're now rebasing the end commit on top of the start commit. The resulting tree
+		// is then going to be the tree of the squashed commit.
+		rebasedCommitID, err := s.git2go.Rebase(ctx, repo, git2go.RebaseCommand{
+			Repository: repoPath,
+			Committer: git2go.NewSignature(
+				string(req.GetUser().Name), string(req.GetUser().Email), commitDate,
+			),
+			CommitID:         endCommit,
+			UpstreamCommitID: startCommit,
+		})
+		if err != nil {
+			return "", fmt.Errorf("rebasing end onto start commit: %w", err)
+		}
+
+		treeID, err := repo.ResolveRevision(ctx, rebasedCommitID.Revision()+"^{tree}")
+		if err != nil {
+			return "", fmt.Errorf("cannot resolve rebased tree: %w", err)
+		}
+
 		commitEnv := []string{
 			"GIT_COMMITTER_NAME=" + string(req.GetUser().Name),
 			"GIT_COMMITTER_EMAIL=" + string(req.GetUser().Email),