From 2853e246f0451e8c07e0f74491351f474cd84a2d Mon Sep 17 00:00:00 2001 From: John Cai Date: Mon, 9 Jun 2025 13:14:25 -0400 Subject: [PATCH] gitcmd: Pass in configs to enable custom diff algorithms Pass in a set of configs that is of the form [diff ""] algorithm = This allows adding the following to .gitattributes to set diff algorithms based on a path: *.json diff= meyers, patience, histogram, minimal can be used in --- internal/featureflag/ff_diff_algorithm.go | 9 ++ internal/git/gitcmd/command_description.go | 14 ++ internal/git/gitcmd/diff_alg_test.go | 176 +++++++++++++++++++++ internal/testhelper/testhelper.go | 2 + 4 files changed, 201 insertions(+) create mode 100644 internal/featureflag/ff_diff_algorithm.go create mode 100644 internal/git/gitcmd/diff_alg_test.go diff --git a/internal/featureflag/ff_diff_algorithm.go b/internal/featureflag/ff_diff_algorithm.go new file mode 100644 index 0000000000..0d42125461 --- /dev/null +++ b/internal/featureflag/ff_diff_algorithm.go @@ -0,0 +1,9 @@ +package featureflag + +// DiffAlgorithm enables users to set a diff algorithm per file path +var DiffAlgorithm = NewFeatureFlag( + "diff_algorithm", + "v18.2.0", + "https://gitlab.com/gitlab-org/gitaly/-/issues/6791", + false, +) diff --git a/internal/git/gitcmd/command_description.go b/internal/git/gitcmd/command_description.go index 0446d2ecbd..5dc72afc69 100644 --- a/internal/git/gitcmd/command_description.go +++ b/internal/git/gitcmd/command_description.go @@ -7,6 +7,7 @@ import ( "runtime" "strings" + "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" ) @@ -87,6 +88,19 @@ var commandDescriptions = map[string]commandDescription{ }, "diff": { flags: scNoRefUpdates, + opts: func(ctx context.Context) []GlobalOption { + options := []GlobalOption{} + if featureflag.DiffAlgorithm.IsEnabled(ctx) { + options = append(options, + ConfigPair{Key: "diff.histogram.algorithm", Value: "histogram"}, + ConfigPair{Key: "diff.myers.algorithm", Value: "myers"}, + ConfigPair{Key: "diff.patience.algorithm", Value: "patience"}, + ConfigPair{Key: "diff.minimal.algorithm", Value: "minimal"}, + ) + } + + return options + }, }, "diff-pairs": { flags: scNoRefUpdates, diff --git a/internal/git/gitcmd/diff_alg_test.go b/internal/git/gitcmd/diff_alg_test.go new file mode 100644 index 0000000000..2abd0bf4d3 --- /dev/null +++ b/internal/git/gitcmd/diff_alg_test.go @@ -0,0 +1,176 @@ +package gitcmd_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/diff" + "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" + "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg" +) + +func TestCustomDiffAlgorithms(t *testing.T) { + ctx := featureflag.ContextWithFeatureFlag(testhelper.Context(t), featureflag.DiffAlgorithm, true) + cfg := testcfg.Build(t) + + gitCmdFactory, cleanup, err := gitcmd.NewExecCommandFactory(cfg, testhelper.SharedLogger(t)) + require.NoError(t, err) + defer cleanup() + + testCases := []struct { + algorithm string + expectedResult string + }{ + { + algorithm: "myers", + expectedResult: `@@ -1,9 +1,9 @@ +-unique line 1 ++unique line 3 + common + common + common +-unique line 2 ++unique line 1 + common + common + common +-unique line 3 ++unique line 2 +`, + }, + { + algorithm: "minimal", + expectedResult: `@@ -1,9 +1,9 @@ +-unique line 1 ++unique line 3 + common + common + common +-unique line 2 ++unique line 1 + common + common + common +-unique line 3 ++unique line 2 +`, + }, + { + algorithm: "histogram", + expectedResult: `@@ -1,9 +1,9 @@ ++unique line 3 ++common ++common ++common + unique line 1 + common + common + common + unique line 2 +-common +-common +-common +-unique line 3 +`, + }, + { + algorithm: "patience", + expectedResult: `@@ -1,9 +1,9 @@ ++unique line 3 ++common ++common ++common + unique line 1 + common + common + common + unique line 2 +-common +-common +-common +-unique line 3 +`, + }, + { + algorithm: "unknown", // unknown should default to myers + expectedResult: `@@ -1,9 +1,9 @@ +-unique line 1 ++unique line 3 + common + common + common +-unique line 2 ++unique line 1 + common + common + common +-unique line 3 ++unique line 2 +`, + }, + } + + for _, tc := range testCases { + t.Run(tc.algorithm, func(t *testing.T) { + repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + SkipCreationViaService: true, + }) + + before := gittest.TreeEntry{Path: "file.txt", Mode: "100644", Content: `unique line 1 +common +common +common +unique line 2 +common +common +common +unique line 3 +`} + after := gittest.TreeEntry{Path: "file.txt", Mode: "100644", Content: `unique line 3 +common +common +common +unique line 1 +common +common +common +unique line 2 +`} + tree := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{{Path: ".gitattributes", Mode: "100644", Content: fmt.Sprintf("*.txt diff=%s", tc.algorithm)}}) + gitAttributes := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTree(tree), gittest.WithBranch("HEAD")) + + commitB := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(before), gittest.WithParents(gitAttributes)) + commitC := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(after), gittest.WithParents(commitB)) + + cmd, err := gitCmdFactory.New(ctx, repo, gitcmd.Command{ + Name: "diff", + Flags: []gitcmd.Option{ + gitcmd.Flag{Name: "--raw"}, + gitcmd.Flag{Name: "--patch"}, + gitcmd.Flag{Name: fmt.Sprintf("--abbrev=%d", gittest.DefaultObjectHash.EncodedLen())}, + }, + Args: []string{commitB.String(), commitC.String()}, + PostSepArgs: []string{"file.txt"}, + }, + gitcmd.WithConfig(gitcmd.ConfigPair{Key: "attr.tree", Value: tree.String()}), + gitcmd.WithSetupStdout()) + require.NoError(t, err) + + diffParser := diff.NewDiffParser(gittest.DefaultObjectHash, cmd, diff.Limits{EnforceLimits: false}) + + for !diffParser.Parse() { + require.Nil(t, diffParser.Err()) + } + + require.NoError(t, cmd.Wait()) + + result := diffParser.Diff() + + require.Equal(t, []byte(tc.expectedResult), result.Patch) + }) + } +} diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 454a9ae389..953213cd93 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -311,6 +311,8 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context { // deep in the call stack, so almost every test function would have to inject it into its // context. The values of these flags should be randomized to increase the test coverage. + // Randomly enable diff algorithm + ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.DiffAlgorithm, rnd.Int()%2 == 0) // Randomly enable mailmap ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.MailmapOptions, rnd.Int()%2 == 0) // Randomly enable multi-pack reuse of objects. -- GitLab