From 6a32f1ccb55ad9cc684c0cf67130204dd985ed30 Mon Sep 17 00:00:00 2001 From: John Cai Date: Thu, 24 Oct 2019 16:42:15 -0700 Subject: Add GetObjectPool RPC Adds an RPC to get a repository's object pool. Also added a method under internal/git/objectpool to get an object pool of a repository. --- internal/git/objectpool/link.go | 51 +++++++------------ internal/git/objectpool/pool.go | 98 ++++++++++++++++++++++++++++++++++++ internal/git/objectpool/pool_test.go | 70 ++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 32 deletions(-) (limited to 'internal/git') diff --git a/internal/git/objectpool/link.go b/internal/git/objectpool/link.go index 92edeca99..9f062cb0c 100644 --- a/internal/git/objectpool/link.go +++ b/internal/git/objectpool/link.go @@ -1,7 +1,6 @@ package objectpool import ( - "bufio" "context" "errors" "fmt" @@ -13,7 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/remote" "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -29,23 +27,18 @@ func (o *ObjectPool) Link(ctx context.Context, repo *gitalypb.Repository) error return err } - relPath, err := o.getRelativeObjectPath(repo) + expectedRelPath, err := o.getRelativeObjectPath(repo) if err != nil { return err } - expectedContent := filepath.Join(relPath, "objects") - - actualContentBytes, err := ioutil.ReadFile(altPath) - if err == nil { - actualContent := text.ChompBytes(actualContentBytes) - if actualContent == expectedContent { - return nil - } + linked, err := o.LinkedToRepository(repo) + if err != nil { + return err + } - if filepath.Clean(actualContent) != filepath.Join(o.FullPath(), "objects") { - return fmt.Errorf("unexpected alternates content: %q", actualContent) - } + if linked { + return nil } if err != nil && !os.IsNotExist(err) { @@ -58,7 +51,7 @@ func (o *ObjectPool) Link(ctx context.Context, repo *gitalypb.Repository) error } defer os.Remove(tmp.Name()) - if _, err := io.WriteString(tmp, expectedContent); err != nil { + if _, err := io.WriteString(tmp, expectedRelPath); err != nil { return err } @@ -142,36 +135,30 @@ func (o *ObjectPool) getRelativeObjectPath(repo *gitalypb.Repository) (string, e return "", err } - return relPath, nil + return filepath.Join(relPath, "objects"), nil } // LinkedToRepository tests if a repository is linked to an object pool func (o *ObjectPool) LinkedToRepository(repo *gitalypb.Repository) (bool, error) { - altPath, err := git.InfoAlternatesPath(repo) + relPath, err := getAlternateObjectDir(repo) if err != nil { + if err == ErrAlternateObjectDirNotExist { + return false, nil + } return false, err } - relPath, err := o.getRelativeObjectPath(repo) + expectedRelPath, err := o.getRelativeObjectPath(repo) if err != nil { return false, err } - if stat, err := os.Stat(altPath); err == nil && stat.Size() > 0 { - alternatesFile, err := os.Open(altPath) - if err != nil { - return false, err - } - defer alternatesFile.Close() - - r := bufio.NewReader(alternatesFile) - - b, err := r.ReadBytes('\n') - if err != nil && err != io.EOF { - return false, fmt.Errorf("reading alternates file: %v", err) - } + if relPath == expectedRelPath { + return true, nil + } - return string(b) == filepath.Join(relPath, "objects"), nil + if filepath.Clean(relPath) != filepath.Join(o.FullPath(), "objects") { + return false, fmt.Errorf("unexpected alternates content: %q", relPath) } return false, nil diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go index 4a44bf6bc..5674421ab 100644 --- a/internal/git/objectpool/pool.go +++ b/internal/git/objectpool/pool.go @@ -1,10 +1,15 @@ package objectpool import ( + "bufio" + "bytes" "context" + "errors" "fmt" + "io" "os" "path" + "path/filepath" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -111,3 +116,96 @@ func (o *ObjectPool) Init(ctx context.Context) (err error) { return cmd.Wait() } + +// FromRepo returns an instance of ObjectPool that the repository points to +func FromRepo(repo *gitalypb.Repository) (*ObjectPool, error) { + dir, err := getAlternateObjectDir(repo) + if err != nil { + return nil, err + } + + if dir == "" { + return nil, nil + } + + altPathRelativeToStorage, err := objectPathRelativeToStorage(repo, dir) + if err != nil { + return nil, err + } + + return NewObjectPool(repo.GetStorageName(), filepath.Dir(altPathRelativeToStorage)) +} + +var ( + // ErrInvalidPoolRepository indicates the directory the alternates file points to is not a valid git repository + ErrInvalidPoolRepository = errors.New("object pool is not a valid git repository") + + // ErrAlternateObjectDirNotExist indicates a repository does not have an alternates file + ErrAlternateObjectDirNotExist = errors.New("no alternates directory exists") +) + +// getAlternateObjectDir returns the entry in the objects/info/attributes file if it exists +// it will only return the first line of the file if there are multiple lines. +func getAlternateObjectDir(repo *gitalypb.Repository) (string, error) { + altPath, err := git.InfoAlternatesPath(repo) + if err != nil { + return "", err + } + + if _, err = os.Stat(altPath); err != nil { + if os.IsNotExist(err) { + return "", ErrAlternateObjectDirNotExist + } + return "", err + } + + altFile, err := os.Open(altPath) + if err != nil { + return "", err + } + defer altFile.Close() + + r := bufio.NewReader(altFile) + b, err := r.ReadBytes('\n') + if err != nil && err != io.EOF { + return "", fmt.Errorf("reading alternates file: %v", err) + } + + if err == nil { + b = b[:len(b)-1] + } + + if bytes.HasPrefix(b, []byte("#")) { + return "", ErrAlternateObjectDirNotExist + } + + return string(b), nil +} + +// objectPathRelativeToStorage takes an object path that's relative to a repository's object directory +// and returns the path relative to the storage path of the repository. +func objectPathRelativeToStorage(repo *gitalypb.Repository, path string) (string, error) { + repoPath, err := helper.GetPath(repo) + if err != nil { + return "", err + } + + storagePath, err := helper.GetStorageByName(repo.GetStorageName()) + if err != nil { + return "", err + } + objectDirPath := filepath.Join(repoPath, "objects") + + poolObjectDirFullPath := filepath.Join(objectDirPath, path) + + if !helper.IsGitDirectory(filepath.Dir(poolObjectDirFullPath)) { + return "", ErrInvalidPoolRepository + } + + poolRelPath, err := filepath.Rel(storagePath, poolObjectDirFullPath) + if err != nil { + return "", err + } + + return poolRelPath, nil +} diff --git a/internal/git/objectpool/pool_test.go b/internal/git/objectpool/pool_test.go index e609f5027..530ccee94 100644 --- a/internal/git/objectpool/pool_test.go +++ b/internal/git/objectpool/pool_test.go @@ -1,8 +1,10 @@ package objectpool import ( + "io/ioutil" "os" "path" + "path/filepath" "strings" "testing" @@ -19,6 +21,74 @@ func TestNewObjectPool(t *testing.T) { require.Error(t, err, "creating pool in storage that does not exist should fail") } +func TestNewFromRepoSuccess(t *testing.T) { + ctx, cancel := testhelper.Context() + testRepo, _, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + relativePoolPath := testhelper.NewTestObjectPoolName(t) + + pool, err := NewObjectPool(testRepo.GetStorageName(), relativePoolPath) + require.NoError(t, err) + defer pool.Remove(ctx) + + defer cancel() + require.NoError(t, pool.Create(ctx, testRepo)) + require.NoError(t, pool.Link(ctx, testRepo)) + + poolFromRepo, err := FromRepo(testRepo) + require.NoError(t, err) + require.Equal(t, relativePoolPath, poolFromRepo.relativePath) + require.Equal(t, pool.storageName, poolFromRepo.storageName) +} + +func TestNewFromRepoNoObjectPool(t *testing.T) { + testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + // no alternates file + poolFromRepo, err := FromRepo(testRepo) + require.Equal(t, ErrAlternateObjectDirNotExist, err) + require.Nil(t, poolFromRepo) + + // with an alternates file + testCases := []struct { + desc string + fileContent []byte + expectedErr error + }{ + { + desc: "points to non existent path", + fileContent: []byte("/tmp/invalid_path"), + expectedErr: ErrInvalidPoolRepository, + }, + { + desc: "empty file", + fileContent: nil, + expectedErr: nil, + }, + { + desc: "first line commented out", + fileContent: []byte("#/tmp/invalid/path"), + expectedErr: ErrAlternateObjectDirNotExist, + }, + } + + require.NoError(t, os.MkdirAll(filepath.Join(testRepoPath, "objects", "info"), 0755)) + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + alternateFilePath := filepath.Join(testRepoPath, "objects", "info", "alternates") + require.NoError(t, ioutil.WriteFile(alternateFilePath, tc.fileContent, 0644)) + poolFromRepo, err := FromRepo(testRepo) + require.Equal(t, tc.expectedErr, err) + require.Nil(t, poolFromRepo) + + require.NoError(t, os.Remove(alternateFilePath)) + }) + } +} + func TestCreate(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() -- cgit v1.2.3