From 26f0dde7e0d8c341a39fb616fdd36355b9e2d75e Mon Sep 17 00:00:00 2001
From: Eric Ju <eju@gitlab.com>
Date: Tue, 23 Jan 2024 15:01:57 -0400
Subject: repository: Make GetInfoAttributes read from HEAD:.gitattribute

This is a solution for N+1 problem we come across, see
https://gitlab.com/groups/gitlab-org/-/epics/9006#note_1730745750 for
details.

In order to remove the reliance of info/attribute and avoid breaking
Rails with TooManyInvocationsError. GetInfoAttributes is set to read
from HEAD:.gitattribute.

It will also try to delete info/attribute before reading.

After proper refactor on Rails to solve the N+1 case.
GetInfoAttributes can be removed totally.
---
 .../gitaly/service/repository/info_attributes.go   | 51 ++++++++++++++++++----
 .../service/repository/info_attributes_test.go     | 17 +++++++-
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/internal/gitaly/service/repository/info_attributes.go b/internal/gitaly/service/repository/info_attributes.go
index 9ddf04fdc..9f8f54a24 100644
--- a/internal/gitaly/service/repository/info_attributes.go
+++ b/internal/gitaly/service/repository/info_attributes.go
@@ -1,16 +1,19 @@
 package repository
 
 import (
+	"bufio"
+	"errors"
 	"io"
 	"os"
-	"path/filepath"
+	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v16/streamio"
 )
 
-func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) error {
+func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) (returnedErr error) {
 	repository := in.GetRepository()
 	if err := s.locator.ValidateRepository(repository); err != nil {
 		return structerr.NewInvalidArgument("%w", err)
@@ -20,14 +23,46 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream
 		return err
 	}
 
-	attrFile := filepath.Join(repoPath, "info", "attributes")
-	f, err := os.Open(attrFile)
+	// In git 2.43.0+, gitattributes supports reading from HEAD:.gitattributes,
+	// so info/attributes is no longer needed. To make sure info/attributes file is cleaned up,
+	// we delete it if it exists when reading from HEAD:.gitattributes is called.
+	// This logic can be removed when ApplyGitattributes and GetInfoAttributes PRC are totally removed from
+	// the code base.
+	deletionErr := deleteInfoAttributesFile(repoPath)
+	if !os.IsNotExist(deletionErr) {
+		s.logger.WithError(deletionErr).Error("failed to delete info/gitattributes file at " + repoPath)
+	}
+
+	repo := s.localrepo(in.GetRepository())
+	ctx := stream.Context()
+	var stderr strings.Builder
+	// Call cat-file -p HEAD:.gitattributes instead of cat info/attributes
+	catFileCmd, err := repo.Exec(ctx, git.Command{
+		Name: "cat-file",
+		Flags: []git.Option{
+			git.Flag{Name: "-p"},
+		},
+		Args: []string{"HEAD:.gitattributes"},
+	},
+		git.WithSetupStdout(),
+		git.WithStderr(&stderr),
+	)
 	if err != nil {
-		if os.IsNotExist(err) {
-			return stream.Send(&gitalypb.GetInfoAttributesResponse{})
+		return structerr.NewInternal("failure to read HEAD:.gitattributes: %w", err)
+	}
+	defer func() {
+		if err := catFileCmd.Wait(); err != nil {
+			s.logger.Error("git cat-file command error: " + stderr.String())
+			if returnedErr != nil {
+				returnedErr = structerr.NewInternal("failure to read HEAD:.gitattributes: %w", err)
+			}
 		}
+	}()
 
-		return structerr.NewInternal("failure to read info attributes: %w", err)
+	buf := bufio.NewReader(catFileCmd)
+	_, err = buf.Peek(1)
+	if errors.Is(err, io.EOF) {
+		return stream.Send(&gitalypb.GetInfoAttributesResponse{})
 	}
 
 	sw := streamio.NewWriter(func(p []byte) error {
@@ -35,7 +70,7 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream
 			Attributes: p,
 		})
 	})
+	_, err = io.Copy(sw, buf)
 
-	_, err = io.Copy(sw, f)
 	return err
 }
diff --git a/internal/gitaly/service/repository/info_attributes_test.go b/internal/gitaly/service/repository/info_attributes_test.go
index 908761822..8106fba70 100644
--- a/internal/gitaly/service/repository/info_attributes_test.go
+++ b/internal/gitaly/service/repository/info_attributes_test.go
@@ -33,6 +33,13 @@ func TestGetInfoAttributesExisting(t *testing.T) {
 	err := os.WriteFile(attrsPath, data, perm.SharedFile)
 	require.NoError(t, err)
 
+	gitattributesContent := "*.go diff=go text\n*.md text\n*.jpg -text"
+	gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithBranch("main"),
+		gittest.WithTreeEntries(
+			gittest.TreeEntry{Path: ".gitattributes", Mode: "100644", Content: gitattributesContent},
+		))
+
 	request := &gitalypb.GetInfoAttributesRequest{Repository: repo}
 
 	//nolint:staticcheck
@@ -45,7 +52,15 @@ func TestGetInfoAttributesExisting(t *testing.T) {
 	}))
 
 	require.NoError(t, err)
-	require.Equal(t, data, receivedData)
+	require.Equal(t, gitattributesContent, string(receivedData))
+
+	if !testhelper.IsWALEnabled() {
+		// Supporting info/attributes file is deprecating,
+		// so we don't need to support committing them through the WAL.
+		// Skip asserting the info/attributes file is removed.
+		// And this test should be removed, once all info/attributes files clean up.
+		require.NoFileExists(t, attrsPath)
+	}
 }
 
 func TestGetInfoAttributesNonExisting(t *testing.T) {
-- 
cgit v1.2.3