Merge branch 'toon-linguist-cleanup' into 'master'

linguist: Remove Ruby implementation

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5016

Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>

5 files changed, 48 insertions, 178 deletions

diff --git a/internal/gitaly/linguist/language_stats_test.go b/internal/gitaly/linguist/language_stats_test.go
index d03e71cf1..74f66a568 100644
--- a/internal/gitaly/linguist/language_stats_test.go
+++ b/internal/gitaly/linguist/language_stats_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package linguist
 
 import (
diff --git a/internal/gitaly/linguist/linguist.go b/internal/gitaly/linguist/linguist.go
index 30ba2d612..0bd9bfd46 100644
--- a/internal/gitaly/linguist/linguist.go
+++ b/internal/gitaly/linguist/linguist.go
@@ -3,11 +3,8 @@ package linguist
 import (
 	"context"
 	"crypto/sha256"
-	"encoding/json"
 	"fmt"
 	"io"
-	"os"
-	"os/exec"
 
 	"github.com/go-enry/go-enry/v2"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
@@ -18,8 +15,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 )
 
 // ByteCountPerLanguage represents a counter value (bytes) per language.
@@ -42,34 +37,6 @@ func New(cfg config.Cfg, catfileCache catfile.Cache, repo *localrepo.Repo) *Inst
 	}
 }
 
-// Stats returns the repository's language stats as reported by 'git-linguist'.
-func (inst *Instance) Stats(ctx context.Context, commitID string) (ByteCountPerLanguage, error) {
-	if featureflag.GoLanguageStats.IsEnabled(ctx) {
-		return inst.enryStats(ctx, commitID)
-	}
-
-	cmd, err := inst.startGitLinguist(ctx, commitID)
-	if err != nil {
-		return nil, fmt.Errorf("starting linguist: %w", err)
-	}
-
-	data, err := io.ReadAll(cmd)
-	if err != nil {
-		return nil, fmt.Errorf("reading linguist output: %w", err)
-	}
-
-	if err := cmd.Wait(); err != nil {
-		return nil, fmt.Errorf("waiting for linguist: %w", err)
-	}
-
-	stats := make(ByteCountPerLanguage)
-	if err := json.Unmarshal(data, &stats); err != nil {
-		return nil, fmt.Errorf("unmarshaling stats: %w", err)
-	}
-
-	return stats, nil
-}
-
 // Color returns the color Linguist has assigned to language.
 func Color(language string) string {
 	if color := enry.GetColor(language); color != "#cccccc" {
@@ -80,32 +47,8 @@ func Color(language string) string {
 	return fmt.Sprintf("#%x", colorSha[0:3])
 }
 
-func (inst *Instance) startGitLinguist(ctx context.Context, commitID string) (*command.Command, error) {
-	repoPath, err := inst.repo.Path()
-	if err != nil {
-		return nil, fmt.Errorf("get repo path: %w", err)
-	}
-
-	bundle, err := exec.LookPath("bundle")
-	if err != nil {
-		return nil, fmt.Errorf("finding bundle executable: %w", err)
-	}
-
-	cmd := []string{bundle, "exec", "bin/gitaly-linguist", "--repository=" + repoPath, "--commit=" + commitID}
-
-	internalCmd, err := command.New(ctx, cmd,
-		command.WithDir(inst.cfg.Ruby.Dir),
-		command.WithEnvironment(env.AllowedRubyEnvironment(os.Environ())),
-		command.WithCommandName("git-linguist", "stats"),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("creating command: %w", err)
-	}
-
-	return internalCmd, nil
-}
-
-func (inst *Instance) enryStats(ctx context.Context, commitID string) (ByteCountPerLanguage, error) {
+// Stats returns the repository's language statistics.
+func (inst *Instance) Stats(ctx context.Context, commitID string) (ByteCountPerLanguage, error) {
 	stats, err := initLanguageStats(inst.repo)
 	if err != nil {
 		ctxlogrus.Extract(ctx).WithError(err).Info("linguist load from cache")
@@ -158,12 +101,17 @@ func (inst *Instance) enryStats(ctx context.Context, commitID string) (ByteCount
 		// Stats are cached for one commit, so get the git-diff-tree(1)
 		// between that commit and the one we're calculating stats for.
 
+		hash, err := git.DetectObjectHash(ctx, inst.repo)
+		if err != nil {
+			return nil, fmt.Errorf("linguist: detect object hash: %w", err)
+		}
+
 		skipFunc := func(result *gitpipe.RevisionResult) (bool, error) {
 			var skip bool
 
 			// Skip files that are deleted, or
 			// an excluded filetype based on filename.
-			if git.ObjectHashSHA1.IsZeroOID(result.OID) {
+			if hash.IsZeroOID(result.OID) {
 				skip = true
 			} else {
 				f, err := newFileInstance(string(result.ObjectName), checkAttr)
diff --git a/internal/gitaly/linguist/linguist_test.go b/internal/gitaly/linguist/linguist_test.go
index 5498126f4..3a67ce07e 100644
--- a/internal/gitaly/linguist/linguist_test.go
+++ b/internal/gitaly/linguist/linguist_test.go
@@ -1,10 +1,6 @@
-//go:build !gitaly_test_sha256
-
 package linguist
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -17,7 +13,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -28,18 +23,14 @@ func TestMain(m *testing.M) {
 }
 
 func TestInstance_Stats(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testInstanceStats)
-}
+	t.Parallel()
 
-func testInstanceStats(t *testing.T, ctx context.Context) {
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
 	catfileCache := catfile.NewCache(cfg)
 	t.Cleanup(catfileCache.Stop)
 
-	languageStatsFilename := filenameForCache(ctx)
-
 	for _, tc := range []struct {
 		desc          string
 		setup         func(t *testing.T) (*gitalypb.Repository, string, git.ObjectID)
@@ -561,7 +552,7 @@ func TestInstance_Stats_failureGitattributes(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
-	ctx := featureflag.ContextWithFeatureFlag(testhelper.Context(t), featureflag.GoLanguageStats, true)
+	ctx := testhelper.Context(t)
 	locator := config.NewLocator(cfg)
 
 	catfileCache := catfile.NewCache(cfg)
@@ -594,28 +585,6 @@ func TestInstance_Stats_failureGitattributes(t *testing.T) {
 	require.ErrorContains(t, err, expectedErr)
 }
 
-func TestInstance_Stats_unmarshalJSONError(t *testing.T) {
-	cfg := testcfg.Build(t)
-	ctx := featureflag.ContextWithFeatureFlag(testhelper.Context(t), featureflag.GoLanguageStats, false)
-	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
-	invalidRepo := &gitalypb.Repository{StorageName: "fake", RelativePath: "path"}
-
-	catfileCache := catfile.NewCache(cfg)
-	t.Cleanup(catfileCache.Stop)
-
-	repo := localrepo.New(config.NewLocator(cfg), gitCmdFactory, catfileCache, invalidRepo)
-
-	ling := New(cfg, catfileCache, repo)
-
-	// When an error occurs, this used to trigger JSON marshaling of a plain string
-	// the new behaviour shouldn't do that, and return a command error
-	_, err := ling.Stats(ctx, "deadbeef")
-	require.Error(t, err)
-
-	_, ok := err.(*json.SyntaxError)
-	require.False(t, ok, "expected the error not be a json Syntax Error")
-}
-
 func TestColor(t *testing.T) {
 	t.Parallel()
 
@@ -637,23 +606,9 @@ func TestColor(t *testing.T) {
 	}
 }
 
-// filenameForCache returns the filename where the cache is stored, depending on
-// the feature flag.
-func filenameForCache(ctx context.Context) string {
-	if featureflag.GoLanguageStats.IsDisabled(ctx) {
-		return "language-stats.cache"
-	}
-	return languageStatsFilename
-}
-
 func BenchmarkInstance_Stats(b *testing.B) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Bench(b, benchmarkInstanceStats)
-}
-
-func benchmarkInstanceStats(b *testing.B, ctx context.Context) {
 	cfg := testcfg.Build(b)
-	languageStatsFilename := filenameForCache(ctx)
+	ctx := testhelper.Context(b)
 
 	catfileCache := catfile.NewCache(cfg)
 	b.Cleanup(catfileCache.Stop)
diff --git a/internal/gitaly/service/commit/languages_test.go b/internal/gitaly/service/commit/languages_test.go
index 996eb4a2d..2df41a0aa 100644
--- a/internal/gitaly/service/commit/languages_test.go
+++ b/internal/gitaly/service/commit/languages_test.go
@@ -3,12 +3,10 @@
 package commit
 
 import (
-	"context"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -17,13 +15,10 @@ import (
 )
 
 func TestLanguages(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testLanguagesFeatured)
-}
-
-func testLanguagesFeatured(t *testing.T, ctx context.Context) {
 	t.Parallel()
+
 	cfg := testcfg.Build(t)
+	ctx := testhelper.Context(t)
 
 	cfg.SocketPath = startTestServices(t, cfg)
 
@@ -53,13 +48,9 @@ func testLanguagesFeatured(t *testing.T, ctx context.Context) {
 }
 
 func TestFileCountIsZeroWhenFeatureIsDisabled(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testFileCountIsZeroWhenFeatureIsDisabled)
-}
-
-func testFileCountIsZeroWhenFeatureIsDisabled(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
 
 	request := &gitalypb.CommitLanguagesRequest{
@@ -79,13 +70,9 @@ func testFileCountIsZeroWhenFeatureIsDisabled(t *testing.T, ctx context.Context)
 }
 
 func TestLanguagesEmptyRevision(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testLanguagesEmptyRevisionFeatured)
-}
-
-func testLanguagesEmptyRevisionFeatured(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
 
 	request := &gitalypb.CommitLanguagesRequest{
@@ -106,13 +93,9 @@ func testLanguagesEmptyRevisionFeatured(t *testing.T, ctx context.Context) {
 }
 
 func TestInvalidCommitLanguagesRequestRevision(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testInvalidCommitLanguagesRequestRevisionFeatured)
-}
-
-func testInvalidCommitLanguagesRequestRevisionFeatured(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
 
 	_, err := client.CommitLanguages(ctx, &gitalypb.CommitLanguagesRequest{
@@ -123,13 +106,9 @@ func testInvalidCommitLanguagesRequestRevisionFeatured(t *testing.T, ctx context
 }
 
 func TestAmbiguousRefCommitLanguagesRequestRevision(t *testing.T) {
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).
-		Run(t, testAmbiguousRefCommitLanguagesRequestRevisionFeatured)
-}
-
-func testAmbiguousRefCommitLanguagesRequestRevisionFeatured(t *testing.T, ctx context.Context) {
 	t.Parallel()
 
+	ctx := testhelper.Context(t)
 	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
 
 	// gitlab-test repo has both a branch and a tag named 'v1.1.0'
@@ -144,35 +123,35 @@ func testAmbiguousRefCommitLanguagesRequestRevisionFeatured(t *testing.T, ctx co
 
 func TestCommitLanguages_validateRequest(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.GoLanguageStats).Run(t, func(t *testing.T, ctx context.Context) {
-		_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-		for _, tc := range []struct {
-			desc        string
-			req         *gitalypb.CommitLanguagesRequest
-			expectedErr error
-		}{
-			{
-				desc: "no repository provided",
-				req:  &gitalypb.CommitLanguagesRequest{Repository: nil},
-				expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage(
-					"empty Repository",
-					"repo scoped: empty Repository",
-				)),
-			},
-			{
-				desc: "invalid revision provided",
-				req: &gitalypb.CommitLanguagesRequest{
-					Repository: repo,
-					Revision:   []byte("invalid revision"),
-				},
-				expectedErr: status.Error(codes.InvalidArgument, "revision can't contain whitespace"),
+
+	ctx := testhelper.Context(t)
+	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
+
+	for _, tc := range []struct {
+		desc        string
+		req         *gitalypb.CommitLanguagesRequest
+		expectedErr error
+	}{
+		{
+			desc: "no repository provided",
+			req:  &gitalypb.CommitLanguagesRequest{Repository: nil},
+			expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage(
+				"empty Repository",
+				"repo scoped: empty Repository",
+			)),
+		},
+		{
+			desc: "invalid revision provided",
+			req: &gitalypb.CommitLanguagesRequest{
+				Repository: repo,
+				Revision:   []byte("invalid revision"),
 			},
-		} {
-			t.Run(tc.desc, func(t *testing.T) {
-				_, err := client.CommitLanguages(ctx, tc.req)
-				testhelper.RequireGrpcError(t, tc.expectedErr, err)
-			})
-		}
-	})
+			expectedErr: status.Error(codes.InvalidArgument, "revision can't contain whitespace"),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := client.CommitLanguages(ctx, tc.req)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+		})
+	}
 }
diff --git a/internal/metadata/featureflag/ff_go_language_stats.go b/internal/metadata/featureflag/ff_go_language_stats.go
deleted file mode 100644
index cf4626a06..000000000
--- a/internal/metadata/featureflag/ff_go_language_stats.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// GoLanguageStats flag enables getting CommitLanguages statistics written in
-// Go.
-var GoLanguageStats = NewFeatureFlag(
-	"go_language_stats",
-	"v15.2.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/4254",
-	false,
-)