From 3e0a09f1f940eeda81df8c797a62bc918127e874 Mon Sep 17 00:00:00 2001
From: Sami Hiltunen <shiltunen@gitlab.com>
Date: Wed, 16 Dec 2020 14:16:38 +0100
Subject: [PATCH 1/2] stub out hook manager when hooks are disabled in tests

Gitaly has a `GITALY_TESTING_NO_GIT_HOOKS` environment variable
to disable git hooks during tests. Currently, it only skips configuring
the interal API client but leaves the hook manager in place. This causes
nil pointer dereferencing panics when executing code that would invoke
hooks when the environment variable is set.

This commit fixes the problem by stubbing out the whole hook manager
when the environment variable is set.
---
 cmd/gitaly/main.go                       |  9 ++++---
 internal/gitaly/hook/disabled_manager.go | 31 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 internal/gitaly/hook/disabled_manager.go

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 73f1a38fa7c..eb43165752d 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -127,6 +127,7 @@ func run(b *bootstrap.Bootstrap) error {
 	var gitlabAPI hook.GitlabAPI
 	var err error
 
+	hookManager := hook.Manager(hook.DisabledManager{})
 	if config.SkipHooks() {
 		log.Warn("skipping GitLab API client creation since hooks are bypassed via GITALY_TESTING_NO_GIT_HOOKS")
 	} else {
@@ -134,10 +135,12 @@ func run(b *bootstrap.Bootstrap) error {
 		if err != nil {
 			log.Fatalf("could not create GitLab API client: %v", err)
 		}
-	}
 
-	hookManager := hook.NewManager(config.NewLocator(config.Config), gitlabAPI, config.Config)
-	prometheus.MustRegister(hookManager)
+		hm := hook.NewManager(config.NewLocator(config.Config), gitlabAPI, config.Config)
+		prometheus.MustRegister(hm)
+
+		hookManager = hm
+	}
 
 	conns := client.NewPoolWithOptions(
 		client.WithDialer(client.HealthCheckDialer(client.DialContext)),
diff --git a/internal/gitaly/hook/disabled_manager.go b/internal/gitaly/hook/disabled_manager.go
new file mode 100644
index 00000000000..192739aefd7
--- /dev/null
+++ b/internal/gitaly/hook/disabled_manager.go
@@ -0,0 +1,31 @@
+package hook
+
+import (
+	"context"
+	"io"
+
+	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+)
+
+// DisabledManager never executes hooks and simply returns a nil error.
+type DisabledManager struct{}
+
+// PreReceiveHook ignores its parameters and returns a nil error.
+func (DisabledManager) PreReceiveHook(context.Context, *gitalypb.Repository, []string, io.Reader, io.Writer, io.Writer) error {
+	return nil
+}
+
+// PostReceiveHook ignores its parameters and returns a nil error.
+func (DisabledManager) PostReceiveHook(context.Context, *gitalypb.Repository, []string, []string, io.Reader, io.Writer, io.Writer) error {
+	return nil
+}
+
+// UpdateHook ignores its parameters and returns a nil error.
+func (DisabledManager) UpdateHook(context.Context, *gitalypb.Repository, string, string, string, []string, io.Writer, io.Writer) error {
+	return nil
+}
+
+// ReferenceTransactionHook ignores its parameters and returns a nil error.
+func (DisabledManager) ReferenceTransactionHook(context.Context, ReferenceTransactionState, []string, io.Reader) error {
+	return nil
+}
-- 
GitLab


From 9fdabd0652159aecb94404ed1a85fae5ca7737b6 Mon Sep 17 00:00:00 2001
From: Sami Hiltunen <shiltunen@gitlab.com>
Date: Wed, 16 Dec 2020 14:19:48 +0100
Subject: [PATCH 2/2] Revert "Merge branch 'smh-revert-user-commit-files' into
 'master'"

This reverts commit bca59804605b4afd0bebacbaa0952c5b4ca16141, reversing
changes made to 97b0280d97d847e9c09948d17587cc20398907c0.
---
 changelogs/unreleased/smh-enable-user-commit-files.yml | 5 +++++
 internal/metadata/featureflag/feature_flags.go         | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/smh-enable-user-commit-files.yml

diff --git a/changelogs/unreleased/smh-enable-user-commit-files.yml b/changelogs/unreleased/smh-enable-user-commit-files.yml
new file mode 100644
index 00000000000..2334da4d3e4
--- /dev/null
+++ b/changelogs/unreleased/smh-enable-user-commit-files.yml
@@ -0,0 +1,5 @@
+---
+title: Enable Go port of UserCommitFiles by default
+merge_request: 2922
+author:
+type: performance
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index d4f33cc582b..d9e2fb9f293 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -28,7 +28,7 @@ var (
 	// GoUserSquash enables the Go implementation of UserSquash
 	GoUserSquash = FeatureFlag{Name: "go_user_squash", OnByDefault: true}
 	// GoUserCommitFiles enables the Go implementation of UserCommitFiles
-	GoUserCommitFiles = FeatureFlag{Name: "go_user_commit_files", OnByDefault: false}
+	GoUserCommitFiles = FeatureFlag{Name: "go_user_commit_files", OnByDefault: true}
 	// GoResolveConflicts enables the Go implementation of ResolveConflicts
 	GoResolveConflicts = FeatureFlag{Name: "go_resolve_conflicts", OnByDefault: false}
 	// GoUserUpdateSubmodule enables the Go implementation of
-- 
GitLab