diff options
author | John Cai <jcai@gitlab.com> | 2022-08-16 22:18:40 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-08-16 22:18:40 +0300 |
commit | 4807366d4977d267f86df45fdb1214110c7f927c (patch) | |
tree | bf912d758b5b8a4bf039dfc360a2cfa797b89667 /internal | |
parent | cb95057b1a1a5aa979fdacb01f82f615fa600ef3 (diff) | |
parent | d0e60a800ddadd6531a5c4030d86b09564004a21 (diff) |
Merge branch 'jt-update-autocrlf-config' into 'master'
config: Set autocrlf to false
Closes #3476
See merge request gitlab-org/gitaly!4801
Diffstat (limited to 'internal')
-rw-r--r-- | internal/git/command_factory.go | 17 | ||||
-rw-r--r-- | internal/git/command_factory_test.go | 25 | ||||
-rw-r--r-- | internal/git/localrepo/objects_test.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/operations/commit_files_test.go | 12 | ||||
-rw-r--r-- | internal/metadata/featureflag/ff_autocrlf_config.go | 8 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 2 |
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) |