Merge branch 'jt-update-autocrlf-config' into 'master'

config: Set autocrlf to false

Closes #3476

See merge request gitlab-org/gitaly!4801

6 files changed, 51 insertions, 17 deletions

diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 77ff1976c..ca343652c 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 )
 
 // CommandFactory is designed to create and run git commands in a protected and fully managed manner.
@@ -497,6 +498,12 @@ func (cf *ExecCommandFactory) globalConfiguration(ctx context.Context) ([]Global
 		return nil, fmt.Errorf("determining Git version: %w", err)
 	}
 
+	// 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
@@ -514,11 +521,11 @@ func (cf *ExecCommandFactory) globalConfiguration(ctx context.Context) ([]Global
 		// of it ourselves.
 		ConfigPair{Key: "gc.auto", Value: "0"},
 
-		// 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.
-		ConfigPair{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.
+		ConfigPair{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
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index 76dccbb7d..7414f3bbf 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -4,6 +4,7 @@ package git_test
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -471,8 +472,10 @@ func TestExecCommandFactory_GitVersion(t *testing.T) {
 
 func TestExecCommandFactory_config(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.AutocrlfConfig).Run(t, testExecCommandFactoryConfig)
+}
 
-	ctx := testhelper.Context(t)
+func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) {
 	cfg := testcfg.Build(t)
 
 	// Create a repository and remove its gitconfig to bring us into a known state where there
@@ -482,9 +485,15 @@ 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"
+	}
+
 	commonEnv := []string{
 		"gc.auto=0",
-		"core.autocrlf=input",
+		"core.autocrlf=" + autocrlf,
 		"core.usereplacerefs=false",
 		"commitgraph.generationversion=1",
 	}
@@ -535,17 +544,25 @@ func TestExecCommandFactory_config(t *testing.T) {
 
 func TestExecCommandFactory_SidecarGitConfiguration(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.AutocrlfConfig).Run(t, testExecCommandFactorySidecarGitConfiguration)
+}
 
-	ctx := testhelper.Context(t)
+func testExecCommandFactorySidecarGitConfiguration(t *testing.T, ctx context.Context) {
 	cfg := testcfg.Build(t)
 
 	cfg.Git.Config = []config.GitConfig{
 		{Key: "custom.key", Value: "injected"},
 	}
 
+	// Feature flag to change the default global configuration of autocrlf.
+	autocrlf := "input"
+	if featureflag.AutocrlfConfig.IsEnabled(ctx) {
+		autocrlf = "false"
+	}
+
 	commonHead := []git.ConfigPair{
 		{Key: "gc.auto", Value: "0"},
-		{Key: "core.autocrlf", Value: "input"},
+		{Key: "core.autocrlf", Value: autocrlf},
 		{Key: "core.useReplaceRefs", Value: "false"},
 		{Key: "commitGraph.generationVersion", Value: "1"},
 	}
diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go
index 6bc6056b1..f33a4cf22 100644
--- a/internal/git/localrepo/objects_test.go
+++ b/internal/git/localrepo/objects_test.go
@@ -56,9 +56,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/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index 1c23a7646..c275a8764 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -265,7 +265,7 @@ func TestUserCommitFiles(t *testing.T) {
 			},
 		},
 		{
-			desc: "create file normalizes line endings",
+			desc: "create file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -276,7 +276,7 @@ func TestUserCommitFiles(t *testing.T) {
 					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"},
 					},
 				},
 			},
@@ -330,7 +330,7 @@ func TestUserCommitFiles(t *testing.T) {
 			},
 		},
 		{
-			desc: "update file normalizes line endings",
+			desc: "update file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -342,7 +342,7 @@ func TestUserCommitFiles(t *testing.T) {
 					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"},
 					},
 				},
 			},
@@ -601,7 +601,7 @@ func TestUserCommitFiles(t *testing.T) {
 			},
 		},
 		{
-			desc: "move file normalizes line endings",
+			desc: "move file does not normalize line endings",
 			steps: []step{
 				{
 					actions: []*gitalypb.UserCommitFilesRequest{
@@ -620,7 +620,7 @@ func TestUserCommitFiles(t *testing.T) {
 						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/metadata/featureflag/ff_autocrlf_config.go b/internal/metadata/featureflag/ff_autocrlf_config.go
new file mode 100644
index 000000000..dfdb67368
--- /dev/null
+++ b/internal/metadata/featureflag/ff_autocrlf_config.go
@@ -0,0 +1,8 @@
+package featureflag
+
+var AutocrlfConfig = NewFeatureFlag(
+	"autocrlf_false",
+	"v15.3.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4425",
+	false,
+)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index f1db352f5..081b4e90a 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -179,6 +179,8 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// PraefectGeneratedReplicaPaths affects many tests as it changes the repository creation logic.
 	// Randomly enable the flag to exercise both paths to some extent.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.PraefectGeneratedReplicaPaths, rnd.Int()%2 == 0)
+	// AutocrlfConfig affects the global options configuration.
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.AutocrlfConfig, true)
 
 	for _, opt := range opts {
 		ctx = opt(ctx)