diff options
author | Justin Tobler <jtobler@gitlab.com> | 2022-08-18 23:13:56 +0300 |
---|---|---|
committer | Justin Tobler <jtobler@gitlab.com> | 2022-08-18 23:13:56 +0300 |
commit | 7bb407a26bb72e8d9d6e8601ed12192c69f7d829 (patch) | |
tree | d2e5dc2667506bb21461678e153a87aff3950562 | |
parent | b0bf444e393a97eff7bf35a2cd8dd43543181132 (diff) |
Revert "config: Set autocrlf to false"
This reverts commit d0e60a800ddadd6531a5c4030d86b09564004a21. Due to an
issue with some tests in the main GitLab project this commit needs to be
reverted to unblock the update flow and allow time for further
investigation.
-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, 17 insertions, 51 deletions
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go index ca343652c..77ff1976c 100644 --- a/internal/git/command_factory.go +++ b/internal/git/command_factory.go @@ -17,7 +17,6 @@ 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. @@ -498,12 +497,6 @@ 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 @@ -521,11 +514,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. With feature flag "autocrlf_false" enabled - // CRLF line endings will not get replaced and be left alone. - ConfigPair{Key: "core.autocrlf", Value: autocrlf}, + // 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"}, // 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 d4847028a..f20863535 100644 --- a/internal/git/command_factory_test.go +++ b/internal/git/command_factory_test.go @@ -4,7 +4,6 @@ package git_test import ( "bytes" - "context" "errors" "fmt" "io" @@ -473,10 +472,8 @@ 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) { + ctx := testhelper.Context(t) cfg := testcfg.Build(t) // Create a repository and remove its gitconfig to bring us into a known state where there @@ -486,15 +483,9 @@ func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) { }) 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=" + autocrlf, + "core.autocrlf=input", "core.usereplacerefs=false", "commitgraph.generationversion=1", } @@ -545,25 +536,17 @@ func testExecCommandFactoryConfig(t *testing.T, ctx context.Context) { func TestExecCommandFactory_SidecarGitConfiguration(t *testing.T) { t.Parallel() - testhelper.NewFeatureSets(featureflag.AutocrlfConfig).Run(t, testExecCommandFactorySidecarGitConfiguration) -} -func testExecCommandFactorySidecarGitConfiguration(t *testing.T, ctx context.Context) { + ctx := testhelper.Context(t) 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: autocrlf}, + {Key: "core.autocrlf", Value: "input"}, {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 f33a4cf22..6bc6056b1 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 not converted to LF due to global git config", + desc: "CRLF converted to LF due to global git config", input: strings.NewReader("\r\n"), - content: "\r\n", + content: "\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 c275a8764..1c23a7646 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 does not normalize line endings", + desc: "create file normalizes 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\r\n content-2\r\n"}, + {Mode: DefaultMode, Path: "file-1", Content: "content-1\n content-2\n"}, }, }, }, @@ -330,7 +330,7 @@ func TestUserCommitFiles(t *testing.T) { }, }, { - desc: "update file does not normalize line endings", + desc: "update file normalizes 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\r\n"}, + {Mode: DefaultMode, Path: "file-1", Content: "content-2\n"}, }, }, }, @@ -601,7 +601,7 @@ func TestUserCommitFiles(t *testing.T) { }, }, { - desc: "move file does not normalize line endings", + desc: "move file normalizes 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\r\n"}, + {Mode: DefaultMode, Path: "moved-file", Content: "new-content\n"}, }, }, }, diff --git a/internal/metadata/featureflag/ff_autocrlf_config.go b/internal/metadata/featureflag/ff_autocrlf_config.go deleted file mode 100644 index dfdb67368..000000000 --- a/internal/metadata/featureflag/ff_autocrlf_config.go +++ /dev/null @@ -1,8 +0,0 @@ -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 081b4e90a..f1db352f5 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -179,8 +179,6 @@ 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) |