Refactor update reference with hooks code

This extracts the `update_with_hooks.go` code into the `updateref`
package in preparation for using this code to fix conflict resolution.

Part of https://gitlab.com/gitlab-org/gitlab/-/issues/18917 and split
off from https://gitlab.com/gitlab-org/gitaly/-/merge_requests/3619.

Changelog: changed

12 files changed, 214 insertions, 151 deletions

diff --git a/internal/git/updateref/update_with_hooks.go b/internal/git/updateref/update_with_hooks.go
new file mode 100644
index 000000000..f8bb13841
--- /dev/null
+++ b/internal/git/updateref/update_with_hooks.go
@@ -0,0 +1,169 @@
+package updateref
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/hook"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+)
+
+// UpdaterWithHooks updates a ref with Git hooks.
+type UpdaterWithHooks struct {
+	cfg           config.Cfg
+	hookManager   hook.Manager
+	gitCmdFactory git.CommandFactory
+	catfileCache  catfile.Cache
+}
+
+// PreReceiveError contains an error message for a git pre-receive failure
+type PreReceiveError struct {
+	Message string
+}
+
+func (e PreReceiveError) Error() string {
+	return e.Message
+}
+
+// Error reports an error in git update-ref
+type Error struct {
+	reference string
+}
+
+func (e Error) Error() string {
+	return fmt.Sprintf("Could not update %s. Please refresh and try again.", e.reference)
+}
+
+func hookErrorMessage(sout string, serr string, err error) string {
+	if err != nil && errors.As(err, &hook.NotAllowedError{}) {
+		return err.Error()
+	}
+
+	if len(strings.TrimSpace(serr)) > 0 {
+		return serr
+	}
+
+	return sout
+}
+
+// NewUpdaterWithHooks creates a new instance of a class that will update a Git branch
+func NewUpdaterWithHooks(
+	cfg config.Cfg,
+	hookManager hook.Manager,
+	gitCmdFactory git.CommandFactory,
+	catfileCache catfile.Cache,
+) *UpdaterWithHooks {
+	return &UpdaterWithHooks{
+		cfg:           cfg,
+		hookManager:   hookManager,
+		gitCmdFactory: gitCmdFactory,
+		catfileCache:  catfileCache,
+	}
+}
+
+// UpdateReferenceWithHooks updates a branch with a given commit ID using the Git hooks
+func (u *UpdaterWithHooks) UpdateReferenceWithHooks(
+	ctx context.Context,
+	repo *gitalypb.Repository,
+	user *gitalypb.User,
+	reference git.ReferenceName,
+	newrev, oldrev git.ObjectID,
+	pushOptions ...string,
+) error {
+	var transaction *txinfo.Transaction
+	if tx, err := txinfo.TransactionFromContext(ctx); err == nil {
+		transaction = &tx
+	} else if !errors.Is(err, txinfo.ErrTransactionNotFound) {
+		return err
+	}
+
+	payload, err := git.NewHooksPayload(u.cfg, repo, transaction, &git.ReceiveHooksPayload{
+		UserID:   user.GetGlId(),
+		Username: user.GetGlUsername(),
+		Protocol: "web",
+	}, git.ReceivePackHooks, featureflag.RawFromContext(ctx)).Env()
+	if err != nil {
+		return err
+	}
+
+	if reference == "" {
+		return helper.ErrInternalf("updateReferenceWithHooks: got no reference")
+	}
+	if err := git.ValidateObjectID(oldrev.String()); err != nil {
+		return helper.ErrInternalf("updateReferenceWithHooks: got invalid old value: %w", err)
+	}
+	if err := git.ValidateObjectID(newrev.String()); err != nil {
+		return helper.ErrInternalf("updateReferenceWithHooks: got invalid new value: %w", err)
+	}
+
+	env := []string{
+		payload,
+	}
+
+	changes := fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference)
+	var stdout, stderr bytes.Buffer
+
+	if err := u.hookManager.PreReceiveHook(ctx, repo, pushOptions, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
+		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
+		return PreReceiveError{Message: msg}
+	}
+	if err := u.hookManager.UpdateHook(ctx, repo, reference.String(), oldrev.String(), newrev.String(), env, &stdout, &stderr); err != nil {
+		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
+		return PreReceiveError{Message: msg}
+	}
+
+	if err := u.hookManager.ReferenceTransactionHook(ctx, hook.ReferenceTransactionPrepared, env, strings.NewReader(changes)); err != nil {
+		return PreReceiveError{Message: err.Error()}
+	}
+
+	localRepo := u.localrepo(repo)
+
+	// We are already manually invoking the reference-transaction hook, so there is no need to
+	// set up hooks again here. One could argue that it would be easier to just have git handle
+	// execution of the reference-transaction hook. But unfortunately, it has proven to be
+	// problematic: if we queue a deletion, and the reference to be deleted exists both as
+	// packed-ref and as loose ref, then we would see two transactions: first a transaction
+	// deleting the packed-ref which would otherwise get unshadowed by deleting the loose ref,
+	// and only then do we see the deletion of the loose ref. So this depends on how well a repo
+	// is packed, which is obviously a bad thing as Gitaly nodes may be differently packed. We
+	// thus continue to manually drive the reference-transaction hook here, which doesn't have
+	// this problem.
+	updater, err := New(ctx, u.cfg, localRepo, WithDisabledTransactions())
+	if err != nil {
+		return err
+	}
+
+	if err := updater.Update(reference, newrev.String(), oldrev.String()); err != nil {
+		return err
+	}
+
+	if err := updater.Wait(); err != nil {
+		return Error{reference: reference.String()}
+	}
+
+	if err := u.hookManager.ReferenceTransactionHook(ctx, hook.ReferenceTransactionCommitted, env, strings.NewReader(changes)); err != nil {
+		return PreReceiveError{Message: err.Error()}
+	}
+
+	if err := u.hookManager.PostReceiveHook(ctx, repo, pushOptions, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
+		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
+		return PreReceiveError{Message: msg}
+	}
+
+	return nil
+}
+
+func (u *UpdaterWithHooks) localrepo(repo repository.GitRepo) *localrepo.Repo {
+	return localrepo.New(u.gitCmdFactory, u.catfileCache, repo, u.cfg)
+}
diff --git a/internal/gitaly/service/operations/update_with_hooks_test.go b/internal/git/updateref/update_with_hooks_test.go
index 404576810..831630183 100644
--- a/internal/gitaly/service/operations/update_with_hooks_test.go
+++ b/internal/git/updateref/update_with_hooks_test.go
@@ -1,4 +1,4 @@
-package operations
+package updateref
 
 import (
 	"context"
@@ -63,7 +63,7 @@ func TestUpdateReferenceWithHooks_invalidParameters(t *testing.T) {
 
 	revA, revB := git.ObjectID(strings.Repeat("a", 40)), git.ObjectID(strings.Repeat("b", 40))
 
-	server := NewServer(cfg, nil, &mockHookManager{}, nil, nil, nil, nil)
+	updater := NewUpdaterWithHooks(cfg, &mockHookManager{}, nil, nil)
 
 	testCases := []struct {
 		desc           string
@@ -107,7 +107,7 @@ func TestUpdateReferenceWithHooks_invalidParameters(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			err := server.updateReferenceWithHooks(ctx, repo, user, tc.ref, tc.newRev, tc.oldRev)
+			err := updater.UpdateReferenceWithHooks(ctx, repo, user, tc.ref, tc.newRev, tc.oldRev)
 			require.Contains(t, err.Error(), tc.expectedErr)
 		})
 	}
@@ -275,9 +275,9 @@ func TestUpdateReferenceWithHooks(t *testing.T) {
 			}
 
 			gitCmdFactory := git.NewExecCommandFactory(cfg)
-			hookServer := NewServer(cfg, nil, hookManager, nil, nil, gitCmdFactory, nil)
+			updater := NewUpdaterWithHooks(cfg, hookManager, gitCmdFactory, nil)
 
-			err := hookServer.updateReferenceWithHooks(ctx, repo, user, git.ReferenceName("refs/heads/master"), git.ZeroOID, git.ObjectID(oldRev))
+			err := updater.UpdateReferenceWithHooks(ctx, repo, user, git.ReferenceName("refs/heads/master"), git.ZeroOID, git.ObjectID(oldRev))
 			if tc.expectedErr == "" {
 				require.NoError(t, err)
 			} else {
diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go
index 0839e2958..ba83ceef8 100644
--- a/internal/gitaly/service/operations/branches.go
+++ b/internal/gitaly/service/operations/branches.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -51,14 +52,14 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, startPointOID, git.ZeroOID); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserCreateBranchResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			return nil, status.Error(codes.FailedPrecondition, err.Error())
 		}
@@ -113,10 +114,10 @@ func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateB
 	referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName))
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, newOID, oldOID); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserUpdateBranchResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
@@ -152,14 +153,14 @@ func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteB
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, git.ZeroOID, referenceValue); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserDeleteBranchResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			return nil, status.Error(codes.FailedPrecondition, err.Error())
 		}
diff --git a/internal/gitaly/service/operations/cherry_pick.go b/internal/gitaly/service/operations/cherry_pick.go
index dbf94f0b8..1bfd3bc90 100644
--- a/internal/gitaly/service/operations/cherry_pick.go
+++ b/internal/gitaly/service/operations/cherry_pick.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/golang/protobuf/ptypes"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -106,7 +107,7 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, newrev, oldrev); err != nil {
-		if errors.As(err, &preReceiveError{}) {
+		if errors.As(err, &updateref.PreReceiveError{}) {
 			return &gitalypb.UserCherryPickResponse{
 				PreReceiveError: err.Error(),
 			}, nil
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index b92e82b0d..33011e2f3 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -16,6 +16,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/remoterepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitalyssh"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
@@ -68,7 +69,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile
 		var (
 			response        gitalypb.UserCommitFilesResponse
 			indexError      git2go.IndexError
-			preReceiveError preReceiveError
+			preReceiveError updateref.PreReceiveError
 		)
 
 		switch {
@@ -322,7 +323,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, header.Repository, header.User, targetBranchName, commitID, oldRevision); err != nil {
-		if errors.As(err, &updateRefError{}) {
+		if errors.As(err, &updateref.Error{}) {
 			return status.Errorf(codes.FailedPrecondition, err.Error())
 		}
 
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index f227f0efd..9b6c925f3 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/golang/protobuf/ptypes"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -103,12 +104,12 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, firstRequest.Repository, firstRequest.User, referenceName, mergeOID, revision); err != nil {
-		var preReceiveError preReceiveError
-		var updateRefError updateRefError
+		var preReceiveError updateref.PreReceiveError
+		var updateRefError updateref.Error
 
 		if errors.As(err, &preReceiveError) {
 			err = stream.Send(&gitalypb.UserMergeBranchResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			})
 		} else if errors.As(err, &updateRefError) {
 			// When an error happens updating the reference, e.g. because of a race
@@ -176,14 +177,14 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, in.Repository, in.User, referenceName, commitID, revision); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserFFBranchResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			// When an error happens updating the reference, e.g. because of a race
 			// with another update, then Ruby code didn't send an error but just an
diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go
index 864b64a89..06308d2c0 100644
--- a/internal/gitaly/service/operations/rebase.go
+++ b/internal/gitaly/service/operations/rebase.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/golang/protobuf/ptypes"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -97,7 +98,7 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
 		oldrev,
 		header.GitPushOptions...); err != nil {
 		switch {
-		case errors.As(err, &preReceiveError{}):
+		case errors.As(err, &updateref.PreReceiveError{}):
 			return stream.Send(&gitalypb.UserRebaseConfirmableResponse{
 				PreReceiveError: err.Error(),
 			})
diff --git a/internal/gitaly/service/operations/revert.go b/internal/gitaly/service/operations/revert.go
index 03047cbdd..715e7b0c1 100644
--- a/internal/gitaly/service/operations/revert.go
+++ b/internal/gitaly/service/operations/revert.go
@@ -9,6 +9,7 @@ import (
 	"github.com/golang/protobuf/ptypes"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/remoterepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -104,10 +105,10 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, newrev, oldrev); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserRevertResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
diff --git a/internal/gitaly/service/operations/server.go b/internal/gitaly/service/operations/server.go
index 7f560cbfb..a3720dc5c 100644
--- a/internal/gitaly/service/operations/server.go
+++ b/internal/gitaly/service/operations/server.go
@@ -6,6 +6,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/hook"
@@ -24,6 +25,7 @@ type Server struct {
 	git2go        git2go.Executor
 	gitCmdFactory git.CommandFactory
 	catfileCache  catfile.Cache
+	updater       *updateref.UpdaterWithHooks
 }
 
 // NewServer creates a new instance of a grpc OperationServiceServer
@@ -45,6 +47,7 @@ func NewServer(
 		git2go:        git2go.New(cfg.BinDir, cfg.Git.BinPath),
 		gitCmdFactory: gitCmdFactory,
 		catfileCache:  catfileCache,
+		updater:       updateref.NewUpdaterWithHooks(cfg, hookManager, gitCmdFactory, catfileCache),
 	}
 }
 
diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go
index 3bae9a40f..2d7117450 100644
--- a/internal/gitaly/service/operations/submodules.go
+++ b/internal/gitaly/service/operations/submodules.go
@@ -11,6 +11,7 @@ import (
 	"github.com/golang/protobuf/ptypes"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -150,14 +151,14 @@ func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda
 		commitID,
 		branchOID,
 	); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserUpdateSubmoduleResponse{
 				PreReceiveError: preReceiveError.Error(),
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			return &gitalypb.UserUpdateSubmoduleResponse{
 				CommitError: err.Error(),
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index ed8251681..22e55158f 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -12,6 +12,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -34,14 +35,14 @@ func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagR
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, git.ZeroOID, revision); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserDeleteTagResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			return nil, status.Error(codes.FailedPrecondition, err.Error())
 		}
@@ -191,14 +192,14 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR
 
 	referenceName := git.ReferenceName(fmt.Sprintf("refs/tags/%s", req.TagName))
 	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, refObjectID, git.ZeroOID); err != nil {
-		var preReceiveError preReceiveError
+		var preReceiveError updateref.PreReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserCreateTagResponse{
-				PreReceiveError: preReceiveError.message,
+				PreReceiveError: preReceiveError.Message,
 			}, nil
 		}
 
-		var updateRefError updateRefError
+		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			refNameOK, err := git.CheckRefFormat(ctx, s.gitCmdFactory, referenceName.String())
 			if refNameOK {
diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go
index d0a545efe..b4ada473c 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -1,49 +1,12 @@
 package operations
 
 import (
-	"bytes"
 	"context"
-	"errors"
-	"fmt"
-	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git/updateref"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/hook"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/txinfo"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
-type preReceiveError struct {
-	message string
-}
-
-func (e preReceiveError) Error() string {
-	return e.message
-}
-
-type updateRefError struct {
-	reference string
-}
-
-func (e updateRefError) Error() string {
-	return fmt.Sprintf("Could not update %s. Please refresh and try again.", e.reference)
-}
-
-func hookErrorMessage(sout string, serr string, err error) string {
-	if err != nil && errors.As(err, &hook.NotAllowedError{}) {
-		return err.Error()
-	}
-
-	if len(strings.TrimSpace(serr)) > 0 {
-		return serr
-	}
-
-	return sout
-}
-
 func (s *Server) updateReferenceWithHooks(
 	ctx context.Context,
 	repo *gitalypb.Repository,
@@ -52,85 +15,5 @@ func (s *Server) updateReferenceWithHooks(
 	newrev, oldrev git.ObjectID,
 	pushOptions ...string,
 ) error {
-	var transaction *txinfo.Transaction
-	if tx, err := txinfo.TransactionFromContext(ctx); err == nil {
-		transaction = &tx
-	} else if !errors.Is(err, txinfo.ErrTransactionNotFound) {
-		return err
-	}
-
-	payload, err := git.NewHooksPayload(s.cfg, repo, transaction, &git.ReceiveHooksPayload{
-		UserID:   user.GetGlId(),
-		Username: user.GetGlUsername(),
-		Protocol: "web",
-	}, git.ReceivePackHooks, featureflag.RawFromContext(ctx)).Env()
-	if err != nil {
-		return err
-	}
-
-	if reference == "" {
-		return helper.ErrInternalf("updateReferenceWithHooks: got no reference")
-	}
-	if err := git.ValidateObjectID(oldrev.String()); err != nil {
-		return helper.ErrInternalf("updateReferenceWithHooks: got invalid old value: %w", err)
-	}
-	if err := git.ValidateObjectID(newrev.String()); err != nil {
-		return helper.ErrInternalf("updateReferenceWithHooks: got invalid new value: %w", err)
-	}
-
-	env := []string{
-		payload,
-	}
-
-	changes := fmt.Sprintf("%s %s %s\n", oldrev, newrev, reference)
-	var stdout, stderr bytes.Buffer
-
-	if err := s.hookManager.PreReceiveHook(ctx, repo, pushOptions, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
-		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
-		return preReceiveError{message: msg}
-	}
-	if err := s.hookManager.UpdateHook(ctx, repo, reference.String(), oldrev.String(), newrev.String(), env, &stdout, &stderr); err != nil {
-		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
-		return preReceiveError{message: msg}
-	}
-
-	if err := s.hookManager.ReferenceTransactionHook(ctx, hook.ReferenceTransactionPrepared, env, strings.NewReader(changes)); err != nil {
-		return preReceiveError{message: err.Error()}
-	}
-
-	localRepo := s.localrepo(repo)
-
-	// We are already manually invoking the reference-transaction hook, so there is no need to
-	// set up hooks again here. One could argue that it would be easier to just have git handle
-	// execution of the reference-transaction hook. But unfortunately, it has proven to be
-	// problematic: if we queue a deletion, and the reference to be deleted exists both as
-	// packed-ref and as loose ref, then we would see two transactions: first a transaction
-	// deleting the packed-ref which would otherwise get unshadowed by deleting the loose ref,
-	// and only then do we see the deletion of the loose ref. So this depends on how well a repo
-	// is packed, which is obviously a bad thing as Gitaly nodes may be differently packed. We
-	// thus continue to manually drive the reference-transaction hook here, which doesn't have
-	// this problem.
-	updater, err := updateref.New(ctx, s.cfg, localRepo, updateref.WithDisabledTransactions())
-	if err != nil {
-		return err
-	}
-
-	if err := updater.Update(reference, newrev.String(), oldrev.String()); err != nil {
-		return err
-	}
-
-	if err := updater.Wait(); err != nil {
-		return updateRefError{reference: reference.String()}
-	}
-
-	if err := s.hookManager.ReferenceTransactionHook(ctx, hook.ReferenceTransactionCommitted, env, strings.NewReader(changes)); err != nil {
-		return preReceiveError{message: err.Error()}
-	}
-
-	if err := s.hookManager.PostReceiveHook(ctx, repo, pushOptions, env, strings.NewReader(changes), &stdout, &stderr); err != nil {
-		msg := hookErrorMessage(stdout.String(), stderr.String(), err)
-		return preReceiveError{message: msg}
-	}
-
-	return nil
+	return s.updater.UpdateReferenceWithHooks(ctx, repo, user, reference, newrev, oldrev, pushOptions...)
 }