diff --git a/cmd/gitaly-git2go/submodule_test.go b/cmd/gitaly-git2go/submodule_test.go index 51bb84d9f57f526204667ebd1e387f1a2eb509a8..cf9ab14b1144cfa268d7a423dfac28c8c7f10474 100644 --- a/cmd/gitaly-git2go/submodule_test.go +++ b/cmd/gitaly-git2go/submodule_test.go @@ -101,7 +101,7 @@ func TestSubmodule(t *testing.T) { } require.NoError(t, err) - commit, err := log.GetCommit(ctx, config.NewLocator(config.Config), testRepo, response.CommitID) + commit, err := log.GetCommit(ctx, testRepo, response.CommitID) require.NoError(t, err) require.Equal(t, commit.Author.Email, testhelper.TestUser.Email) require.Equal(t, commit.Committer.Email, testhelper.TestUser.Email) diff --git a/internal/git/alternates/alternates.go b/internal/git/alternates/alternates.go index f74180a6de257ff5019c0736477d06d3a288d24f..60636da83e880619cdd8842fe57bc21193add6b1 100644 --- a/internal/git/alternates/alternates.go +++ b/internal/git/alternates/alternates.go @@ -4,8 +4,27 @@ import ( "fmt" "path/filepath" "strings" + + "gitlab.com/gitlab-org/gitaly/internal/git/repository" + "gitlab.com/gitlab-org/gitaly/internal/helper" ) +// PathAndEnv finds the disk path to a repository, and returns the +// alternate object directory environment variables encoded in the +// gitalypb.Repository instance. +// Deprecated: please use storage.Locator to define the project path and alternates.Env +// to get alternate object directory environment variables. +func PathAndEnv(repo repository.GitRepo) (string, []string, error) { + repoPath, err := helper.GetRepoPath(repo) + if err != nil { + return "", nil, err + } + + env := Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) + + return repoPath, env, nil +} + // Env returns the alternate object directory environment variables. func Env(repoPath, objectDirectory string, alternateObjectDirectories []string) []string { var env []string diff --git a/internal/git/catfile/batch.go b/internal/git/catfile/batch.go index f4a8d125e0fb5722cea0c3c57fc6d8f22921c13e..58bf98c306ccd70e4db0f13529ff7410bc71115f 100644 --- a/internal/git/catfile/batch.go +++ b/internal/git/catfile/batch.go @@ -12,7 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/alternates" "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/labkit/correlation" ) @@ -35,14 +34,12 @@ type batchProcess struct { sync.Mutex } -func newBatchProcess(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (*batchProcess, error) { - repoPath, err := locator.GetRepoPath(repo) +func newBatchProcess(ctx context.Context, repo repository.GitRepo) (*batchProcess, error) { + repoPath, env, err := alternates.PathAndEnv(repo) if err != nil { return nil, err } - env := alternates.Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) - totalCatfileProcesses.Inc() b := &batchProcess{} diff --git a/internal/git/catfile/batchcheck.go b/internal/git/catfile/batchcheck.go index f11075de7f041c1f0163f3df11a367ab109da270..28afdd9b717924e8c3d6f94b0a5b5a6a512a4de5 100644 --- a/internal/git/catfile/batchcheck.go +++ b/internal/git/catfile/batchcheck.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/alternates" "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/labkit/correlation" ) @@ -22,14 +21,12 @@ type batchCheck struct { sync.Mutex } -func newBatchCheck(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (*batchCheck, error) { - repoPath, err := locator.GetRepoPath(repo) +func newBatchCheck(ctx context.Context, repo repository.GitRepo) (*batchCheck, error) { + repoPath, env, err := alternates.PathAndEnv(repo) if err != nil { return nil, err } - env := alternates.Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) - bc := &batchCheck{} var stdinReader io.Reader diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go index f07026c61e58f25fe333d58e6ee18336d725a4a1..57769d7965e35096c5445a97abdf8efcab0bf9da 100644 --- a/internal/git/catfile/catfile.go +++ b/internal/git/catfile/catfile.go @@ -8,7 +8,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/metadata" - "gitlab.com/gitlab-org/gitaly/internal/storage" ) var catfileCacheCounter = prometheus.NewCounterVec( @@ -142,14 +141,14 @@ func (c *batch) isClosed() bool { // New returns a new Batch instance. It is important that ctx gets canceled // somewhere, because if it doesn't the cat-file processes spawned by // New() never terminate. -func New(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (Batch, error) { +func New(ctx context.Context, repo repository.GitRepo) (Batch, error) { if ctx.Done() == nil { panic("empty ctx.Done() in catfile.Batch.New()") } sessionID := metadata.GetValue(ctx, SessionIDField) if sessionID == "" { - c, err := newBatch(ctx, locator, repo) + c, err := newBatch(ctx, repo) if err != nil { return nil, err } @@ -167,7 +166,7 @@ func New(ctx context.Context, locator storage.Locator, repo repository.GitRepo) // if we are using caching, create a fresh context for the new batch // and initialize the new batch with a cache key and cancel function cacheCtx, cacheCancel := context.WithCancel(context.Background()) - c, err := newBatch(cacheCtx, locator, repo) + c, err := newBatch(cacheCtx, repo) if err != nil { cacheCancel() return nil, err @@ -201,7 +200,7 @@ type simulatedBatchSpawnError struct{} func (simulatedBatchSpawnError) Error() string { return "simulated spawn error" } -func newBatch(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (_ *batch, err error) { +func newBatch(ctx context.Context, repo repository.GitRepo) (_ *batch, err error) { ctx, cancel := context.WithCancel(ctx) defer func() { if err != nil { @@ -209,12 +208,12 @@ func newBatch(ctx context.Context, locator storage.Locator, repo repository.GitR } }() - b, err := newBatchProcess(ctx, locator, repo) + b, err := newBatchProcess(ctx, repo) if err != nil { return nil, err } - batchCheck, err := newBatchCheck(ctx, locator, repo) + batchCheck, err := newBatchCheck(ctx, repo) if err != nil { return nil, err } diff --git a/internal/git/catfile/catfile_test.go b/internal/git/catfile/catfile_test.go index 9f0a6827484483eb6c26e2575f9f45249fb5f2f3..a370e686de9ac0f7d6553f452d301a2a7fae7b3e 100644 --- a/internal/git/catfile/catfile_test.go +++ b/internal/git/catfile/catfile_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -27,7 +26,7 @@ func TestInfo(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) testCases := []struct { @@ -63,7 +62,7 @@ func TestBlob(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) gitignoreBytes, err := ioutil.ReadFile("testdata/blob-dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82") @@ -130,7 +129,7 @@ func TestCommit(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) commitBytes, err := ioutil.ReadFile("testdata/commit-e63f41fe459e62e1228fcef60d7189127aeba95a") @@ -168,7 +167,7 @@ func TestTag(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) tagBytes, err := ioutil.ReadFile("testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76") @@ -235,7 +234,7 @@ func TestTree(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) treeBytes, err := ioutil.ReadFile("testdata/tree-7e2f26d033ee47cd0745649d1a28277c56197921") @@ -302,7 +301,7 @@ func TestRepeatedCalls(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, config.NewLocator(config.Config), testRepository) + c, err := New(ctx, testRepository) require.NoError(t, err) treeOid := "7e2f26d033ee47cd0745649d1a28277c56197921" @@ -417,7 +416,7 @@ func catfileWithFreshSessionID(ctx context.Context, repo *gitalypb.Repository) ( SessionIDField: id, }) - return New(metadata.NewIncomingContext(ctx, md), config.NewLocator(config.Config), repo) + return New(metadata.NewIncomingContext(ctx, md), repo) } func waitTrue(callback func() bool) bool { diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go index cf25df46565aabf1921513bd2c2fd4562df2995d..e6bf695c8604317a4ec7dca4382a2c8b8e6efbb9 100644 --- a/internal/git/log/commit.go +++ b/internal/git/log/commit.go @@ -13,14 +13,13 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) // GetCommit tries to resolve revision to a Git commit. Returns nil if // no object is found at revision. -func GetCommit(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, revision string) (*gitalypb.GitCommit, error) { - c, err := catfile.New(ctx, locator, repo) +func GetCommit(ctx context.Context, repo *gitalypb.Repository, revision string) (*gitalypb.GitCommit, error) { + c, err := catfile.New(ctx, repo) if err != nil { return nil, err } diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go index 4fcf0777b20dea7f72c6e67e95804b9d5d85e0dd..470ddfb5a125a136d5715cdf388b822986eb53eb 100644 --- a/internal/git/log/commit_test.go +++ b/internal/git/log/commit_test.go @@ -8,7 +8,6 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/metadata" @@ -151,7 +150,7 @@ func TestGetCommitCatfile(t *testing.T) { }, } - c, err := catfile.New(ctx, config.NewLocator(config.Config), testRepo) + c, err := catfile.New(ctx, testRepo) require.NoError(t, err) for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/git/log/log.go b/internal/git/log/log.go index 6081a43d7c01c87810d228cb7512a5eebf974f5e..462a9ed557c8a7200afb48619c0865464de7d7ee 100644 --- a/internal/git/log/log.go +++ b/internal/git/log/log.go @@ -7,7 +7,6 @@ import ( "io" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -23,8 +22,8 @@ type Parser struct { } // NewLogParser returns a new Parser -func NewLogParser(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, src io.Reader) (*Parser, error) { - c, err := catfile.New(ctx, locator, repo) +func NewLogParser(ctx context.Context, repo *gitalypb.Repository, src io.Reader) (*Parser, error) { + c, err := catfile.New(ctx, repo) if err != nil { return nil, err } diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go index 31a639890f6403993ae73dcb486c89d7a683cdb8..ab975ee25f1b7509b21a4535bb16042cab127c0e 100644 --- a/internal/git/log/tag_test.go +++ b/internal/git/log/tag_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -47,7 +46,7 @@ func TestGetTag(t *testing.T) { }, } - c, err := catfile.New(ctx, config.NewLocator(config.Config), testRepo) + c, err := catfile.New(ctx, testRepo) require.NoError(t, err) for _, testCase := range testCases { t.Run(testCase.tagName, func(t *testing.T) { diff --git a/internal/git/objectpool/link.go b/internal/git/objectpool/link.go index 22a5c75e5dc0d7679802dbf502027cba0b335df1..ea48e2787a739b9585738c503df3f2add3beafbb 100644 --- a/internal/git/objectpool/link.go +++ b/internal/git/objectpool/link.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/remote" "gitlab.com/gitlab-org/gitaly/internal/git/repository" + "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -71,12 +72,7 @@ func (o *ObjectPool) Link(ctx context.Context, repo *gitalypb.Repository) error // change "eventually" to "immediately", so that users won't see the // warning. https://gitlab.com/gitlab-org/gitaly/issues/1728 func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { - poolPath, err := o.locator.GetPath(o) - if err != nil { - return err - } - - poolBitmaps, err := getBitmaps(poolPath) + poolBitmaps, err := getBitmaps(o) if err != nil { return err } @@ -84,12 +80,7 @@ func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { return nil } - repoPath, err := o.locator.GetPath(repo) - if err != nil { - return err - } - - memberBitmaps, err := getBitmaps(repoPath) + memberBitmaps, err := getBitmaps(repo) if err != nil { return err } @@ -106,7 +97,12 @@ func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { return nil } -func getBitmaps(repoPath string) ([]string, error) { +func getBitmaps(repo repository.GitRepo) ([]string, error) { + repoPath, err := helper.GetPath(repo) + if err != nil { + return nil, err + } + packDir := filepath.Join(repoPath, "objects/pack") entries, err := ioutil.ReadDir(packDir) if err != nil { diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 1fc2ff7229d3da4adcbcc123fcfe8653ed2b1c5b..889cf7c12d902f9f96af7cd0839be5625a1dca45 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -42,8 +41,7 @@ func TestCreate(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - locator := config.NewLocator(config.Config) - headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -56,7 +54,7 @@ func TestCreate(t *testing.T) { require.NoError(t, updater.Wait()) // check the ref was created - commit, logErr := log.GetCommit(ctx, locator, testRepo, ref) + commit, logErr := log.GetCommit(ctx, testRepo, ref) require.NoError(t, logErr) require.Equal(t, commit.Id, sha, "reference was created with the wrong SHA") } @@ -65,8 +63,7 @@ func TestUpdate(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - locator := config.NewLocator(config.Config) - headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -76,7 +73,7 @@ func TestUpdate(t *testing.T) { sha := headCommit.Id // Sanity check: ensure the ref exists before we start - commit, logErr := log.GetCommit(ctx, locator, testRepo, ref) + commit, logErr := log.GetCommit(ctx, testRepo, ref) require.NoError(t, logErr) require.NotEqual(t, commit.Id, sha, "%s points to HEAD: %s in the test repository", ref, sha) @@ -84,17 +81,17 @@ func TestUpdate(t *testing.T) { require.NoError(t, updater.Wait()) // check the ref was updated - commit, logErr = log.GetCommit(ctx, locator, testRepo, ref) + commit, logErr = log.GetCommit(ctx, testRepo, ref) require.NoError(t, logErr) require.Equal(t, commit.Id, sha, "reference was not updated") // since ref has been updated to HEAD, we know that it does not point to HEAD^. So, HEAD^ is an invalid "old value" for updating ref - parentCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD^") + parentCommit, err := log.GetCommit(ctx, testRepo, "HEAD^") require.NoError(t, err) require.Error(t, updater.Update(ref, parentCommit.Id, parentCommit.Id)) // check the ref was not updated - commit, logErr = log.GetCommit(ctx, locator, testRepo, ref) + commit, logErr = log.GetCommit(ctx, testRepo, ref) require.NoError(t, logErr) require.NotEqual(t, commit.Id, parentCommit.Id, "reference was updated when it shouldn't have been") } @@ -111,10 +108,8 @@ func TestDelete(t *testing.T) { require.NoError(t, updater.Delete(ref)) require.NoError(t, updater.Wait()) - locator := config.NewLocator(config.Config) - // check the ref was removed - _, err = log.GetCommit(ctx, locator, testRepo, ref) + _, err = log.GetCommit(ctx, testRepo, ref) require.True(t, log.IsNotFound(err), "expected 'not found' error got %v", err) } @@ -122,9 +117,7 @@ func TestBulkOperation(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - locator := config.NewLocator(config.Config) - - headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -146,9 +139,7 @@ func TestContextCancelAbortsRefChanges(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - locator := config.NewLocator(config.Config) - - headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") require.NoError(t, err) childCtx, childCancel := context.WithCancel(ctx) @@ -164,7 +155,7 @@ func TestContextCancelAbortsRefChanges(t *testing.T) { require.Error(t, updater.Wait()) // check the ref doesn't exist - _, err = log.GetCommit(ctx, locator, testRepo, ref) + _, err = log.GetCommit(ctx, testRepo, ref) require.True(t, log.IsNotFound(err), "expected 'not found' error got %v", err) } diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go index c3f36d4d2f8afa7ab320f99dd1adcb9a3bd111a6..b94c4e0308759b22a619f77c98491903c8db9082 100644 --- a/internal/gitaly/service/blob/get_blob.go +++ b/internal/gitaly/service/blob/get_blob.go @@ -19,7 +19,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic return status.Errorf(codes.InvalidArgument, "GetBlob: %v", err) } - c, err := catfile.New(stream.Context(), s.locator, in.Repository) + c, err := catfile.New(stream.Context(), in.Repository) if err != nil { return status.Errorf(codes.Internal, "GetBlob: %v", err) } diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go index f8eb5ad867d98a8f036c9c3d398187ebbafcbfab..ba3fec0b49177ed0f78e8fcfd89c883551e5fc18 100644 --- a/internal/gitaly/service/blob/get_blobs.go +++ b/internal/gitaly/service/blob/get_blobs.go @@ -139,12 +139,12 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob return nil } -func (s *server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { +func (*server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { if err := validateGetBlobsRequest(req); err != nil { return err } - c, err := catfile.New(stream.Context(), s.locator, req.Repository) + c, err := catfile.New(stream.Context(), req.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go index 67b80995ae37f0019da903945bd21fe74fa73879..f20ee57fcaafbf532405bf5e02f5e8ab462af70f 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go @@ -40,7 +40,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg reader := &bfgStreamReader{firstRequest: firstRequest, server: server} chunker := chunk.New(&bfgStreamWriter{server: server}) - notifier, err := notifier.New(ctx, s.locator, repo, chunker) + notifier, err := notifier.New(ctx, repo, chunker) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go index d910b20b57c861c202b5150afa1f07564ec413da..ec436f60c0dbfeb71b6f0a7cbe7e13557a14504a 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go @@ -18,8 +18,6 @@ import ( ) func TestApplyBfgObjectMapStreamSuccess(t *testing.T) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runCleanupServiceServer(t, config.Config) defer stop() @@ -32,7 +30,7 @@ func TestApplyBfgObjectMapStreamSuccess(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") require.NoError(t, err) // A known blob: the CHANGELOG in the test repository diff --git a/internal/gitaly/service/cleanup/notifier/notifier.go b/internal/gitaly/service/cleanup/notifier/notifier.go index 7817c4d601074d18763ac44888f705168f334b63..ab2163f9a671253cd56f9e800439e2415a2ee846 100644 --- a/internal/gitaly/service/cleanup/notifier/notifier.go +++ b/internal/gitaly/service/cleanup/notifier/notifier.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -17,8 +16,8 @@ type Notifier struct { } // New instantiates a new Notifier -func New(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, chunker *chunk.Chunker) (*Notifier, error) { - catfile, err := catfile.New(ctx, locator, repo) +func New(ctx context.Context, repo *gitalypb.Repository, chunker *chunk.Chunker) (*Notifier, error) { + catfile, err := catfile.New(ctx, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/cleanup/server.go b/internal/gitaly/service/cleanup/server.go index 4e8137dbbf689c950953b93448094eb66e830539..e482b6069e9fdc25377678e74c3ecb006ff1d7bc 100644 --- a/internal/gitaly/service/cleanup/server.go +++ b/internal/gitaly/service/cleanup/server.go @@ -1,15 +1,13 @@ package cleanup import ( - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) type server struct { - locator storage.Locator } // NewServer creates a new instance of a grpc CleanupServer -func NewServer(locator storage.Locator) gitalypb.CleanupServiceServer { - return &server{locator: locator} +func NewServer() gitalypb.CleanupServiceServer { + return &server{} } diff --git a/internal/gitaly/service/cleanup/testhelper_test.go b/internal/gitaly/service/cleanup/testhelper_test.go index 45125dddd3abe18170a8cb50ad06c0e559770239..a0807ca3d861ba9edee8ad4072baf67f6ab94ecc 100644 --- a/internal/gitaly/service/cleanup/testhelper_test.go +++ b/internal/gitaly/service/cleanup/testhelper_test.go @@ -29,9 +29,8 @@ func testMain(m *testing.M) int { func runCleanupServiceServer(t *testing.T, cfg config.Cfg) (string, func()) { srv := testhelper.NewServer(t, nil, nil, testhelper.WithInternalSocket(cfg)) - locator := config.NewLocator(cfg) - gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer(locator)) - gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(cfg, hook.NewManager(locator, hook.GitlabAPIStub, cfg))) + gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer()) + gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(cfg, hook.NewManager(config.NewLocator(cfg), hook.GitlabAPIStub, cfg))) reflection.Register(srv.GrpcServer()) require.NoError(t, srv.Start()) diff --git a/internal/gitaly/service/commit/between.go b/internal/gitaly/service/commit/between.go index 05e8df7fa3ffe76d19e7c2894a999c769bdf7359..2a424ab4851d6db29a739f725f6655bb9088150e 100644 --- a/internal/gitaly/service/commit/between.go +++ b/internal/gitaly/service/commit/between.go @@ -31,7 +31,7 @@ func (s *server) CommitsBetween(in *gitalypb.CommitsBetweenRequest, stream gital sender := &commitsBetweenSender{stream: stream} revisionRange := fmt.Sprintf("%s..%s", in.GetFrom(), in.GetTo()) - if err := sendCommits(stream.Context(), sender, s.locator, in.GetRepository(), []string{revisionRange}, nil, nil, git.Flag{Name: "--reverse"}); err != nil { + if err := sendCommits(stream.Context(), sender, in.GetRepository(), []string{revisionRange}, nil, nil, git.Flag{Name: "--reverse"}); err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/commit_messages.go b/internal/gitaly/service/commit/commit_messages.go index 86ec95292c6a9d77358db672fa5584d33affbf29..f66ea7b5ed674f19b59f81e3717467bc53da53dd 100644 --- a/internal/gitaly/service/commit/commit_messages.go +++ b/internal/gitaly/service/commit/commit_messages.go @@ -16,16 +16,16 @@ func (s *server) GetCommitMessages(request *gitalypb.GetCommitMessagesRequest, s if err := validateGetCommitMessagesRequest(request); err != nil { return helper.ErrInvalidArgument(err) } - if err := s.getAndStreamCommitMessages(request, stream); err != nil { + if err := getAndStreamCommitMessages(request, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error { +func getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, request.GetRepository()) + c, err := catfile.New(ctx, request.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go index f78f03f53fcbd4c738554308f1f3ff544c40816c..0601cdeb95c2d3e8d32f3bd38bc2bec05dff3d32 100644 --- a/internal/gitaly/service/commit/commit_signatures.go +++ b/internal/gitaly/service/commit/commit_signatures.go @@ -23,13 +23,13 @@ func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesReques return status.Errorf(codes.InvalidArgument, "GetCommitSignatures: %v", err) } - return s.getCommitSignatures(request, stream) + return getCommitSignatures(s, request, stream) } -func (s *server) getCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { +func getCommitSignatures(s *server, request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, request.GetRepository()) + c, err := catfile.New(ctx, request.GetRepository()) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/commits_by_message.go b/internal/gitaly/service/commit/commits_by_message.go index 5ce5b1e02efb3e330751c7d0adcbfb12734cec0e..9a2388202f7a997438b7823c8e9a5a786f63cfec 100644 --- a/internal/gitaly/service/commit/commits_by_message.go +++ b/internal/gitaly/service/commit/commits_by_message.go @@ -6,7 +6,6 @@ import ( "github.com/golang/protobuf/proto" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -29,14 +28,14 @@ func (s *server) CommitsByMessage(in *gitalypb.CommitsByMessageRequest, stream g return helper.ErrInvalidArgument(err) } - if err := commitsByMessage(s.locator, in, stream); err != nil { + if err := commitsByMessage(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func commitsByMessage(locator storage.Locator, in *gitalypb.CommitsByMessageRequest, stream gitalypb.CommitService_CommitsByMessageServer) error { +func commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream gitalypb.CommitService_CommitsByMessageServer) error { ctx := stream.Context() sender := &commitsByMessageSender{stream: stream} @@ -66,7 +65,7 @@ func commitsByMessage(locator storage.Locator, in *gitalypb.CommitsByMessageRequ paths = append(paths, string(path)) } - return sendCommits(stream.Context(), sender, locator, in.GetRepository(), []string{string(revision)}, paths, in.GetGlobalOptions(), gitLogExtraOptions...) + return sendCommits(stream.Context(), sender, in.GetRepository(), []string{string(revision)}, paths, in.GetGlobalOptions(), gitLogExtraOptions...) } func validateCommitsByMessageRequest(in *gitalypb.CommitsByMessageRequest) error { diff --git a/internal/gitaly/service/commit/commits_helper.go b/internal/gitaly/service/commit/commits_helper.go index 1d92a9e11332298ad46ce120c4a5743611addbd4..ba486b880ae57d0500700e31ca91e41946e90bbb 100644 --- a/internal/gitaly/service/commit/commits_helper.go +++ b/internal/gitaly/service/commit/commits_helper.go @@ -7,17 +7,16 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) -func sendCommits(ctx context.Context, sender chunk.Sender, locator storage.Locator, repo *gitalypb.Repository, revisionRange []string, paths []string, options *gitalypb.GlobalOptions, extraArgs ...git.Option) error { +func sendCommits(ctx context.Context, sender chunk.Sender, repo *gitalypb.Repository, revisionRange []string, paths []string, options *gitalypb.GlobalOptions, extraArgs ...git.Option) error { cmd, err := log.GitLogCommand(ctx, repo, revisionRange, paths, options, extraArgs...) if err != nil { return err } - logParser, err := log.NewLogParser(ctx, locator, repo, cmd) + logParser, err := log.NewLogParser(ctx, repo, cmd) if err != nil { return err } diff --git a/internal/gitaly/service/commit/filter_shas_with_signatures.go b/internal/gitaly/service/commit/filter_shas_with_signatures.go index 3152e52a045c69f9b97606d0f0c5a6bd606f6d88..92f2aae5d6e85fa92ce853e20510118a33f42526 100644 --- a/internal/gitaly/service/commit/filter_shas_with_signatures.go +++ b/internal/gitaly/service/commit/filter_shas_with_signatures.go @@ -36,7 +36,7 @@ func validateFirstFilterShasWithSignaturesRequest(in *gitalypb.FilterShasWithSig func (s *server) filterShasWithSignatures(bidi gitalypb.CommitService_FilterShasWithSignaturesServer, firstRequest *gitalypb.FilterShasWithSignaturesRequest) error { ctx := bidi.Context() - c, err := catfile.New(ctx, s.locator, firstRequest.GetRepository()) + c, err := catfile.New(ctx, firstRequest.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/commit/find_all_commits.go b/internal/gitaly/service/commit/find_all_commits.go index 40878218e00e0b4d69353081f5c3e4b826e735cf..5e02cad77330e5c08136c56b3b463eb89a294c57 100644 --- a/internal/gitaly/service/commit/find_all_commits.go +++ b/internal/gitaly/service/commit/find_all_commits.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ref" "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -47,7 +46,7 @@ func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gital revisions = []string{string(in.GetRevision())} } - if err := findAllCommits(s.locator, in, stream, revisions); err != nil { + if err := findAllCommits(in, stream, revisions); err != nil { return helper.ErrInternal(err) } @@ -62,7 +61,7 @@ func validateFindAllCommitsRequest(in *gitalypb.FindAllCommitsRequest) error { return nil } -func findAllCommits(locator storage.Locator, in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer, revisions []string) error { +func findAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer, revisions []string) error { sender := &findAllCommitsSender{stream: stream} var gitLogExtraOptions []git.Option @@ -81,5 +80,5 @@ func findAllCommits(locator storage.Locator, in *gitalypb.FindAllCommitsRequest, gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{Name: "--topo-order"}) } - return sendCommits(stream.Context(), sender, locator, in.GetRepository(), revisions, nil, nil, gitLogExtraOptions...) + return sendCommits(stream.Context(), sender, in.GetRepository(), revisions, nil, nil, gitLogExtraOptions...) } diff --git a/internal/gitaly/service/commit/find_commit.go b/internal/gitaly/service/commit/find_commit.go index 9e149d3c609d39cccb0ac7ef34131e3960bebb17..ca2cf7ca137fbd08d5985c151e7d7015bb087c0e 100644 --- a/internal/gitaly/service/commit/find_commit.go +++ b/internal/gitaly/service/commit/find_commit.go @@ -17,7 +17,7 @@ func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest) repo := in.GetRepository() - commit, err := log.GetCommit(ctx, s.locator, repo, string(revision)) + commit, err := log.GetCommit(ctx, repo, string(revision)) if log.IsNotFound(err) { return &gitalypb.FindCommitResponse{}, nil } diff --git a/internal/gitaly/service/commit/find_commit_test.go b/internal/gitaly/service/commit/find_commit_test.go index 166a378d2a1d1d18bd1a41335a7df3d9ad571ff2..82f23e042dd00ec279d5ab1cfbe1b3a11b7c0ee4 100644 --- a/internal/gitaly/service/commit/find_commit_test.go +++ b/internal/gitaly/service/commit/find_commit_test.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -36,14 +35,12 @@ func TestSuccessfulFindCommitRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - bigMessage := "An empty commit with REALLY BIG message\n\n" + strings.Repeat("MOAR!\n", 20*1024) bigCommitID := testhelper.CreateCommit(t, testRepoPath, "local-big-commits", &testhelper.CreateCommitOpts{ Message: bigMessage, ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, locator, testRepo, bigCommitID) + bigCommit, err := log.GetCommit(ctx, testRepo, bigCommitID) require.NoError(t, err) testCases := []struct { diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go index ca95a1429d41b053d6d325c2fbc9374ba64e26c2..9f074adea9fef2c256325c977e35c3ebc3c71a47 100644 --- a/internal/gitaly/service/commit/find_commits.go +++ b/internal/gitaly/service/commit/find_commits.go @@ -40,21 +40,21 @@ func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.C } } - if err := s.findCommits(ctx, req, stream); err != nil { + if err := findCommits(ctx, req, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) findCommits(ctx context.Context, req *gitalypb.FindCommitsRequest, stream gitalypb.CommitService_FindCommitsServer) error { +func findCommits(ctx context.Context, req *gitalypb.FindCommitsRequest, stream gitalypb.CommitService_FindCommitsServer) error { globals := git.ConvertGlobalOptions(req.GetGlobalOptions()) logCmd, err := git.SafeCmd(ctx, req.GetRepository(), globals, getLogCommandSubCmd(req)) if err != nil { return fmt.Errorf("error when creating git log command: %v", err) } - batch, err := catfile.New(ctx, s.locator, req.GetRepository()) + batch, err := catfile.New(ctx, req.GetRepository()) if err != nil { return fmt.Errorf("creating catfile: %v", err) } diff --git a/internal/gitaly/service/commit/last_commit_for_path.go b/internal/gitaly/service/commit/last_commit_for_path.go index b13c9fed6c82b47365857d99d5e72436887addbb..ff54582d73a53cefa688ae11dbbf4a2dc72f2f5b 100644 --- a/internal/gitaly/service/commit/last_commit_for_path.go +++ b/internal/gitaly/service/commit/last_commit_for_path.go @@ -15,7 +15,7 @@ func (s *server) LastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF return nil, helper.ErrInvalidArgument(err) } - resp, err := s.lastCommitForPath(ctx, in) + resp, err := lastCommitForPath(ctx, in) if err != nil { return nil, helper.ErrInternal(err) } @@ -23,14 +23,14 @@ func (s *server) LastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF return resp, nil } -func (s *server) lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitForPathRequest) (*gitalypb.LastCommitForPathResponse, error) { +func lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitForPathRequest) (*gitalypb.LastCommitForPathResponse, error) { path := string(in.GetPath()) if len(path) == 0 || path == "/" { path = "." } repo := in.GetRepository() - c, err := catfile.New(ctx, s.locator, repo) + c, err := catfile.New(ctx, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/commit/list_commits_by_oid.go b/internal/gitaly/service/commit/list_commits_by_oid.go index 27c1fb84b447f0105bb8180beb5ec3fc0bc6cbde..24875761a96b287b9ad181e68a0a9dffee0b17bc 100644 --- a/internal/gitaly/service/commit/list_commits_by_oid.go +++ b/internal/gitaly/service/commit/list_commits_by_oid.go @@ -29,7 +29,7 @@ func init() { func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream gitalypb.CommitService_ListCommitsByOidServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, in.Repository) + c, err := catfile.New(ctx, in.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/commit/list_commits_by_ref_name.go b/internal/gitaly/service/commit/list_commits_by_ref_name.go index 62bbd2ea4917a7662456b5b1b29da2df2ccc36bc..f0573dcf637ea1443bb1dd0e6dc3b1fa103ca26e 100644 --- a/internal/gitaly/service/commit/list_commits_by_ref_name.go +++ b/internal/gitaly/service/commit/list_commits_by_ref_name.go @@ -12,7 +12,7 @@ import ( func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest, stream gitalypb.CommitService_ListCommitsByRefNameServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, in.Repository) + c, err := catfile.New(ctx, in.Repository) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/list_last_commits_for_tree.go b/internal/gitaly/service/commit/list_last_commits_for_tree.go index c7cac99f21ff40d98a71c0ce32fde2d08cc8a324..8ca02c3d3a5e9c01651c3390bc7b249e0da7b09d 100644 --- a/internal/gitaly/service/commit/list_last_commits_for_tree.go +++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go @@ -30,14 +30,14 @@ func (s *server) ListLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeReque return helper.ErrInvalidArgument(err) } - if err := s.listLastCommitsForTree(in, stream); err != nil { + if err := listLastCommitsForTree(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error { +func listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error { cmd, parser, err := newLSTreeParser(in, stream) if err != nil { return err @@ -45,7 +45,7 @@ func (s *server) listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeReque ctx := stream.Context() repo := in.GetRepository() - c, err := catfile.New(ctx, s.locator, repo) + c, err := catfile.New(ctx, repo) if err != nil { return err } diff --git a/internal/gitaly/service/commit/stats.go b/internal/gitaly/service/commit/stats.go index 2102b757d4a645fcdce1d57f0ba1e60b7ed8daf4..1eb7efd0a139c5a9c61536f70cabfabe40b51612 100644 --- a/internal/gitaly/service/commit/stats.go +++ b/internal/gitaly/service/commit/stats.go @@ -18,7 +18,7 @@ func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsReques return nil, helper.ErrInvalidArgument(err) } - resp, err := s.commitStats(ctx, in) + resp, err := commitStats(ctx, in) if err != nil { return nil, helper.ErrInternal(err) } @@ -26,8 +26,8 @@ func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsReques return resp, nil } -func (s *server) commitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { - commit, err := log.GetCommit(ctx, s.locator, in.Repository, string(in.Revision)) +func commitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { + commit, err := log.GetCommit(ctx, in.Repository, string(in.Revision)) if err != nil { return nil, err } diff --git a/internal/gitaly/service/commit/tree_entries.go b/internal/gitaly/service/commit/tree_entries.go index 7d5c3c2f91fda709796deda3a27ca42e7b60c8bd..4e873627ac0c7e36a0d4b7b5e221eab2d371f15c 100644 --- a/internal/gitaly/service/commit/tree_entries.go +++ b/internal/gitaly/service/commit/tree_entries.go @@ -99,7 +99,7 @@ func (s *server) GetTreeEntries(in *gitalypb.GetTreeEntriesRequest, stream gital return status.Errorf(codes.InvalidArgument, "TreeEntry: %v", err) } - c, err := catfile.New(stream.Context(), s.locator, in.Repository) + c, err := catfile.New(stream.Context(), in.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go index 00663bfa37c134d2d3d03cf03316a8919e9a43ba..b11fdd658f4d5e5a9db1ce59bb160f54763fa99a 100644 --- a/internal/gitaly/service/commit/tree_entry.go +++ b/internal/gitaly/service/commit/tree_entry.go @@ -125,7 +125,7 @@ func (s *server) TreeEntry(in *gitalypb.TreeEntryRequest, stream gitalypb.Commit requestPath = strings.TrimRight(requestPath, "/") } - c, err := catfile.New(stream.Context(), s.locator, in.Repository) + c, err := catfile.New(stream.Context(), in.Repository) if err != nil { return err diff --git a/internal/gitaly/service/conflicts/resolve_conflicts_test.go b/internal/gitaly/service/conflicts/resolve_conflicts_test.go index 508e667bb45b3973e5b78846bd35a841235c2a19..1e334be30dc1e97a4b0fe11aafb6922bffba5e56 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts_test.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts_test.go @@ -37,8 +37,6 @@ func TestSuccessfulResolveConflictsRequest(t *testing.T) { } func testSuccessfulResolveConflictsRequest(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, clean := runFullServer(t) defer clean() @@ -112,7 +110,7 @@ func testSuccessfulResolveConflictsRequest(t *testing.T, ctx context.Context) { require.NoError(t, err) require.Empty(t, r.GetResolutionError()) - headCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) + headCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) require.NoError(t, err) require.Contains(t, headCommit.ParentIds, "1450cd639e0bc6721eb02800169e464f212cde06") require.Contains(t, headCommit.ParentIds, "824be604a34828eb682305f0d963056cfac87b2d") diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index b941fe2128075a66e9940cee72d4586e8bc99f97..0bd5b9e3d78c555dbfb420be2747d60bdd0c2292 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -82,7 +82,7 @@ func TestLink(t *testing.T) { require.NoError(t, err, "error from LinkRepositoryToObjectPool") - commit, err := log.GetCommit(ctx, locator, testRepo, poolCommitID) + commit, err := log.GetCommit(ctx, testRepo, poolCommitID) require.NoError(t, err) require.NotNil(t, commit) require.Equal(t, poolCommitID, commit.Id) diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index f7225ee59e5b098d8014ab5fd0c75d21f15a215a..1b24b23915133606ced3741f9f4e8a62ab839527 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -125,8 +124,7 @@ func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) { } for index, sha := range shas { - locator := config.NewLocator(config.Config) - commit, err := log.GetCommit(ctx, locator, testRepo, sha) + commit, err := log.GetCommit(ctx, testRepo, sha) require.NoError(t, err) require.NotNil(t, commit) diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index 61a43740eca4534dfeee9d99619bdfb01d223f41..7da311fc5ae56a7c69c701d887da532170a3d479 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -40,7 +40,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB // // startPointReference, err := git.NewRepository(req.Repository).GetReference(ctx, "refs/heads/"+string(req.StartPoint)) // startPointCommit, err := log.GetCommit(ctx, req.Repository, startPointReference.Target) - startPointCommit, err := log.GetCommit(ctx, s.locator, req.Repository, string(req.StartPoint)) + startPointCommit, err := log.GetCommit(ctx, req.Repository, string(req.StartPoint)) // END TODO if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", req.StartPoint) diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go index c29f2a4c5e6cd7df11396ed70c35f60aa3d4752f..f7e279d5ef7f8eb85b0853995c5c84869a9efbf8 100644 --- a/internal/gitaly/service/operations/branches_test.go +++ b/internal/gitaly/service/operations/branches_test.go @@ -48,9 +48,8 @@ func testSuccessfulCreateBranchRequest(t *testing.T, ctx context.Context) { client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() - locator := config.NewLocator(config.Config) startPoint := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" - startPointCommit, err := log.GetCommit(ctx, locator, testRepo, startPoint) + startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint) require.NoError(t, err) testCases := []struct { @@ -321,8 +320,7 @@ func testSuccessfulCreateBranchRequestWithStartPointRefPrefix(t *testing.T, ctx // //targetCommitOK, err := log.GetCommit(ctx, testRepo, testCase.startPointCommit) // END TODO - locator := config.NewLocator(config.Config) - targetCommitOK, err := log.GetCommit(ctx, locator, testRepo, "1e292f8fedd741b75372e19097c76d327140c312") + targetCommitOK, err := log.GetCommit(ctx, testRepo, "1e292f8fedd741b75372e19097c76d327140c312") require.NoError(t, err) response, err := client.UserCreateBranch(ctx, request) diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go index 81b322e74d24805bdeda4569e03f352d6127f457..1af9c597480af116f8f35190c9377caa24c5d2d2 100644 --- a/internal/gitaly/service/operations/cherry_pick_test.go +++ b/internal/gitaly/service/operations/cherry_pick_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -17,8 +16,6 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -31,10 +28,10 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - masterHeadCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) testRepoCopy, testRepoCopyPath, cleanup := testhelper.NewTestRepo(t) // read-only repo @@ -157,7 +154,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { response, err := client.UserCherryPick(ctx, testCase.request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, locator, testCase.request.Repository, string(testCase.request.BranchName)) + headCommit, err := log.GetCommit(ctx, testCase.request.Repository, string(testCase.request.BranchName)) require.NoError(t, err) expectedBranchUpdate := testCase.branchUpdate @@ -186,8 +183,6 @@ func TestSuccessfulGitHooksForUserCherryPickRequest(t *testing.T) { } func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -200,7 +195,7 @@ func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter conte destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -235,8 +230,6 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -246,7 +239,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) destinationBranch := "cherry-picking-dst" @@ -317,8 +310,6 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -331,7 +322,7 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -364,8 +355,6 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -379,7 +368,7 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") // This commit already exists in master - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e") + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -403,8 +392,6 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -419,7 +406,7 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") - cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) + cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index de5d815ffd4e54add8cf6b24e0c1e3d27e8e3bf3..32433e0bb4c0768331099b97bc87a35d5d58514d 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -842,8 +841,6 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -923,7 +920,7 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) { require.Equal(t, tc.repoCreated, resp.GetBranchUpdate().GetRepoCreated()) require.Equal(t, tc.branchCreated, resp.GetBranchUpdate().GetBranchCreated()) - headCommit, err := log.GetCommit(ctx, locator, tc.repo, tc.branchName) + headCommit, err := log.GetCommit(ctx, tc.repo, tc.branchName) require.NoError(t, err) require.Equal(t, authorName, headCommit.Author.Name) require.Equal(t, testhelper.TestUser.Name, headCommit.Committer.Name) @@ -1011,8 +1008,6 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { } func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1027,10 +1022,10 @@ func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.C targetBranchName := "feature" startBranchName := []byte("master") - startBranchCommit, err := log.GetCommit(ctx, locator, testRepo, string(startBranchName)) + startBranchCommit, err := log.GetCommit(ctx, testRepo, string(startBranchName)) require.NoError(t, err) - targetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) + targetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) require.NoError(t, err) mergeBaseOut := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "merge-base", targetBranchCommit.Id, startBranchCommit.Id) @@ -1052,7 +1047,7 @@ func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.C require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1064,8 +1059,6 @@ func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) { } func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1077,7 +1070,7 @@ func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Cont targetBranchName := "new" - startCommit, err := log.GetCommit(ctx, locator, testRepo, "master") + startCommit, err := log.GetCommit(ctx, testRepo, "master") require.NoError(t, err) headerRequest := headerRequest(testRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) @@ -1093,7 +1086,7 @@ func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Cont require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1116,8 +1109,6 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header // Regular table driven test did not work here as there is some state shared in the helpers between the subtests. // Running them in different top level tests works, so we use a parameterized function instead to share the code. return func(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1138,7 +1129,7 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header targetBranchName := "new" - startCommit, err := log.GetCommit(ctx, locator, testRepo, "master") + startCommit, err := log.GetCommit(ctx, testRepo, "master") require.NoError(t, err) headerRequest := headerRequest(newRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) @@ -1155,7 +1146,7 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, locator, newRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, newRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1168,8 +1159,6 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes } func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1210,7 +1199,7 @@ func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes _, err = stream.CloseAndRecv() require.NoError(t, err) - newCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) + newCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, tc.author.Name, newCommit.Author.Name, "author name") diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index 3a9ac8ba0d53dd2fc37c3a14b6f68517d96ba9ec..3f4e1c7351ac58d2252159b9af2ce065446ec3f2 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -46,8 +46,6 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -89,7 +87,7 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { firstResponse, err := mergeBidi.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.CommitId) + _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.CommitId) require.NoError(t, err, "look up git commit before merge is applied") require.NoError(t, mergeBidi.Send(&gitalypb.UserMergeBranchRequest{Apply: true}), "apply merge") @@ -103,7 +101,7 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { }) require.NoError(t, err, "consume EOF") - commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) require.NoError(t, err, "look up git commit after call has finished") require.Equal(t, gitalypb.OperationBranchUpdate{CommitId: commit.Id}, *(secondResponse.BranchUpdate)) @@ -136,8 +134,6 @@ func TestAbortedMerge(t *testing.T) { } func testAbortedMerge(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -194,7 +190,7 @@ func testAbortedMerge(t *testing.T, ctx context.Context) { require.Equal(t, "", secondResponse.GetBranchUpdate().GetCommitId(), "merge should not have been applied") require.Error(t, err) - commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) require.NoError(t, err, "look up git commit after call has finished") require.Equal(t, mergeBranchHeadBefore, commit.Id, "branch should not change when the merge is aborted") @@ -210,8 +206,6 @@ func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -247,7 +241,7 @@ func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) { require.NoError(t, err, "receive second response") testhelper.ProtoEqual(t, secondResponse, &gitalypb.UserMergeBranchResponse{}) - commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) require.NoError(t, err, "get commit after RPC finished") require.Equal(t, commit.Id, concurrentCommitID, "RPC should not have trampled concurrent update") } @@ -585,8 +579,6 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { ctx, cleanup := testhelper.Context() defer cleanup() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -658,7 +650,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { FirstParentRef: testCase.firstParentRef, } - commitBeforeRefMerge, fetchRefBeforeMergeErr := gitlog.GetCommit(ctx, locator, testRepo, string(testCase.targetRef)) + commitBeforeRefMerge, fetchRefBeforeMergeErr := gitlog.GetCommit(ctx, testRepo, string(testCase.targetRef)) if testCase.emptyRef { require.Error(t, fetchRefBeforeMergeErr, "error when fetching empty ref commit") } else { @@ -668,7 +660,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { resp, err := client.UserMergeToRef(ctx, request) require.NoError(t, err) - commit, err := gitlog.GetCommit(ctx, locator, testRepo, string(testCase.targetRef)) + commit, err := gitlog.GetCommit(ctx, testRepo, string(testCase.targetRef)) require.NoError(t, err, "look up git commit after call has finished") // Asserts commit parent SHAs diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index 8aeaf51b55a670d23440fa0e9c3e58d1bf077d46..1741968e60f7a040a63eab19ad8ee6f217530aa2 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -33,8 +32,6 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { require.NoError(t, ruby.Start()) defer ruby.Stop() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServerWithRubyServer(t, &ruby) defer stop() @@ -76,7 +73,7 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -248,8 +245,6 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -276,7 +271,7 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(false) @@ -296,8 +291,6 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -332,7 +325,7 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -407,8 +400,6 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -443,7 +434,7 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -467,8 +458,6 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { } func TestRebaseOntoRemoteBranch(t *testing.T) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -501,7 +490,7 @@ func TestRebaseOntoRemoteBranch(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - _, err = gitlog.GetCommit(ctx, locator, localRepo, remoteBranchHash) + _, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash) require.True(t, catfile.IsNotFound(err), "remote commit does not yet exist in local repository") headerRequest := buildHeaderRequest(localRepo, testhelper.TestUser, "1", localBranch, localBranchHash, remoteRepo, remoteBranch) @@ -510,7 +499,7 @@ func TestRebaseOntoRemoteBranch(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, locator, localRepo, remoteBranchHash) + _, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash) require.NoError(t, err, "remote commit does now exist in local repository") applyRequest := buildApplyRequest(true) diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go index c674c28a5d96072d36876c40d90747c89b3963e5..4786eaf9973ccd530af9859e032b3e71f7e050d9 100644 --- a/internal/gitaly/service/operations/revert_test.go +++ b/internal/gitaly/service/operations/revert_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -18,8 +17,6 @@ func TestServer_UserRevert_successful(t *testing.T) { } func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -32,10 +29,10 @@ func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - masterHeadCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) testRepoCopy, testRepoCopyPath, cleanup := testhelper.NewTestRepo(t) // read-only repo @@ -158,7 +155,7 @@ func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { response, err := client.UserRevert(ctx, testCase.request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, locator, testCase.request.Repository, string(testCase.request.BranchName)) + headCommit, err := log.GetCommit(ctx, testCase.request.Repository, string(testCase.request.BranchName)) require.NoError(t, err) expectedBranchUpdate := testCase.branchUpdate @@ -184,8 +181,6 @@ func TestServer_UserRevert_successful_into_empty_repo(t *testing.T) { } func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -195,10 +190,10 @@ func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Co startRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - revertedCommit, err := log.GetCommit(ctxOuter, locator, startRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, startRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) - masterHeadCommit, err := log.GetCommit(ctxOuter, locator, startRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, startRepo, "master") require.NoError(t, err) testRepo, _, cleanup := testhelper.InitBareRepo(t) @@ -220,7 +215,7 @@ func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Co response, err := client.UserRevert(ctx, request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, locator, testRepo, string(request.BranchName)) + headCommit, err := log.GetCommit(ctx, testRepo, string(request.BranchName)) require.NoError(t, err) expectedBranchUpdate := &gitalypb.OperationBranchUpdate{ @@ -241,8 +236,6 @@ func TestServer_UserRevert_successful_git_hooks(t *testing.T) { } func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -255,7 +248,7 @@ func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctxOuter context.Conte destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -291,8 +284,6 @@ func TestServer_UserRevert_failued_due_to_validations(t *testing.T) { } func testServerUserRevertFailuedDueToValidations(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -302,7 +293,7 @@ func testServerUserRevertFailuedDueToValidations(t *testing.T, ctxOuter context. testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) destinationBranch := "revert-dst" @@ -374,8 +365,6 @@ func TestServer_UserRevert_failed_due_to_pre_receive_error(t *testing.T) { } func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -388,7 +377,7 @@ func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctxOuter conte destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -422,8 +411,6 @@ func TestServer_UserRevert_failed_due_to_create_tree_error(t *testing.T) { } func testServerUserRevertFailedDueToCreateTreeError(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -437,7 +424,7 @@ func testServerUserRevertFailedDueToCreateTreeError(t *testing.T, ctxOuter conte testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") // This revert patch of the following commit cannot be applied to the destinationBranch above - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb") + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -462,8 +449,6 @@ func TestServer_UserRevert_failed_due_to_commit_error(t *testing.T) { } func testServerUserRevertFailedDueToCommitError(t *testing.T, ctxOuter context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -478,7 +463,7 @@ func testServerUserRevertFailedDueToCommitError(t *testing.T, ctxOuter context.C testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "a5391128b0ef5d21df5dd23d98557f4ef12fae20") - revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) + revertedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserRevertRequest{ diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go index f890a45100a9f9ebca80fcb412ebffe2539c7e76..58d3a11ad438db4e207f0aede20ce9764542218f 100644 --- a/internal/gitaly/service/operations/squash_test.go +++ b/internal/gitaly/service/operations/squash_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -47,8 +46,6 @@ func TestSuccessfulUserSquashRequest(t *testing.T) { } func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context, start, end string) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -72,7 +69,7 @@ func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context, start, e require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{start}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) @@ -103,8 +100,6 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -129,7 +124,7 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{"6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) @@ -188,8 +183,6 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -238,7 +231,7 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{startCommitID}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go index 51d060d4ca562c3e3d80dbe30603033b2d6096c2..779d9ca4f8727b9516a744be0caeacebfa3a28ef 100644 --- a/internal/gitaly/service/operations/submodules_test.go +++ b/internal/gitaly/service/operations/submodules_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/git/lstree" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -23,8 +22,6 @@ func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) { } func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -72,7 +69,7 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) require.Empty(t, response.GetCommitError()) require.Empty(t, response.GetPreReceiveError()) - commit, err := log.GetCommit(ctx, locator, testRepo, response.BranchUpdate.CommitId) + commit, err := log.GetCommit(ctx, testRepo, response.BranchUpdate.CommitId) require.NoError(t, err) require.Equal(t, commit.Author.Email, testhelper.TestUser.Email) require.Equal(t, commit.Committer.Email, testhelper.TestUser.Email) diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index ccd03e157a88d018f69fcb88e606877998475797..93596dfca8528f783e7fba530e3d06661f08b182 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -158,8 +158,6 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { } func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -170,7 +168,7 @@ func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { defer cleanupFn() targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" - targetRevisionCommit, err := log.GetCommit(ctx, locator, testRepo, targetRevision) + targetRevisionCommit, err := log.GetCommit(ctx, testRepo, targetRevision) require.NoError(t, err) inputTagName := "to-be-créated-soon" @@ -383,8 +381,6 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -467,7 +463,7 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) { // Fake it up for all levels, except for ^{} == "commit" responseOk.Tag.TargetCommit = response.Tag.TargetCommit if testCase.targetObjectType == "commit" { - responseOk.Tag.TargetCommit, err = log.GetCommit(ctx, locator, testRepo, testCase.targetObject) + responseOk.Tag.TargetCommit, err = log.GetCommit(ctx, testRepo, testCase.targetObject) require.NoError(t, err) } require.Equal(t, responseOk, response) @@ -545,8 +541,6 @@ func TestUserCreateTagsuccessfulCreationOfPrefixedTag(t *testing.T) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -585,7 +579,7 @@ func TestUserCreateTagsuccessfulCreationOfPrefixedTag(t *testing.T) { response, err := client.UserCreateTag(ctx, request) require.Equal(t, testCase.err, err) - commitOk, err := log.GetCommit(ctx, locator, testRepo, testCase.tagTargetRevisionInput) + commitOk, err := log.GetCommit(ctx, testRepo, testCase.tagTargetRevisionInput) require.NoError(t, err) responseOk := &gitalypb.UserCreateTagResponse{ diff --git a/internal/gitaly/service/operations/update_branches_test.go b/internal/gitaly/service/operations/update_branches_test.go index cd55343c6343f15e82384a84bee0fbb067e3e340..0af47e90e13187a05b110f817d46f3be53b1ec32 100644 --- a/internal/gitaly/service/operations/update_branches_test.go +++ b/internal/gitaly/service/operations/update_branches_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -31,8 +30,6 @@ func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - locator := config.NewLocator(config.Config) - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -52,7 +49,7 @@ func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) { require.NoError(t, err) require.Empty(t, response.PreReceiveError) - branchCommit, err := log.GetCommit(ctx, locator, testRepo, updateBranchName) + branchCommit, err := log.GetCommit(ctx, testRepo, updateBranchName) require.NoError(t, err) require.Equal(t, string(newrev), branchCommit.Id) diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index d92489f5d84ab3909fc6efbc06561d1de2a25d0e..67dcaa6a59fa36db45b15d43783d2887ca1f2b16 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -34,7 +34,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest return nil, err } - commit, err := log.GetCommit(ctx, s.locator, repo, branch.Target) + commit, err := log.GetCommit(ctx, repo, branch.Target) if err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/branches_test.go b/internal/gitaly/service/ref/branches_test.go index 2c1557dac75cef5eb4913be7a04b436716dbbf5f..0dbde77a8945885c564ddf9803e39c06bc589153 100644 --- a/internal/gitaly/service/ref/branches_test.go +++ b/internal/gitaly/service/ref/branches_test.go @@ -5,7 +5,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -15,7 +14,6 @@ func TestSuccessfulFindBranchRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -26,7 +24,7 @@ func TestSuccessfulFindBranchRequest(t *testing.T) { defer cleanupFn() branchNameInput := "master" - branchTarget, err := log.GetCommit(ctx, locator, testRepo, branchNameInput) + branchTarget, err := log.GetCommit(ctx, testRepo, branchNameInput) require.NoError(t, err) branch := &gitalypb.Branch{ diff --git a/internal/gitaly/service/ref/list_new_blobs.go b/internal/gitaly/service/ref/list_new_blobs.go index d23019c84a8427a6ed2056221f18b19278434f96..f942eee529ad829f714b15c1acc4b3f59406a466 100644 --- a/internal/gitaly/service/ref/list_new_blobs.go +++ b/internal/gitaly/service/ref/list_new_blobs.go @@ -17,14 +17,14 @@ func (s *server) ListNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb. return helper.ErrInvalidArgument(err) } - if err := s.listNewBlobs(in, stream, oid); err != nil { + if err := listNewBlobs(in, stream, oid); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.RefService_ListNewBlobsServer, oid string) error { +func listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.RefService_ListNewBlobsServer, oid string) error { ctx := stream.Context() cmdFlags := []git.Option{git.Flag{Name: "--objects"}, git.Flag{Name: "--not"}, git.Flag{Name: "--all"}} @@ -38,7 +38,7 @@ func (s *server) listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb. return err } - batch, err := catfile.New(ctx, s.locator, in.GetRepository()) + batch, err := catfile.New(ctx, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/list_new_commits.go b/internal/gitaly/service/ref/list_new_commits.go index 1f12f07e62ed4f4aa3ce22ba55e06bc88508f3f8..e5533e42aa3b1d375e7713c91ee47845727921cb 100644 --- a/internal/gitaly/service/ref/list_new_commits.go +++ b/internal/gitaly/service/ref/list_new_commits.go @@ -16,14 +16,14 @@ func (s *server) ListNewCommits(in *gitalypb.ListNewCommitsRequest, stream gital return helper.ErrInvalidArgument(err) } - if err := s.listNewCommits(in, stream, oid); err != nil { + if err := listNewCommits(in, stream, oid); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefService_ListNewCommitsServer, oid string) error { +func listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefService_ListNewCommitsServer, oid string) error { ctx := stream.Context() revList, err := git.SafeCmd(ctx, in.GetRepository(), nil, git.SubCmd{ @@ -35,7 +35,7 @@ func (s *server) listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gital return err } - batch, err := catfile.New(ctx, s.locator, in.GetRepository()) + batch, err := catfile.New(ctx, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index f122bc8d791bcb09380dc81026b474f779326829..82b7c996496c314ca745502e40c8b8249186e4f3 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -87,7 +87,7 @@ func (t *tagSender) Send() error { }) } -func (s *server) parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream gitalypb.RefService_FindAllTagsServer) error { +func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream gitalypb.RefService_FindAllTagsServer) error { tagsCmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{ Name: "for-each-ref", Flags: []git.Option{ @@ -99,7 +99,7 @@ func (s *server) parseAndReturnTags(ctx context.Context, repo *gitalypb.Reposito return fmt.Errorf("for-each-ref error: %v", err) } - c, err := catfile.New(ctx, s.locator, repo) + c, err := catfile.New(ctx, repo) if err != nil { return fmt.Errorf("error creating catfile: %v", err) } @@ -136,7 +136,7 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re return helper.ErrInvalidArgument(err) } - if err := s.parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { + if err := parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { return helper.ErrInternal(err) } return nil @@ -291,16 +291,16 @@ func parseSortKey(sortKey gitalypb.FindLocalBranchesRequest_SortBy) string { // FindLocalBranches creates a stream of branches for all local branches in the given repository func (s *server) FindLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { - if err := s.findLocalBranches(in, stream); err != nil { + if err := findLocalBranches(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { +func findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, in.Repository) + c, err := catfile.New(ctx, in.Repository) if err != nil { return err } @@ -317,14 +317,14 @@ func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream } func (s *server) FindAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { - if err := s.findAllBranches(in, stream); err != nil { + if err := findAllBranches(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { +func findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { args := []git.Option{ // %00 inserts the null character into the output (see for-each-ref docs) git.Flag{Name: "--format=" + strings.Join(localBranchFormatFields, "%00")}, @@ -350,7 +350,7 @@ func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream git } ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, in.Repository) + c, err := catfile.New(ctx, in.Repository) if err != nil { return err } @@ -371,7 +371,7 @@ func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*git var tag *gitalypb.Tag - if tag, err = s.findTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { + if tag, err = findTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { return nil, helper.ErrInternal(err) } @@ -412,7 +412,7 @@ func parseTagLine(ctx context.Context, c catfile.Batch, tagLine string) (*gitaly } } -func (s *server) findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { +func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { tagCmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{ Name: "tag", @@ -427,7 +427,7 @@ func (s *server) findTag(ctx context.Context, repository *gitalypb.Repository, t return nil, fmt.Errorf("for-each-ref error: %v", err) } - c, err := catfile.New(ctx, s.locator, repository) + c, err := catfile.New(ctx, repository) if err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index fbee01fbf50cf8676907b5c283938bbfb3b9f8a2..0a655e2f9bd41df2a158d411fc9673367c594772 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -471,7 +471,6 @@ func TestInvalidRepoFindDefaultBranchNameRequest(t *testing.T) { } func TestSuccessfulFindAllTagsRequest(t *testing.T) { - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -499,7 +498,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { Message: "An empty commit with REALLY BIG message\n\n" + strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1), ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, locator, testRepoCopy, bigCommitID) + bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID) require.NoError(t, err) annotatedTagID := testhelper.CreateTag(t, testRepoCopyPath, "v1.2.0", blobID, &testhelper.CreateTagOpts{Message: "Blob tag"}) @@ -673,7 +672,6 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { } func TestFindAllTagNestedTags(t *testing.T) { - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -718,7 +716,7 @@ func TestFindAllTagNestedTags(t *testing.T) { tags := bytes.NewReader(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoCopyPath, "tag")) testhelper.MustRunCommand(t, tags, "xargs", config.Config.Git.BinPath, "-C", testRepoCopyPath, "tag", "-d") - batch, err := catfile.New(ctx, locator, testRepoCopy) + batch, err := catfile.New(ctx, testRepoCopy) require.NoError(t, err) info, err := batch.Info(ctx, tc.originalOid) @@ -1110,7 +1108,6 @@ func TestSuccessfulFindAllBranchesRequest(t *testing.T) { } func TestSuccessfulFindAllBranchesRequestWithMergedBranches(t *testing.T) { - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1146,7 +1143,7 @@ func TestSuccessfulFindAllBranchesRequestWithMergedBranches(t *testing.T) { expectedBranches = append(expectedBranches, branch) } - masterCommit, err := log.GetCommit(ctx, locator, testRepo, "master") + masterCommit, err := log.GetCommit(ctx, testRepo, "master") require.NoError(t, err) expectedBranches = append(expectedBranches, &gitalypb.FindAllBranchesResponse_Branch{ Name: []byte("refs/heads/master"), @@ -1433,7 +1430,6 @@ func TestListBranchNamesContainingCommit(t *testing.T) { } func TestSuccessfulFindTagRequest(t *testing.T) { - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1452,7 +1448,7 @@ func TestSuccessfulFindTagRequest(t *testing.T) { Message: "An empty commit with REALLY BIG message\n\n" + strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1), ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, locator, testRepoCopy, bigCommitID) + bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID) require.NoError(t, err) annotatedTagID := testhelper.CreateTag(t, testRepoCopyPath, "v1.2.0", blobID, &testhelper.CreateTagOpts{Message: "Blob tag"}) @@ -1604,7 +1600,6 @@ func TestSuccessfulFindTagRequest(t *testing.T) { } func TestFindTagNestedTag(t *testing.T) { - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1652,7 +1647,7 @@ func TestFindTagNestedTag(t *testing.T) { tags := bytes.NewReader(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoCopyPath, "tag")) testhelper.MustRunCommand(t, tags, "xargs", config.Config.Git.BinPath, "-C", testRepoCopyPath, "tag", "-d") - batch, err := catfile.New(ctx, locator, testRepoCopy) + batch, err := catfile.New(ctx, testRepoCopy) require.NoError(t, err) info, err := batch.Info(ctx, tc.originalOid) diff --git a/internal/gitaly/service/ref/remote_branches.go b/internal/gitaly/service/ref/remote_branches.go index 0aeafd478c36d3debf2456954b98c920b9f41072..f72b4d2969b36b95656ac7169e5510b79d6cdab9 100644 --- a/internal/gitaly/service/ref/remote_branches.go +++ b/internal/gitaly/service/ref/remote_branches.go @@ -15,14 +15,14 @@ func (s *server) FindAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques return helper.ErrInvalidArgument(err) } - if err := s.findAllRemoteBranches(req, stream); err != nil { + if err := findAllRemoteBranches(req, stream); err != nil { return helper.ErrInternal(err) } return nil } -func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gitalypb.RefService_FindAllRemoteBranchesServer) error { +func findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gitalypb.RefService_FindAllRemoteBranchesServer) error { args := []git.Option{ git.Flag{Name: "--format=" + strings.Join(localBranchFormatFields, "%00")}, } @@ -30,7 +30,7 @@ func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques patterns := []string{"refs/remotes/" + req.GetRemoteName()} ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, req.GetRepository()) + c, err := catfile.New(ctx, req.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/remote_branches_test.go b/internal/gitaly/service/ref/remote_branches_test.go index 23650367df6aa5d343011b13bf2f48081bf45eba..a88ca9f6130ddc277d89929100f6da7d34d11e7e 100644 --- a/internal/gitaly/service/ref/remote_branches_test.go +++ b/internal/gitaly/service/ref/remote_branches_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -16,7 +15,6 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -55,7 +53,7 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { require.Len(t, branches, len(expectedBranches)) for branchName, commitID := range expectedBranches { - targetCommit, err := log.GetCommit(ctx, locator, testRepo, commitID) + targetCommit, err := log.GetCommit(ctx, testRepo, commitID) require.NoError(t, err) expectedBranch := &gitalypb.Branch{ @@ -67,7 +65,7 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { } for branchName, commitID := range excludedBranches { - targetCommit, err := log.GetCommit(ctx, locator, testRepo, commitID) + targetCommit, err := log.GetCommit(ctx, testRepo, commitID) require.NoError(t, err) excludedBranch := &gitalypb.Branch{ diff --git a/internal/gitaly/service/ref/tag_messages.go b/internal/gitaly/service/ref/tag_messages.go index 1539b19c39863afc239dd38f2e8a0bb81eb56f4d..d4ec3e07ecd859828c39bce87c1ec605102e8fd6 100644 --- a/internal/gitaly/service/ref/tag_messages.go +++ b/internal/gitaly/service/ref/tag_messages.go @@ -36,7 +36,7 @@ func validateGetTagMessagesRequest(request *gitalypb.GetTagMessagesRequest) erro func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, s.locator, request.GetRepository()) + c, err := catfile.New(ctx, request.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go index 4d55026eeada180b0a10024736e33ea5ef54bd98..65f898fc74b7ea945014636589668138c4664bb7 100644 --- a/internal/gitaly/service/register.go +++ b/internal/gitaly/service/register.go @@ -70,7 +70,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver }) gitalypb.RegisterBlobServiceServer(grpcServer, blob.NewServer(rubyServer, locator)) - gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer(locator)) + gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer()) gitalypb.RegisterCommitServiceServer(grpcServer, commit.NewServer(cfg, locator)) gitalypb.RegisterDiffServiceServer(grpcServer, diff.NewServer(locator)) gitalypb.RegisterNamespaceServiceServer(grpcServer, namespace.NewServer(locator)) diff --git a/internal/gitaly/service/remote/fetch_internal_remote_test.go b/internal/gitaly/service/remote/fetch_internal_remote_test.go index 274047d2746d8c672a0e424ca8e875c276b390df..bd18b512ff1530916bbe2b64d0cbfcff6ed2c35d 100644 --- a/internal/gitaly/service/remote/fetch_internal_remote_test.go +++ b/internal/gitaly/service/remote/fetch_internal_remote_test.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/remote" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ssh" "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -60,12 +59,12 @@ func TestSuccessfulFetchInternalRemote(t *testing.T) { repo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - gitaly0Repo, gitaly0RepoPath, cleanup := cloneRepoAtStorage(t, locator, repo, "gitaly-0") + gitaly0Repo, gitaly0RepoPath, cleanup := cloneRepoAtStorage(t, repo, "gitaly-0") defer cleanup() testhelper.CreateCommit(t, gitaly0RepoPath, "master", nil) - gitaly1Repo, gitaly1RepoPath, cleanup := cloneRepoAtStorage(t, locator, repo, "gitaly-1") + gitaly1Repo, gitaly1RepoPath, cleanup := cloneRepoAtStorage(t, repo, "gitaly-1") defer cleanup() testhelper.MustRunCommand(t, nil, "git", "-C", gitaly1RepoPath, "symbolic-ref", "HEAD", "refs/heads/feature") @@ -188,14 +187,14 @@ func runFullServer(t *testing.T) (string, func()) { } } -func cloneRepoAtStorage(t testing.TB, locator storage.Locator, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { +func cloneRepoAtStorage(t testing.TB, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { dst := *src dst.StorageName = storageName - dstP, err := locator.GetPath(&dst) + dstP, err := helper.GetPath(&dst) require.NoError(t, err) - srcP, err := locator.GetPath(src) + srcP, err := helper.GetPath(src) require.NoError(t, err) require.NoError(t, os.MkdirAll(dstP, 0755)) diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index eef1e868a8442c67bf51abbbddc3ebfb587a35fc..a49b5556c237b6b4cd9e3780c0f5e10ccffaa2c2 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -89,7 +89,7 @@ func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitat return nil, status.Errorf(codes.InvalidArgument, "ApplyGitAttributes: revision: %v", err) } - c, err := catfile.New(ctx, s.locator, repo) + c, err := catfile.New(ctx, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go index 7c620e5a24c09d7ded2febb0982d8c3b6da2d0bf..4cbc96273b0244fdbf1c887bc71cb1a7fa0c0277 100644 --- a/internal/gitaly/service/repository/archive.go +++ b/internal/gitaly/service/repository/archive.go @@ -62,7 +62,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo return err } - if err := s.validateGetArchivePrecondition(ctx, in, path, exclude); err != nil { + if err := validateGetArchivePrecondition(ctx, in, path, exclude); err != nil { return err } @@ -134,8 +134,8 @@ func validateGetArchiveRequest(in *gitalypb.GetArchiveRequest, format string, pa return nil } -func (s *server) validateGetArchivePrecondition(ctx context.Context, in *gitalypb.GetArchiveRequest, path string, exclude []string) error { - c, err := catfile.New(ctx, s.locator, in.GetRepository()) +func validateGetArchivePrecondition(ctx context.Context, in *gitalypb.GetArchiveRequest, path string, exclude []string) error { + c, err := catfile.New(ctx, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/repository/create_from_bundle_test.go b/internal/gitaly/service/repository/create_from_bundle_test.go index 68263b6a04179a7377387a684dcc4c262ec951a7..4ad385ac454049cc3ee46502ea44a257d9168722 100644 --- a/internal/gitaly/service/repository/create_from_bundle_test.go +++ b/internal/gitaly/service/repository/create_from_bundle_test.go @@ -81,7 +81,7 @@ func TestServer_CreateRepositoryFromBundle_successful(t *testing.T) { require.NoError(t, err) require.NotEqual(t, 0, info.Mode()&os.ModeSymlink) - commit, err := log.GetCommit(ctx, locator, importedRepo, "refs/custom-refs/ref1") + commit, err := log.GetCommit(ctx, importedRepo, "refs/custom-refs/ref1") require.NoError(t, err) require.NotNil(t, commit) } diff --git a/internal/gitaly/service/repository/fetch_test.go b/internal/gitaly/service/repository/fetch_test.go index e7176aa0305c72f94d23e943dcf7c76112654611..296172074f328211d688c732713e469ede3d2a7c 100644 --- a/internal/gitaly/service/repository/fetch_test.go +++ b/internal/gitaly/service/repository/fetch_test.go @@ -75,7 +75,7 @@ func TestFetchSourceBranchSourceRepositorySuccess(t *testing.T) { require.NoError(t, err) require.True(t, resp.Result, "response.Result should be true") - fetchedCommit, err := gitLog.GetCommit(ctx, locator, targetRepo, targetRef) + fetchedCommit, err := gitLog.GetCommit(ctx, targetRepo, targetRef) require.NoError(t, err) require.Equal(t, newCommitID, fetchedCommit.GetId()) }) @@ -132,7 +132,7 @@ func TestFetchSourceBranchSameRepositorySuccess(t *testing.T) { require.NoError(t, err) require.True(t, resp.Result, "response.Result should be true") - fetchedCommit, err := gitLog.GetCommit(ctx, locator, repo, targetRef) + fetchedCommit, err := gitLog.GetCommit(ctx, repo, targetRef) require.NoError(t, err) require.Equal(t, newCommitID, fetchedCommit.GetId()) }) diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go index e7e6f7a3b8b8f1833b78e3ef2c87bb863ed6dfa2..28528e9b11ca9dae4345cd93414b7a672334c4b3 100644 --- a/internal/gitaly/service/repository/gc.go +++ b/internal/gitaly/service/repository/gc.go @@ -120,7 +120,7 @@ func (s *server) cleanupKeepArounds(ctx context.Context, repo *gitalypb.Reposito return nil } - batch, err := catfile.New(ctx, s.locator, repo) + batch, err := catfile.New(ctx, repo) if err != nil { return nil } diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index 967ede2a4f618bcec38d16feef50caacaf80d0f9..f4564564f51d70551a6d1090cf56279dfdd4f5fb 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -21,7 +21,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly ctx := stream.Context() repo := req.Repository - batch, err := catfile.New(stream.Context(), s.locator, repo) + batch, err := catfile.New(stream.Context(), repo) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go index 592ebe9d9020fcf81a6aeb1379f85b5bc5b4fd42..3b9bdce4038288fc2417e1707b1e210f27e667fd 100644 --- a/internal/gitaly/service/repository/snapshot_test.go +++ b/internal/gitaly/service/repository/snapshot_test.go @@ -134,14 +134,14 @@ func TestGetSnapshotWithDedupe(t *testing.T) { commitSha := testhelper.CreateCommitInAlternateObjectDirectory(t, repoPath, alternateObjDir, cmd) originalAlternatesCommit := string(commitSha) - locator := config.NewLocator(config.Config) - // ensure commit cannot be found in current repository - c, err := catfile.New(ctx, locator, testRepo) + c, err := catfile.New(ctx, testRepo) require.NoError(t, err) _, err = c.Info(ctx, originalAlternatesCommit) require.True(t, catfile.IsNotFound(err)) + locator := config.NewLocator(config.Config) + // write alternates file to point to alt objects folder alternatesPath, err := locator.InfoAlternatesPath(testRepo) require.NoError(t, err) @@ -154,7 +154,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) { "commit", "--allow-empty", "-m", "Another empty commit") commitSha = testhelper.CreateCommitInAlternateObjectDirectory(t, repoPath, alternateObjDir, cmd) - c, err = catfile.New(ctx, locator, testRepo) + c, err = catfile.New(ctx, testRepo) require.NoError(t, err) _, err = c.Info(ctx, string(commitSha)) require.NoError(t, err) diff --git a/internal/gitaly/service/wiki/delete_page_test.go b/internal/gitaly/service/wiki/delete_page_test.go index 07610988fcc1c79191f053ce13e3ad14980f9400..084812daf24f1e1200cdbd04de622925e3a2022a 100644 --- a/internal/gitaly/service/wiki/delete_page_test.go +++ b/internal/gitaly/service/wiki/delete_page_test.go @@ -73,7 +73,7 @@ func TestSuccessfulWikiDeletePageRequest(t *testing.T) { require.NoError(t, err) headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit after deleting a wiki page") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") diff --git a/internal/gitaly/service/wiki/testhelper_test.go b/internal/gitaly/service/wiki/testhelper_test.go index d469e2674a631ce990bfc181c990995f36c397da..f79dfa578d36ea9e2a16571d91750416cc9160ed 100644 --- a/internal/gitaly/service/wiki/testhelper_test.go +++ b/internal/gitaly/service/wiki/testhelper_test.go @@ -197,7 +197,7 @@ func createTestWikiPage(t *testing.T, locator storage.Locator, client gitalypb.W require.NoError(t, err) writeWikiPage(t, client, wikiRepo, opts) head1ID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - pageCommit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(head1ID)) + pageCommit, err := gitlog.GetCommit(ctx, wikiRepo, string(head1ID)) require.NoError(t, err, "look up git commit after writing a wiki page") return pageCommit diff --git a/internal/gitaly/service/wiki/update_page_test.go b/internal/gitaly/service/wiki/update_page_test.go index 3565b02fd873b5f81f3e827392d7a9e132f82622..88d2ade88dcd3d8c073e58913795cfd34e71a11f 100644 --- a/internal/gitaly/service/wiki/update_page_test.go +++ b/internal/gitaly/service/wiki/update_page_test.go @@ -99,7 +99,7 @@ func TestSuccessfulWikiUpdatePageRequest(t *testing.T) { require.NoError(t, err) headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit before merge is applied") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") diff --git a/internal/gitaly/service/wiki/write_page_test.go b/internal/gitaly/service/wiki/write_page_test.go index 6a6f4c6fd95d25690ff447b43ca29d3ee9305ae2..be0e3579eef202882e0f12ca514047e76fd2b44b 100644 --- a/internal/gitaly/service/wiki/write_page_test.go +++ b/internal/gitaly/service/wiki/write_page_test.go @@ -102,7 +102,7 @@ func TestSuccessfulWikiWritePageRequest(t *testing.T) { require.Empty(t, resp.DuplicateError, "DuplicateError must be empty") headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit after writing a wiki page") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") diff --git a/internal/helper/repo.go b/internal/helper/repo.go index 603c793198f8b2ef68742086e878f50e66109b79..2d61a73eca4c459b1694d4b95e00c57502e8e3cc 100644 --- a/internal/helper/repo.go +++ b/internal/helper/repo.go @@ -1,19 +1,71 @@ package helper import ( + "os" + "path/filepath" + "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) +// GetRepoPath returns the full path of the repository referenced by an +// RPC Repository message. The errors returned are gRPC errors with +// relevant error codes and should be passed back to gRPC without further +// decoration. +// Deprecated: please use storage.Locator to define the project path. +func GetRepoPath(repo repository.GitRepo) (string, error) { + repoPath, err := GetPath(repo) + if err != nil { + return "", err + } + + if repoPath == "" { + return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: empty repo") + } + + if storage.IsGitDirectory(repoPath) { + return repoPath, nil + } + + return "", status.Errorf(codes.NotFound, "GetRepoPath: not a git repository '%s'", repoPath) +} + // RepoPathEqual compares if two repositories are in the same location func RepoPathEqual(a, b repository.GitRepo) bool { return a.GetStorageName() == b.GetStorageName() && a.GetRelativePath() == b.GetRelativePath() } +// GetPath returns the path of the repo passed as first argument. An error is +// returned when either the storage can't be found or the path includes +// constructs trying to perform directory traversal. +func GetPath(repo repository.GitRepo) (string, error) { + storagePath, err := GetStorageByName(repo.GetStorageName()) + if err != nil { + return "", err + } + + if _, err := os.Stat(storagePath); err != nil { + return "", status.Errorf(codes.Internal, "GetPath: storage path: %v", err) + } + + relativePath := repo.GetRelativePath() + if len(relativePath) == 0 { + err := status.Errorf(codes.InvalidArgument, "GetPath: relative path missing from %+v", repo) + return "", err + } + + if _, err := storage.ValidateRelativePath(storagePath, relativePath); err != nil { + return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: %s", err) + } + + return filepath.Join(storagePath, relativePath), nil +} + // GetStorageByName will return the path for the storage, which is fetched by // its key. An error is return if it cannot be found. // Deprecated: please use storage.Locator to define the storage path. diff --git a/internal/helper/repo_test.go b/internal/helper/repo_test.go index 8f4b3d8b694aa41d09b95dca846a2b0c2a4cbc59..ba8648ee3a708e290cd6c2665cd9474ee1a7e7de 100644 --- a/internal/helper/repo_test.go +++ b/internal/helper/repo_test.go @@ -2,11 +2,14 @@ package helper import ( "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" + "google.golang.org/grpc/codes" ) func TestMain(m *testing.M) { @@ -70,3 +73,156 @@ func TestRepoPathEqual(t *testing.T) { }) } } + +func TestGetRepoPath(t *testing.T) { + defer func(oldStorages []config.Storage) { + config.Config.Storages = oldStorages + }(config.Config.Storages) + + testRepo, _, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + repoPath, err := GetRepoPath(testRepo) + if err != nil { + t.Fatal(err) + } + + exampleStorages := []config.Storage{ + {Name: "default", Path: testhelper.GitlabTestStoragePath()}, + {Name: "other", Path: "/home/git/repositories2"}, + {Name: "third", Path: "/home/git/repositories3"}, + } + + testCases := []struct { + desc string + storages []config.Storage + repo *gitalypb.Repository + path string + err codes.Code + }{ + { + desc: "storages configured", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath()}, + path: repoPath, + }, + { + desc: "no storage config, storage name provided", + repo: &gitalypb.Repository{StorageName: "does not exist", RelativePath: testRepo.GetRelativePath()}, + err: codes.InvalidArgument, + }, + { + desc: "no storage config, nil repo", + err: codes.InvalidArgument, + }, + { + desc: "storage config provided, empty repo", + storages: exampleStorages, + repo: &gitalypb.Repository{}, + err: codes.InvalidArgument, + }, + { + desc: "no storage config, empty repo", + repo: &gitalypb.Repository{}, + err: codes.InvalidArgument, + }, + { + desc: "non existing repo", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "made/up/path.git"}, + err: codes.NotFound, + }, + { + desc: "non existing storage", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: "does not exists", RelativePath: testRepo.GetRelativePath()}, + err: codes.InvalidArgument, + }, + { + desc: "storage defined but storage dir does not exist", + storages: []config.Storage{{Name: testRepo.GetStorageName(), Path: "/does/not/exist"}}, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "foobar.git"}, + err: codes.Internal, + }, + { + desc: "relative path with directory traversal", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "../bazqux.git"}, + err: codes.InvalidArgument, + }, + { + desc: "valid path with ..", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "foo../bazqux.git"}, + err: codes.NotFound, // Because the directory doesn't exist + }, + { + desc: "relative path with sneaky directory traversal", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "/../bazqux.git"}, + err: codes.InvalidArgument, + }, + { + desc: "relative path with traversal outside storage", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath() + "/../../../../.."}, + err: codes.InvalidArgument, + }, + { + desc: "relative path with traversal outside storage with trailing slash", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath() + "/../../../../../"}, + err: codes.InvalidArgument, + }, + { + desc: "relative path with deep traversal at the end", + storages: exampleStorages, + repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "bazqux.git/../.."}, + err: codes.InvalidArgument, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + config.Config.Storages = tc.storages + path, err := GetRepoPath(tc.repo) + + if tc.err != codes.OK { + testhelper.RequireGrpcError(t, err, tc.err) + return + } + + if err != nil { + assert.NoError(t, err) + return + } + + assert.Equal(t, tc.path, path) + }) + } +} + +func assertInvalidRepoWithoutFile(t *testing.T, repo *gitalypb.Repository, repoPath, file string) { + oldRoute := filepath.Join(repoPath, file) + renamedRoute := filepath.Join(repoPath, file+"moved") + os.Rename(oldRoute, renamedRoute) + defer func() { + os.Rename(renamedRoute, oldRoute) + }() + + _, err := GetRepoPath(repo) + + testhelper.RequireGrpcError(t, err, codes.NotFound) +} + +func TestGetRepoPathWithCorruptedRepo(t *testing.T) { + testRepo, _, cleanup := testhelper.NewTestRepo(t) + defer cleanup() + + testRepoStoragePath := testhelper.GitlabTestStoragePath() + testRepoPath := filepath.Join(testRepoStoragePath, testRepo.RelativePath) + + for _, file := range []string{"objects", "refs", "HEAD"} { + assertInvalidRepoWithoutFile(t, testRepo, testRepoPath, file) + } +} diff --git a/internal/praefect/info_service_test.go b/internal/praefect/info_service_test.go index f2daeba58cba865ba698736f665aaab46d58a94c..625e6428c3f359302d55898970ce6186ae3921e6 100644 --- a/internal/praefect/info_service_test.go +++ b/internal/praefect/info_service_test.go @@ -50,17 +50,15 @@ func TestInfoService_RepositoryReplicas(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - locator := gconfig.NewLocator(gconfig.Config) - cc, _, cleanup := runPraefectServerWithGitaly(t, gconfig.Config, conf) defer cleanup() - testRepoPrimary, _, cleanup := cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[0].Storage) + testRepoPrimary, _, cleanup := cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[0].Storage) defer cleanup() - _, _, cleanup = cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[1].Storage) + _, _, cleanup = cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[1].Storage) defer cleanup() - _, testRepoSecondary2Path, cleanup := cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[2].Storage) + _, testRepoSecondary2Path, cleanup := cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[2].Storage) defer cleanup() // create a commit in the second replica so we can check that its checksum is different than the primary diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go index 4680a39e770335b4c5b6ba32427fee5b2c3e7ac1..c092b2d8394653e9f04b521842a4f82048c77f9a 100644 --- a/internal/praefect/server_test.go +++ b/internal/praefect/server_test.go @@ -34,7 +34,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/praefect/nodes/tracker" "gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry" "gitlab.com/gitlab-org/gitaly/internal/praefect/transactions" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/promtest" "gitlab.com/gitlab-org/gitaly/internal/version" @@ -390,14 +389,12 @@ func TestRepoRemoval(t *testing.T) { } }() - locator := gconfig.NewLocator(gconfig.Config) - tRepo, _, tCleanup := testhelper.NewTestRepo(t) defer tCleanup() - _, path1, cleanup1 := cloneRepoAtStorage(t, locator, tRepo, conf.VirtualStorages[0].Nodes[1].Storage) + _, path1, cleanup1 := cloneRepoAtStorage(t, tRepo, conf.VirtualStorages[0].Nodes[1].Storage) defer cleanup1() - _, path2, cleanup2 := cloneRepoAtStorage(t, locator, tRepo, conf.VirtualStorages[0].Nodes[2].Storage) + _, path2, cleanup2 := cloneRepoAtStorage(t, tRepo, conf.VirtualStorages[0].Nodes[2].Storage) defer cleanup2() // prerequisite: repos should exist at expected paths @@ -514,16 +511,14 @@ func TestRepoRename(t *testing.T) { require.Len(t, gconfig.Config.Storages, 3, "1 default storage and 2 replicas of it") - locator := gconfig.NewLocator(gconfig.Config) - // repo0 is a template that is used to create replica set by cloning it into other storage (directories) repo0, path0, cleanup0 := testhelper.NewTestRepo(t) defer cleanup0() - _, path1, cleanup1 := cloneRepoAtStorage(t, locator, repo0, virtualStorage.Nodes[1].Storage) + _, path1, cleanup1 := cloneRepoAtStorage(t, repo0, virtualStorage.Nodes[1].Storage) defer cleanup1() - _, path2, cleanup2 := cloneRepoAtStorage(t, locator, repo0, virtualStorage.Nodes[2].Storage) + _, path2, cleanup2 := cloneRepoAtStorage(t, repo0, virtualStorage.Nodes[2].Storage) defer cleanup2() var canCheckRepo sync.WaitGroup @@ -1009,14 +1004,14 @@ func tempStoragePath(t testing.TB) string { return p } -func cloneRepoAtStorage(t testing.TB, locator storage.Locator, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { +func cloneRepoAtStorage(t testing.TB, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { dst := *src dst.StorageName = storageName - dstP, err := locator.GetPath(&dst) + dstP, err := helper.GetPath(&dst) require.NoError(t, err) - srcP, err := locator.GetPath(src) + srcP, err := helper.GetPath(src) require.NoError(t, err) require.NoError(t, os.MkdirAll(dstP, 0755)) diff --git a/internal/tempdir/tempdir_test.go b/internal/tempdir/tempdir_test.go index c53958fded3624d65b8d0a1a572a643eb2f66470..9ee5f4bd4e1a24d2db38ef3008d97ac5d3cae8a2 100644 --- a/internal/tempdir/tempdir_test.go +++ b/internal/tempdir/tempdir_test.go @@ -10,6 +10,7 @@ import ( "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -21,14 +22,13 @@ func TestNewAsRepositorySuccess(t *testing.T) { repo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - locator := config.NewLocator(config.Config) - tempRepo, tempDir, err := NewAsRepository(ctx, repo, locator) + tempRepo, tempDir, err := NewAsRepository(ctx, repo, config.NewLocator(config.Config)) require.NoError(t, err) require.NotEqual(t, repo, tempRepo) require.Equal(t, repo.StorageName, tempRepo.StorageName) require.NotEqual(t, repo.RelativePath, tempRepo.RelativePath) - calculatedPath, err := locator.GetPath(tempRepo) + calculatedPath, err := helper.GetPath(tempRepo) require.NoError(t, err) require.Equal(t, tempDir, calculatedPath)