Merge branch 'jc/remove-user-commit-files-ff' into 'master'

operations: Remove CommitFilesInGit feature flag

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5984

Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>

3 files changed, 8 insertions, 80 deletions

diff --git a/internal/featureflag/ff_commit_files_in_git.go b/internal/featureflag/ff_commit_files_in_git.go
deleted file mode 100644
index 32ad53869..000000000
--- a/internal/featureflag/ff_commit_files_in_git.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// CommitFilesInGit enables implementation of UserCommitFiles using
-// git plumbing commands instead of git2go
-var CommitFilesInGit = NewFeatureFlag(
-	"commit_files_in_git",
-	"16.1.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/5019",
-	false,
-)
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index 8f287ee44..d2617e39c 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/remoterepo"
@@ -410,40 +409,6 @@ func (s *Server) userCommitFilesGit(
 	)
 }
 
-func (s *Server) userCommitFilesGit2Go(
-	ctx context.Context,
-	header *gitalypb.UserCommitFilesRequestHeader,
-	parentCommitOID git.ObjectID,
-	quarantineRepo *localrepo.Repo,
-	repoPath string,
-	actions []git2go.Action,
-) (git.ObjectID, error) {
-	now, err := dateFromProto(header)
-	if err != nil {
-		return "", structerr.NewInvalidArgument("%w", err)
-	}
-
-	committer := git2go.NewSignature(string(header.User.Name), string(header.User.Email), now)
-	author := committer
-	if len(header.CommitAuthorName) > 0 && len(header.CommitAuthorEmail) > 0 {
-		author = git2go.NewSignature(string(header.CommitAuthorName), string(header.CommitAuthorEmail), now)
-	}
-
-	commitID, err := s.git2goExecutor.Commit(ctx, quarantineRepo, git2go.CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    string(header.CommitMessage),
-		Parent:     parentCommitOID.String(),
-		Actions:    actions,
-	})
-	if err != nil {
-		return "", fmt.Errorf("%w", err)
-	}
-
-	return commitID, nil
-}
-
 func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommitFilesRequestHeader, stream gitalypb.OperationService_UserCommitFilesServer) error {
 	quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, header.GetRepository())
 	if err != nil {
@@ -606,28 +571,14 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 		}
 	}
 
-	var commitID git.ObjectID
-
-	if featureflag.CommitFilesInGit.IsEnabled(ctx) {
-		commitID, err = s.userCommitFilesGit(
-			ctx,
-			header,
-			parentCommitOID,
-			quarantineRepo,
-			repoPath,
-			actions,
-		)
-	} else {
-		commitID, err = s.userCommitFilesGit2Go(
-			ctx,
-			header,
-			parentCommitOID,
-			quarantineRepo,
-			repoPath,
-			actions,
-		)
-	}
-
+	commitID, err := s.userCommitFilesGit(
+		ctx,
+		header,
+		parentCommitOID,
+		quarantineRepo,
+		repoPath,
+		actions,
+	)
 	if err != nil {
 		if errors.Is(err, localrepo.ErrDisallowedCharacters) {
 			return structerr.NewInvalidArgument("%w", ErrSignatureMissingNameOrEmail)
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 92bef38bb..ebc4a2bb6 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -35,7 +35,6 @@ func TestUserCommitFiles(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testUserCommitFiles)
 }
@@ -975,7 +974,6 @@ func TestUserCommitFilesStableCommitID(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testUserCommitFilesStableCommitID)
 }
@@ -1039,7 +1037,6 @@ func TestUserCommitFilesQuarantine(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testUserCommitFilesQuarantine)
 }
@@ -1097,7 +1094,6 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessfulUserCommitFilesRequest)
@@ -1260,7 +1256,6 @@ func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessfulUserCommitFilesRequestMove)
@@ -1325,7 +1320,6 @@ func TestSuccessUserCommitFilesRequestForceCommit(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessUserCommitFilesRequestForceCommit)
@@ -1377,7 +1371,6 @@ func TestSuccessUserCommitFilesRequestStartSha(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessUserCommitFilesRequestStartSha)
@@ -1417,7 +1410,6 @@ func TestSuccessUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
@@ -1429,7 +1421,6 @@ func TestSuccessUserCommitFilesRequestStartBranchRemoteRepository(t *testing.T)
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessfulUserCommitFilesRemoteRepositoryRequest(func(header *gitalypb.UserCommitFilesRequest) {
@@ -1479,7 +1470,6 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).
 		Run(t, testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature)
@@ -1537,7 +1527,6 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testFailedUserCommitFilesRequestDueToHooks)
 }
@@ -1592,7 +1581,6 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testFailedUserCommitFilesRequestDueToIndexError)
 }
@@ -1671,7 +1659,6 @@ func TestFailedUserCommitFilesRequest(t *testing.T) {
 	t.Parallel()
 
 	testhelper.NewFeatureSets(
-		featureflag.CommitFilesInGit,
 		featureflag.GPGSigning,
 	).Run(t, testFailedUserCommitFilesRequest)
 }