linguist: Move some variables to linguist Instance

To make it easier to split out code in smaller functions, move some
variables we need to the linguist.Instance struct. This will allow
future refactoring.

3 files changed, 38 insertions, 35 deletions

diff --git a/internal/gitaly/linguist/linguist.go b/internal/gitaly/linguist/linguist.go
index 28a1c8826..5849b0ba5 100644
--- a/internal/gitaly/linguist/linguist.go
+++ b/internal/gitaly/linguist/linguist.go
@@ -26,28 +26,28 @@ type ByteCountPerLanguage map[string]uint64
 
 // Instance is a holder of the defined in the system language settings.
 type Instance struct {
-	cfg config.Cfg
+	cfg          config.Cfg
+	catfileCache catfile.Cache
+	repo         *localrepo.Repo
 }
 
-// New creates a new instance that can be used to calculate language stats.
-func New(cfg config.Cfg) *Instance {
+// New creates a new instance that can be used to calculate language stats for
+// the given repo.
+func New(cfg config.Cfg, catfileCache catfile.Cache, repo *localrepo.Repo) *Instance {
 	return &Instance{
-		cfg: cfg,
+		cfg:          cfg,
+		catfileCache: catfileCache,
+		repo:         repo,
 	}
 }
 
 // Stats returns the repository's language stats as reported by 'git-linguist'.
-func (inst *Instance) Stats(ctx context.Context, repo *localrepo.Repo, commitID string, catfileCache catfile.Cache) (ByteCountPerLanguage, error) {
+func (inst *Instance) Stats(ctx context.Context, commitID string) (ByteCountPerLanguage, error) {
 	if featureflag.GoLanguageStats.IsEnabled(ctx) {
-		return inst.enryStats(ctx, repo, commitID, catfileCache)
+		return inst.enryStats(ctx, commitID)
 	}
 
-	repoPath, err := repo.Path()
-	if err != nil {
-		return nil, fmt.Errorf("get repo path: %w", err)
-	}
-
-	cmd, err := inst.startGitLinguist(ctx, repoPath, commitID)
+	cmd, err := inst.startGitLinguist(ctx, commitID)
 	if err != nil {
 		return nil, fmt.Errorf("starting linguist: %w", err)
 	}
@@ -79,7 +79,12 @@ func Color(language string) string {
 	return fmt.Sprintf("#%x", colorSha[0:3])
 }
 
-func (inst *Instance) startGitLinguist(ctx context.Context, repoPath string, commitID string) (*command.Command, error) {
+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)
@@ -99,8 +104,8 @@ func (inst *Instance) startGitLinguist(ctx context.Context, repoPath string, com
 	return internalCmd, nil
 }
 
-func (inst *Instance) enryStats(ctx context.Context, repo *localrepo.Repo, commitID string, catfileCache catfile.Cache) (ByteCountPerLanguage, error) {
-	stats, err := newLanguageStats(repo)
+func (inst *Instance) enryStats(ctx context.Context, commitID string) (ByteCountPerLanguage, error) {
+	stats, err := newLanguageStats(inst.repo)
 	if err != nil {
 		ctxlogrus.Extract(ctx).WithError(err).Info("linguist load from cache")
 	}
@@ -108,7 +113,7 @@ func (inst *Instance) enryStats(ctx context.Context, repo *localrepo.Repo, commi
 		return stats.Totals, nil
 	}
 
-	objectReader, cancel, err := catfileCache.ObjectReader(ctx, repo)
+	objectReader, cancel, err := inst.catfileCache.ObjectReader(ctx, inst.repo)
 	if err != nil {
 		return nil, fmt.Errorf("create object reader: %w", err)
 	}
@@ -119,7 +124,7 @@ func (inst *Instance) enryStats(ctx context.Context, repo *localrepo.Repo, commi
 	if stats.CommitID == "" {
 		// No existing stats cached, so get all the files for the commit
 		// using git-ls-tree(1).
-		revlistIt = gitpipe.LsTree(ctx, repo,
+		revlistIt = gitpipe.LsTree(ctx, inst.repo,
 			commitID,
 			gitpipe.LsTreeWithRecursive(),
 			gitpipe.LsTreeWithBlobFilter(),
@@ -140,7 +145,7 @@ func (inst *Instance) enryStats(ctx context.Context, repo *localrepo.Repo, commi
 			return false
 		}
 
-		revlistIt = gitpipe.DiffTree(ctx, repo,
+		revlistIt = gitpipe.DiffTree(ctx, inst.repo,
 			stats.CommitID, commitID,
 			gitpipe.DiffTreeWithRecursive(),
 			gitpipe.DiffTreeWithIgnoreSubmodules(),
@@ -187,7 +192,7 @@ func (inst *Instance) enryStats(ctx context.Context, repo *localrepo.Repo, commi
 		return nil, fmt.Errorf("linguist object iterator: %w", err)
 	}
 
-	if err := stats.save(repo, commitID); err != nil {
+	if err := stats.save(inst.repo, commitID); err != nil {
 		return nil, fmt.Errorf("linguist language stats save: %w", err)
 	}
 
diff --git a/internal/gitaly/linguist/linguist_test.go b/internal/gitaly/linguist/linguist_test.go
index a3b98859a..79f0c3edb 100644
--- a/internal/gitaly/linguist/linguist_test.go
+++ b/internal/gitaly/linguist/linguist_test.go
@@ -34,9 +34,6 @@ func TestInstance_Stats(t *testing.T) {
 func testInstanceStats(t *testing.T, ctx context.Context) {
 	cfg := testcfg.Build(t)
 
-	linguist, err := New(cfg)
-	require.NoError(t, err)
-
 	catfileCache := catfile.NewCache(cfg)
 	t.Cleanup(catfileCache.Stop)
 
@@ -106,7 +103,7 @@ func testInstanceStats(t *testing.T, ctx context.Context) {
 
 				// We simply run the linguist once before so that it can already
 				// write the cache.
-				_, err := linguist.Stats(ctx, repo, commitID.String(), catfileCache)
+				_, err := New(cfg, catfileCache, repo).Stats(ctx, commitID.String())
 				require.NoError(t, err)
 				require.FileExists(t, filepath.Join(repoPath, languageStatsFilename))
 
@@ -164,7 +161,7 @@ func testInstanceStats(t *testing.T, ctx context.Context) {
 
 				// Precreate the cache with the old commit. This ensures that
 				// linguist knows to update the cache.
-				stats, err := linguist.Stats(ctx, repo, oldCommitID.String(), catfileCache)
+				stats, err := New(cfg, catfileCache, repo).Stats(ctx, oldCommitID.String())
 				require.NoError(t, err)
 				require.FileExists(t, filepath.Join(repoPath, languageStatsFilename))
 				require.Equal(t, ByteCountPerLanguage{
@@ -203,7 +200,8 @@ func testInstanceStats(t *testing.T, ctx context.Context) {
 			repoProto, repoPath, objectID := tc.setup(t)
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-			stats, err := linguist.Stats(ctx, repo, objectID.String(), catfileCache)
+			linguist := New(cfg, catfileCache, repo)
+			stats, err := linguist.Stats(ctx, objectID.String())
 			if tc.expectedErr == "" {
 				require.NoError(t, err)
 				require.Equal(t, tc.expectedStats, stats)
@@ -226,12 +224,11 @@ func TestInstance_Stats_unmarshalJSONError(t *testing.T) {
 
 	repo := localrepo.New(config.NewLocator(cfg), gitCmdFactory, catfileCache, invalidRepo)
 
-	ling, err := New(cfg)
-	require.NoError(t, err)
+	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, repo, "deadbeef", catfileCache)
+	_, err := ling.Stats(ctx, "deadbeef")
 	require.Error(t, err)
 
 	_, ok := err.(*json.SyntaxError)
@@ -277,9 +274,6 @@ func benchmarkInstanceStats(b *testing.B, ctx context.Context) {
 	cfg := testcfg.Build(b)
 	languageStatsFilename := filenameForCache(ctx)
 
-	linguist, err := New(cfg)
-	require.NoError(b, err)
-
 	catfileCache := catfile.NewCache(cfg)
 	b.Cleanup(catfileCache.Stop)
 
@@ -289,6 +283,8 @@ func benchmarkInstanceStats(b *testing.B, ctx context.Context) {
 	})
 	repo := localrepo.NewTestRepo(b, cfg, repoProto)
 
+	linguist := New(cfg, catfileCache, repo)
+
 	var scratchStat ByteCountPerLanguage
 	var incStats ByteCountPerLanguage
 
@@ -298,7 +294,8 @@ func benchmarkInstanceStats(b *testing.B, ctx context.Context) {
 			require.NoError(b, os.RemoveAll(filepath.Join(repoPath, languageStatsFilename)))
 			b.StartTimer()
 
-			scratchStat, err = linguist.Stats(ctx, repo, "f5dfdd0057cd6bffc6259a5c8533dde5bf6a9d37", catfileCache)
+			var err error
+			scratchStat, err = linguist.Stats(ctx, "f5dfdd0057cd6bffc6259a5c8533dde5bf6a9d37")
 			require.NoError(b, err)
 		}
 	})
@@ -308,10 +305,11 @@ func benchmarkInstanceStats(b *testing.B, ctx context.Context) {
 			b.StopTimer()
 			require.NoError(b, os.RemoveAll(filepath.Join(repoPath, languageStatsFilename)))
 			// a commit about 3 months older than the next
-			_, err = linguist.Stats(ctx, repo, "3c813b292d25a9b2ffda70e7f609f623bfc0cb37", catfileCache)
+			_, err := linguist.Stats(ctx, "3c813b292d25a9b2ffda70e7f609f623bfc0cb37")
+			require.NoError(b, err)
 			b.StartTimer()
 
-			incStats, err = linguist.Stats(ctx, repo, "f5dfdd0057cd6bffc6259a5c8533dde5bf6a9d37", catfileCache)
+			incStats, err = linguist.Stats(ctx, "f5dfdd0057cd6bffc6259a5c8533dde5bf6a9d37")
 			require.NoError(b, err)
 		}
 	})
diff --git a/internal/gitaly/service/commit/languages.go b/internal/gitaly/service/commit/languages.go
index 8eba5a3f8..be2116cc9 100644
--- a/internal/gitaly/service/commit/languages.go
+++ b/internal/gitaly/service/commit/languages.go
@@ -39,7 +39,7 @@ func (s *server) CommitLanguages(ctx context.Context, req *gitalypb.CommitLangua
 		return nil, helper.ErrInternalf("looking up revision: %w", err)
 	}
 
-	stats, err := linguist.New(s.cfg).Stats(ctx, repo, commitID, s.catfileCache)
+	stats, err := linguist.New(s.cfg, s.catfileCache, repo).Stats(ctx, commitID)
 	if err != nil {
 		return nil, helper.ErrInternalf("language stats: %w", err)
 	}