config: Set autocrlf to falsejt-autocrlf-config

Gitaly should be agnostic as a datastore and not transform line
endings. Currently by default `core.autocrlf` is set to `input` which
can transform line endings on commit. This change updates the default
global configuration of `core.autocrlf` to `false` which stops Gitaly
from transforming line endings.

7 files changed, 47 insertions, 10 deletions

diff --git a/internal/featureflag/ff_autocrlf_config.go b/internal/featureflag/ff_autocrlf_config.go
new file mode 100644
index 000000000..8f6bbbe19
--- /dev/null
+++ b/internal/featureflag/ff_autocrlf_config.go
@@ -0,0 +1,11 @@
+package featureflag
+
+// AutocrlfConfig changes the default global git configuration of
+// autocrlf from `input` to `false`. This results in Gitaly being
+// completely agnostic of line endings.
+var AutocrlfConfig = NewFeatureFlag(
+	"autocrlf_false",
+	"v16.5.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4425",
+	false,
+)
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index d92fc7c78..ca8f434dd 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -13,6 +13,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/cgroups"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/command"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/alternates"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2hooks"
@@ -568,6 +569,12 @@ func (cf *ExecCommandFactory) combineArgs(ctx context.Context, sc Command, cc cm
 // GlobalConfiguration returns the global Git configuration that should be applied to every Git
 // command.
 func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]ConfigPair, error) {
+	// Feature flag to change the default global configuration of autocrlf.
+	autocrlf := "input"
+	if featureflag.AutocrlfConfig.IsEnabled(ctx) {
+		autocrlf = "false"
+	}
+
 	// As global options may cancel out each other, we have a clearly defined order in which
 	// globals get applied. The order is similar to how git handles configuration options from
 	// most general to most specific. This allows callsites to override options which would
@@ -594,6 +601,12 @@ func (cf *ExecCommandFactory) GlobalConfiguration(ctx context.Context) ([]Config
 		// required for the web editor.
 		{Key: "core.autocrlf", Value: "input"},
 
+		// CRLF line endings will get replaced with LF line endings when writing blobs to the
+		// object database. No conversion is done when reading blobs from the object database.
+		// This is required for the web editor. With feature flag "autocrlf_false" enabled
+		// CRLF line endings will not get replaced and be left alone.
+		{Key: "core.autocrlf", Value: autocrlf},
+
 		// Git allows the use of replace refs, where a given object ID can be replaced with a
 		// different one. The result is that Git commands would use the new object instead of the
 		// old one in almost all contexts. This is a security threat: an adversary may use this
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index f1dfca0b4..60aed1687 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -16,6 +16,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/trace2"
@@ -592,8 +593,12 @@ func TestExecCommandFactory_GitVersion(t *testing.T) {
 
 func TestExecCommandFactory_config(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.AutocrlfConfig).Run(t, testExecCommandFactoryConfig)
+}
+
+func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
-	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
 	// Create a repository and remove its gitconfig to bring us into a known state where there
@@ -603,10 +608,17 @@ func TestExecCommandFactory_config(t *testing.T) {
 	})
 	require.NoError(t, os.Remove(filepath.Join(repoDir, "config")))
 
+	// Feature flag to change the default global configuration of autocrlf.
+	autocrlf := "input"
+	if featureflag.AutocrlfConfig.IsEnabled(ctx) {
+		autocrlf = "false"
+	}
+
 	expectedEnv := []string{
 		"gc.auto=0",
 		"maintenance.auto=0",
 		"core.autocrlf=input",
+		"core.autocrlf=" + autocrlf,
 		"core.usereplacerefs=false",
 		"core.fsync=objects,derived-metadata,reference",
 		"core.fsyncmethod=fsync",
diff --git a/internal/git/localrepo/blob_test.go b/internal/git/localrepo/blob_test.go
index 5534c7328..66e025f70 100644
--- a/internal/git/localrepo/blob_test.go
+++ b/internal/git/localrepo/blob_test.go
@@ -53,9 +53,9 @@ func TestRepo_WriteBlob(t *testing.T) {
 			content: "\n",
 		},
 		{
-			desc:    "CRLF converted to LF due to global git config",
+			desc:    "CRLF not converted to LF due to global git config",
 			input:   strings.NewReader("\r\n"),
-			content: "\n",
+			content: "\r\n",
 		},
 		{
 			desc:       "line endings preserved in binary files",
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts_test.go b/internal/gitaly/service/conflicts/resolve_conflicts_test.go
index 65c5fd71b..8eb90bee3 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts_test.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts_test.go
@@ -771,7 +771,7 @@ func testResolveConflicts(t *testing.T, ctx context.Context) {
 					expectedResponse:     &gitalypb.ResolveConflictsResponse{},
 					expectedContent: map[string]map[string][]byte{
 						"refs/heads/ours": {
-							"a": []byte("A\nB\nX\nD\nE\n"),
+							"a": []byte("A\r\nB\r\nX\r\nD\r\nE\r\n"),
 						},
 					},
 				}
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 6c363732c..6d24a86d0 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -282,7 +282,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
-			desc: "create file normalizes line endings",
+			desc: "create file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -293,7 +293,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 					repoCreated:   true,
 					branchCreated: true,
 					treeEntries: []gittest.TreeEntry{
-						{Mode: DefaultMode, Path: "file-1", Content: "content-1\n content-2\n"},
+						{Mode: DefaultMode, Path: "file-1", Content: "content-1\r\n content-2\r\n"},
 					},
 				},
 			},
@@ -347,7 +347,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
-			desc: "update file normalizes line endings",
+			desc: "update file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -359,7 +359,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 					repoCreated:   true,
 					branchCreated: true,
 					treeEntries: []gittest.TreeEntry{
-						{Mode: DefaultMode, Path: "file-1", Content: "content-2\n"},
+						{Mode: DefaultMode, Path: "file-1", Content: "content-2\r\n"},
 					},
 				},
 			},
@@ -618,7 +618,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
-			desc: "move file normalizes line endings",
+			desc: "move file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -637,7 +637,7 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 						actionContentRequest("new-content\r\n"),
 					},
 					treeEntries: []gittest.TreeEntry{
-						{Mode: DefaultMode, Path: "moved-file", Content: "new-content\n"},
+						{Mode: DefaultMode, Path: "moved-file", Content: "new-content\r\n"},
 					},
 				},
 			},
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index e1a335e07..1e7319d96 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -257,6 +257,7 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.MailmapOptions, rand.Int()%2 == 0)
 	// Randomly enable limiter.resizableSemaphore
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.UseResizableSemaphoreInConcurrencyLimiter, rand.Int()%2 == 0)
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.AutocrlfConfig, true)
 
 	for _, opt := range opts {
 		ctx = opt(ctx)