Skip to content
Snippets Groups Projects
Verified Commit c35ecf60 authored by Eric Ju's avatar Eric Ju Committed by GitLab
Browse files

Merge branch 'ej-stop-using-info-atrributes' into 'master' 

repository: stop using info/attributes in git 2.43.0

Closes #5348

See merge request !6573



Merged-by: default avatarEric Ju <eju@gitlab.com>
Approved-by: default avatarToon Claes <toon@gitlab.com>
Reviewed-by: default avatarToon Claes <toon@gitlab.com>
Reviewed-by: default avatarJustin Tobler <jtobler@gitlab.com>
Reviewed-by: default avatarEric Ju <eju@gitlab.com>
parents 3f07b901 742b849e
No related branches found
No related tags found
1 merge request!6573repository: stop using info/attributes in git 2.43.0
Pipeline #1240625863 failed
Stage: build
Stage: test
Stage: analyze
Stage: publish
Stage: qa
Stage: deploy
Hide whitespace changes
Inline Side-by-side
Showing
with 173 additions and 534 deletions
internal/gitaly/server/auth_test.go
+ 3
− 10
View file @ c35ecf60
......@@ -5,7 +5,6 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"io"
"net"
"testing"
"time"
......@@ -33,7 +32,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitaly/v16/streamio"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
......@@ -348,20 +346,15 @@ func TestStreamingNoAuth(t *testing.T) {
ctx := testhelper.Context(t)
client := gitalypb.NewRepositoryServiceClient(conn)
//nolint:staticcheck
stream, err := client.GetInfoAttributes(ctx, &gitalypb.GetInfoAttributesRequest{
response, err := client.GetFileAttributes(ctx, &gitalypb.GetFileAttributesRequest{
Repository: &gitalypb.Repository{
StorageName: cfg.Storages[0].Name,
RelativePath: "new/project/path",
},
},
)
require.NoError(t, err)
_, err = io.ReadAll(streamio.NewReader(func() ([]byte, error) {
_, err = stream.Recv()
return nil, err
}))
require.Nil(t, response)
testhelper.RequireGrpcCode(t, err, codes.Unauthenticated)
}
......
internal/gitaly/service/repository/apply_gitattributes.go
+ 19
− 137
View file @ c35ecf60
......@@ -2,155 +2,37 @@ package repository
import (
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
"gitlab.com/gitlab-org/gitaly/v16/internal/safe"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/txinfo"
"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
)
const attributesFileMode os.FileMode = perm.SharedFile
func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, objectReader catfile.ObjectContentReader, repoPath string, revision []byte) (returnedErr error) {
infoPath := filepath.Join(repoPath, "info")
attributesPath := filepath.Join(infoPath, "attributes")
_, err := repo.ResolveRevision(ctx, git.Revision(revision)+"^{commit}")
if err != nil {
if errors.Is(err, git.ErrReferenceNotFound) {
return structerr.NewInvalidArgument("revision does not exist")
}
return err
}
blobObj, err := objectReader.Object(ctx, git.Revision(fmt.Sprintf("%s:.gitattributes", revision)))
if err != nil && !errors.As(err, &catfile.NotFoundError{}) {
return err
}
// Create /info folder if it doesn't exist
if err := os.MkdirAll(infoPath, perm.SharedDir); err != nil {
return err
}
if errors.As(err, &catfile.NotFoundError{}) || blobObj.Type != "blob" {
locker, err := safe.NewLockingFileWriter(attributesPath, safe.LockingFileWriterConfig{
FileWriterConfig: safe.FileWriterConfig{FileMode: attributesFileMode},
})
if err != nil {
return fmt.Errorf("creating gitattributes lock: %w", err)
}
defer func() {
if err := locker.Close(); err != nil {
s.logger.WithError(err).ErrorContext(ctx, "unlocking gitattributes")
}
}()
if err := locker.Lock(); err != nil {
return fmt.Errorf("locking gitattributes: %w", err)
}
// We use the zero OID as placeholder to vote on removal of the
// gitattributes file.
if err := s.vote(ctx, git.ObjectHashSHA1.ZeroOID, voting.Prepared); err != nil {
return fmt.Errorf("preimage vote: %w", err)
}
if err := os.Remove(attributesPath); err != nil && !os.IsNotExist(err) {
return err
}
if err := s.vote(ctx, git.ObjectHashSHA1.ZeroOID, voting.Committed); err != nil {
return fmt.Errorf("postimage vote: %w", err)
}
return nil
}
writer, err := safe.NewLockingFileWriter(attributesPath, safe.LockingFileWriterConfig{
FileWriterConfig: safe.FileWriterConfig{FileMode: attributesFileMode},
})
if err != nil {
return fmt.Errorf("creating gitattributes writer: %w", err)
}
defer func() {
if err := writer.Close(); err != nil && returnedErr == nil {
if !errors.Is(err, safe.ErrAlreadyDone) {
returnedErr = err
}
}
}()
if _, err := io.Copy(writer, blobObj); err != nil {
return err
}
if err := transaction.CommitLockedFile(ctx, s.txManager, writer); err != nil {
return fmt.Errorf("committing gitattributes: %w", err)
}
return nil
}
func (s *server) vote(ctx context.Context, oid git.ObjectID, phase voting.Phase) error {
tx, err := txinfo.TransactionFromContext(ctx)
if errors.Is(err, txinfo.ErrTransactionNotFound) {
return nil
}
hash, err := oid.Bytes()
if err != nil {
return fmt.Errorf("vote with invalid object ID: %w", err)
}
vote, err := voting.VoteFromHash(hash)
if err != nil {
return fmt.Errorf("cannot convert OID to vote: %w", err)
}
if err := s.txManager.Vote(ctx, tx, vote, phase); err != nil {
return fmt.Errorf("vote failed: %w", err)
}
return nil
}
func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitattributesRequest) (*gitalypb.ApplyGitattributesResponse, error) {
// In git 2.43.0+, gitattributes supports reading from HEAD:.gitattributes,
// so info/attributes is no longer needed. Besides that, info/attributes file needs to
// be cleaned because it has a higher precedence
// than HEAD:.gitattributes. We want to avoid HEAD:.gitattributes being
// overridden.
//
// 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 RPC are totally removed from
// the code base.
repository := in.GetRepository()
if err := s.locator.ValidateRepository(repository); err != nil {
return nil, structerr.NewInvalidArgument("%w", err)
return nil, structerr.NewInvalidArgument("validate repo error: %w", err)
}
repo := s.localrepo(repository)
repoPath, err := s.locator.GetRepoPath(repo)
repoPath, err := s.locator.GetRepoPath(repository)
if err != nil {
return nil, err
return nil, structerr.NewInternal("find repo path: %w", err).WithMetadata("path", repoPath)
}
if err := git.ValidateRevision(in.GetRevision()); err != nil {
return nil, structerr.NewInvalidArgument("revision: %w", err)
}
objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo)
if err != nil {
return nil, err
}
defer cancel()
if err := s.applyGitattributes(ctx, repo, objectReader, repoPath, in.GetRevision()); err != nil {
return nil, err
if deletionErr := deleteInfoAttributesFile(repoPath); deletionErr != nil {
return nil, structerr.NewInternal("delete info/gitattributes file: %w", deletionErr).WithMetadata("path", repoPath)
}
// Once git 2.43.0 is deployed, we can stop using info/attributes in related RPCs,
// As a result, ApplyGitattributes() is made as a no-op,
// so that Gitaly clients will stop writing to info/attributes.
// This gRPC will be totally removed in the once all the housekeeping on removing info/attributes is done.
return &gitalypb.ApplyGitattributesResponse{}, nil
}
internal/gitaly/service/repository/apply_gitattributes_test.go deleted 100644 → 0
+ 0
− 327
View file @ 3f07b901
package repository
import (
"bytes"
"context"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/backchannel"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata"
"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/txinfo"
"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"google.golang.org/grpc"
)
func TestApplyGitattributes_successful(t *testing.T) {
t.Parallel()
ctx := testhelper.Context(t)
cfg, client := setupRepositoryService(t)
repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
gitattributesContent := "pattern attr=value"
commitWithGitattributes := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(
gittest.TreeEntry{Path: ".gitattributes", Mode: "100644", Content: gitattributesContent},
))
commitWithoutGitattributes := gittest.WriteCommit(t, cfg, repoPath)
infoPath := filepath.Join(repoPath, "info")
attributesPath := filepath.Join(infoPath, "attributes")
for _, tc := range []struct {
desc string
revision []byte
expectedContent []byte
}{
{
desc: "With a .gitattributes file",
revision: []byte(commitWithGitattributes),
expectedContent: []byte(gitattributesContent),
},
{
desc: "Without a .gitattributes file",
revision: []byte(commitWithoutGitattributes),
expectedContent: nil,
},
} {
t.Run(tc.desc, func(t *testing.T) {
t.Run("without 'info' directory", func(t *testing.T) {
require.NoError(t, os.RemoveAll(infoPath))
requireApplyGitattributes(t, ctx, client, repo, attributesPath, tc.revision, tc.expectedContent)
})
t.Run("without 'info/attributes' directory", func(t *testing.T) {
require.NoError(t, os.RemoveAll(infoPath))
require.NoError(t, os.Mkdir(infoPath, perm.SharedDir))
requireApplyGitattributes(t, ctx, client, repo, attributesPath, tc.revision, tc.expectedContent)
})
t.Run("with preexisting 'info/attributes'", func(t *testing.T) {
require.NoError(t, os.RemoveAll(infoPath))
require.NoError(t, os.Mkdir(infoPath, perm.SharedDir))
require.NoError(t, os.WriteFile(attributesPath, []byte("*.docx diff=word"), perm.SharedFile))
requireApplyGitattributes(t, ctx, client, repo, attributesPath, tc.revision, tc.expectedContent)
})
})
}
}
type testTransactionServer struct {
gitalypb.UnimplementedRefTransactionServer
vote func(*gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error)
}
func (s *testTransactionServer) VoteTransaction(ctx context.Context, in *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
if s.vote != nil {
return s.vote(in)
}
return nil, nil
}
func TestApplyGitattributes_transactional(t *testing.T) {
t.Parallel()
ctx := testhelper.Context(t)
cfg := testcfg.Build(t)
repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
SkipCreationViaService: true,
})
gitattributesContent := "pattern attr=value"
commitWithGitattributes := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(
gittest.TreeEntry{Path: ".gitattributes", Mode: "100644", Content: gitattributesContent},
))
commitWithoutGitattributes := gittest.WriteCommit(t, cfg, repoPath)
transactionServer := &testTransactionServer{}
runRepositoryService(t, cfg)
// We're using internal listener in order to route around
// Praefect in our tests. Otherwise Praefect would replace our
// carefully crafted transaction and server information.
logger := testhelper.SharedLogger(t)
client := newMuxedRepositoryClient(t, ctx, cfg, "unix://"+cfg.InternalSocketPath(),
backchannel.NewClientHandshaker(
logger,
func() backchannel.Server {
srv := grpc.NewServer()
gitalypb.RegisterRefTransactionServer(srv, transactionServer)
return srv
},
backchannel.DefaultConfiguration(),
),
)
for _, tc := range []struct {
desc string
revision []byte
voteFn func(*testing.T, *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error)
shouldExist bool
expectedErr error
expectedVotes int
}{
{
desc: "successful vote writes gitattributes",
revision: []byte(commitWithGitattributes),
voteFn: func(t *testing.T, request *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
vote := voting.VoteFromData([]byte(gitattributesContent))
expectedHash := vote.Bytes()
require.Equal(t, expectedHash, request.ReferenceUpdatesHash)
return &gitalypb.VoteTransactionResponse{
State: gitalypb.VoteTransactionResponse_COMMIT,
}, nil
},
shouldExist: true,
expectedVotes: 2,
},
{
desc: "aborted vote does not write gitattributes",
revision: []byte(commitWithGitattributes),
voteFn: func(t *testing.T, request *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
return &gitalypb.VoteTransactionResponse{
State: gitalypb.VoteTransactionResponse_ABORT,
}, nil
},
shouldExist: false,
expectedErr: func() error {
return structerr.NewInternal("committing gitattributes: voting on locked file: preimage vote: transaction was aborted")
}(),
expectedVotes: 1,
},
{
desc: "failing vote does not write gitattributes",
revision: []byte(commitWithGitattributes),
voteFn: func(t *testing.T, request *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
return nil, structerr.NewFailedPrecondition("foobar")
},
shouldExist: false,
expectedErr: func() error {
return structerr.NewFailedPrecondition("committing gitattributes: voting on locked file: preimage vote: rpc error: code = FailedPrecondition desc = foobar")
}(),
expectedVotes: 1,
},
{
desc: "commit without gitattributes performs vote",
revision: []byte(commitWithoutGitattributes),
voteFn: func(t *testing.T, request *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
require.Equal(t, bytes.Repeat([]byte{0x00}, 20), request.ReferenceUpdatesHash)
return &gitalypb.VoteTransactionResponse{
State: gitalypb.VoteTransactionResponse_COMMIT,
}, nil
},
shouldExist: false,
expectedVotes: 2,
},
} {
t.Run(tc.desc, func(t *testing.T) {
infoPath := filepath.Join(repoPath, "info")
require.NoError(t, os.RemoveAll(infoPath))
ctx, err := txinfo.InjectTransaction(ctx, 1, "primary", true)
require.NoError(t, err)
ctx = metadata.IncomingToOutgoing(ctx)
var votes int
transactionServer.vote = func(request *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
votes++
return tc.voteFn(t, request)
}
//nolint:staticcheck
_, err = client.ApplyGitattributes(ctx, &gitalypb.ApplyGitattributesRequest{
Repository: repo,
Revision: tc.revision,
})
testhelper.RequireGrpcError(t, tc.expectedErr, err)
path := filepath.Join(infoPath, "attributes")
if tc.shouldExist {
content := testhelper.MustReadFile(t, path)
require.Equal(t, []byte(gitattributesContent), content)
} else {
require.NoFileExists(t, path)
}
require.Equal(t, tc.expectedVotes, votes)
})
}
}
func TestApplyGitattributes_failure(t *testing.T) {
t.Parallel()
ctx := testhelper.Context(t)
cfg, client := setupRepositoryService(t)
repo, _ := gittest.CreateRepository(t, ctx, cfg)
for _, tc := range []struct {
desc string
repo *gitalypb.Repository
revision []byte
expectedErr error
}{
{
desc: "no repository provided",
repo: nil,
revision: nil,
expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
},
{
desc: "unknown storage provided",
repo: &gitalypb.Repository{
RelativePath: "stub",
StorageName: "foo",
},
revision: []byte("master"),
expectedErr: testhelper.ToInterceptedMetadata(structerr.NewInvalidArgument(
"%w", storage.NewStorageNotFoundError("foo"),
)),
},
{
desc: "storage not provided",
repo: &gitalypb.Repository{
RelativePath: repo.GetRelativePath(),
},
revision: []byte("master"),
expectedErr: structerr.NewInvalidArgument("%w", storage.ErrStorageNotSet),
},
{
desc: "repository doesn't exist on disk",
repo: &gitalypb.Repository{
StorageName: repo.GetStorageName(),
RelativePath: "bar",
},
revision: []byte("master"),
expectedErr: testhelper.ToInterceptedMetadata(
structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "bar")),
),
},
{
desc: "no revision provided",
repo: repo,
revision: []byte(""),
expectedErr: structerr.NewInvalidArgument("revision: empty revision"),
},
{
desc: "unknown revision",
repo: repo,
revision: []byte("not-existing-ref"),
expectedErr: structerr.NewInvalidArgument("revision does not exist"),
},
{
desc: "invalid revision",
repo: repo,
revision: []byte("--output=/meow"),
expectedErr: structerr.NewInvalidArgument("revision: revision can't start with '-'"),
},
} {
t.Run(tc.desc, func(t *testing.T) {
//nolint:staticcheck
_, err := client.ApplyGitattributes(ctx, &gitalypb.ApplyGitattributesRequest{
Repository: tc.repo,
Revision: tc.revision,
})
testhelper.RequireGrpcError(t, tc.expectedErr, err)
})
}
}
func requireApplyGitattributes(
t *testing.T,
ctx context.Context,
client gitalypb.RepositoryServiceClient,
repo *gitalypb.Repository,
attributesPath string,
revision, expectedContent []byte,
) {
t.Helper()
//nolint:staticcheck
response, err := client.ApplyGitattributes(ctx, &gitalypb.ApplyGitattributesRequest{
Repository: repo,
Revision: revision,
})
require.NoError(t, err)
testhelper.ProtoEqual(t, &gitalypb.ApplyGitattributesResponse{}, response)
if expectedContent == nil {
require.NoFileExists(t, attributesPath)
} else {
require.Equal(t, expectedContent, testhelper.MustReadFile(t, attributesPath))
info, err := os.Stat(attributesPath)
require.NoError(t, err)
require.Equal(t, attributesFileMode, info.Mode())
}
}
internal/gitaly/service/repository/file_attributes.go
+ 27
− 0
View file @ c35ecf60
......@@ -3,6 +3,8 @@ package repository
import (
"context"
"errors"
"os"
"path/filepath"
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gitattributes"
......@@ -18,6 +20,20 @@ func (s *server) GetFileAttributes(ctx context.Context, in *gitalypb.GetFileAttr
repo := s.localrepo(in.GetRepository())
// 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 RPC are totally removed from
// the code base.
repoPath, err := s.locator.GetRepoPath(repo)
if err != nil {
return nil, structerr.NewInternal("get repo path: %w", err)
}
if deletionErr := deleteInfoAttributesFile(repoPath); deletionErr != nil {
return nil, structerr.NewInternal("delete info/gitattributes file: %w", err).WithMetadata("path", repoPath)
}
checkAttrCmd, finishAttr, err := gitattributes.CheckAttr(ctx, repo, git.Revision(in.GetRevision()), in.GetAttributes())
if err != nil {
return nil, structerr.New("check attr: %w", err)
......@@ -60,3 +76,14 @@ func validateGetFileAttributesRequest(locator storage.Locator, in *gitalypb.GetF
return nil
}
// deleteInfoAttributesFile delete the info/attributes files in the repoPath
func deleteInfoAttributesFile(repoPath string) error {
attrFile := filepath.Join(repoPath, "info", "attributes")
err := os.Remove(attrFile)
if err != nil && !os.IsNotExist(err) {
return err
}
return nil
}
internal/gitaly/service/repository/file_attributes_test.go
+ 41
− 0
View file @ c35ecf60
package repository
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
......@@ -164,3 +167,41 @@ func TestGetFileAttributes(t *testing.T) {
})
}
}
func TestDeletingInfoAttributes(t *testing.T) {
testhelper.SkipWithWAL(t, "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.")
ctx := testhelper.Context(t)
cfg, client := setupRepositoryService(t)
repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
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},
gittest.TreeEntry{Path: "example.go", Mode: "100644", Content: "something important"},
gittest.TreeEntry{Path: "README.md", Mode: "100644", Content: "some text"},
gittest.TreeEntry{Path: "pic.jpg", Mode: "100644", Content: "blob"},
))
path := filepath.Join(repoPath, "info")
require.NoError(t, os.Mkdir(path, os.ModePerm))
path = filepath.Join(path, "attributes")
file, err := os.Create(path)
require.NoError(t, err)
require.NoError(t, file.Close())
request := &gitalypb.GetFileAttributesRequest{
Repository: repoProto,
Revision: []byte("main"),
Attributes: []string{"diff"},
Paths: []string{"example.go"},
}
_, _ = client.GetFileAttributes(ctx, request)
// when gitattributesSupportReadingFromHead is true, the info/attributes file should be deleted
// otherwise it should not be deleted
require.NoFileExists(t, path)
}
internal/gitaly/service/repository/info_attributes.go
+ 42
− 9
View file @ c35ecf60
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
}
internal/gitaly/service/repository/info_attributes_test.go
+ 16
− 1
View file @ c35ecf60
......@@ -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) {
......
internal/gitaly/service/repository/replicate.go
+ 13
− 32
View file @ c35ecf60
......@@ -97,8 +97,19 @@ func (s *server) replicateRepository(ctx context.Context, source, target *gitaly
return fmt.Errorf("synchronizing gitconfig: %w", err)
}
if err := s.syncInfoAttributes(ctx, source, target); err != nil {
return fmt.Errorf("synchronizing gitattributes: %w", err)
// 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 RPC are totally removed from
// the code base.
if target != nil {
repoPath, err := s.locator.GetRepoPath(target)
if err != nil {
return structerr.NewInternal("get repo path: %w", err)
}
if deletionErr := deleteInfoAttributesFile(repoPath); deletionErr != nil {
return structerr.NewInternal("delete info/gitattributes file: %w", deletionErr).WithMetadata("path", repoPath)
}
}
if replicateObjectDeduplicationNetworkMembership {
......@@ -360,36 +371,6 @@ func (s *server) syncGitconfig(ctx context.Context, source, target *gitalypb.Rep
return nil
}
func (s *server) syncInfoAttributes(ctx context.Context, source, target *gitalypb.Repository) error {
repoClient, err := s.newRepoClient(ctx, source.GetStorageName())
if err != nil {
return err
}
repoPath, err := s.locator.GetRepoPath(target)
if err != nil {
return err
}
//nolint:staticcheck
stream, err := repoClient.GetInfoAttributes(ctx, &gitalypb.GetInfoAttributesRequest{
Repository: source,
})
if err != nil {
return err
}
attributesPath := filepath.Join(repoPath, "info", "attributes")
if err := s.writeFile(ctx, attributesPath, attributesFileMode, streamio.NewReader(func() ([]byte, error) {
resp, err := stream.Recv()
return resp.GetAttributes(), err
})); err != nil {
return err
}
return nil
}
// syncObjectPool syncs the Git alternates file and sets up object pools as needed.
func (s *server) syncObjectPool(ctx context.Context, sourceRepoProto, targetRepoProto *gitalypb.Repository) error {
sourceObjectPoolClient, err := s.newObjectPoolClient(ctx, sourceRepoProto.GetStorageName())
......
internal/gitaly/service/repository/replicate_test.go
+ 12
− 18
View file @ c35ecf60
......@@ -606,18 +606,6 @@ attributes from HEAD.`)
"config file must match",
)
// Verify info attributes matches.
sourceAttributesData, err := os.ReadFile(filepath.Join(sourcePath, "info", "attributes"))
if err != nil {
require.ErrorIs(t, err, os.ErrNotExist)
}
require.Equal(t,
string(sourceAttributesData),
string(testhelper.MustReadFile(t, filepath.Join(targetPath, "info", "attributes"))),
"info/attributes file must match",
)
// Verify custom hooks replicated.
var targetHooks []string
targetHooksPath := filepath.Join(targetPath, repoutil.CustomHooksDir)
......@@ -716,8 +704,6 @@ func TestReplicateRepository_transactional(t *testing.T) {
require.NoError(t, err)
// There is no gitattributes file, so we vote on the empty contents of that file.
gitattributesVote := voting.VoteFromData([]byte{})
// There is a gitconfig though, so the vote should reflect its contents.
gitconfigVote := voting.VoteFromData(testhelper.MustReadFile(t, filepath.Join(sourceRepoPath, "config")))
......@@ -738,8 +724,6 @@ func TestReplicateRepository_transactional(t *testing.T) {
votes[0],
gitconfigVote,
gitconfigVote,
gitattributesVote,
gitattributesVote,
noHooksVote,
noHooksVote,
}
......@@ -766,8 +750,6 @@ func TestReplicateRepository_transactional(t *testing.T) {
expectedVotes = []voting.Vote{
gitconfigVote,
gitconfigVote,
gitattributesVote,
gitattributesVote,
replicationVote,
replicationVote,
noHooksVote,
......@@ -918,3 +900,15 @@ func listenGitalySSHCalls(t *testing.T, conf config.Cfg) func() gitalySSHParams
}
}
}
type testTransactionServer struct {
gitalypb.UnimplementedRefTransactionServer
vote func(*gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error)
}
func (s *testTransactionServer) VoteTransaction(ctx context.Context, in *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) {
if s.vote != nil {
return s.vote(in)
}
return nil, nil
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment