Skip to content
Snippets Groups Projects
Commit 742b849e authored by Eric Ju's avatar Eric Ju
Browse files

repository: Make GetInfoAttributes read from HEAD:.gitattribute

This is a workaround solution for N+1 problem we come across, see
&9006 (comment 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 problem in this issue
gitlab#452425,
GetInfoAttributes can be removed totally.
parent e0dd698f
No related branches found
No related tags found
1 merge request!6573repository: stop using info/attributes in git 2.43.0
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 +22,45 @@ 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.
if deletionErr := deleteInfoAttributesFile(repoPath); deletionErr != nil {
return structerr.NewInternal("delete info/gitattributes file: %w", deletionErr).WithMetadata("path", 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("read HEAD:.gitattributes: %w", err)
}
defer func() {
if err := catFileCmd.Wait(); err != nil {
if returnedErr != nil {
returnedErr = structerr.NewInternal("read HEAD:.gitattributes: %w", err).
WithMetadata("stderr", stderr)
}
}
}()
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 +68,7 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream
Attributes: p,
})
})
_, err = io.Copy(sw, buf)
_, err = io.Copy(sw, f)
return err
}
......@@ -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) {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment