From 9b5d2156d486153baa092e2fa49a6043180b6c53 Mon Sep 17 00:00:00 2001 From: Paul Okstad Date: Wed, 11 Nov 2020 21:38:30 -0800 Subject: [PATCH 1/4] Inject config into local repository constructor --- internal/git/repository.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/git/repository.go b/internal/git/repository.go index a11ca80838b..1e79d610067 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -103,12 +103,14 @@ type Repository interface { // LocalRepository represents a local Git repository. type LocalRepository struct { repo repository.GitRepo + cfg config.Cfg } // NewRepository creates a new Repository from its protobuf representation. -func NewRepository(repo repository.GitRepo) *LocalRepository { +func NewRepository(repo repository.GitRepo, cfg config.Cfg) *LocalRepository { return &LocalRepository{ repo: repo, + cfg: cfg, } } -- GitLab From 170bd00adb9c6e9d92391b8988c1b30c841fead2 Mon Sep 17 00:00:00 2001 From: Paul Okstad Date: Thu, 12 Nov 2020 09:30:26 -0800 Subject: [PATCH 2/4] LocalRepository methods should use injected config --- internal/git/repository.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/git/repository.go b/internal/git/repository.go index 1e79d610067..f89aefe7560 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -316,7 +316,7 @@ func (repo *LocalRepository) UpdateRef(ctx context.Context, reference, newrev, o Flags: []Option{Flag{Name: "-z"}, Flag{Name: "--stdin"}}, }, WithStdin(strings.NewReader(fmt.Sprintf("update %s\x00%s\x00%s\x00", reference, newrev, oldrev))), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config), + WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), repo.cfg), ) if err != nil { return err @@ -342,7 +342,7 @@ func (repo *LocalRepository) FetchRemote(ctx context.Context, remoteName string, Args: []string{remoteName}, }, WithStderr(opts.Stderr), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config), + WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), repo.cfg), ) if err != nil { return err -- GitLab From 7d45d381da1f1deaf2627b17dc772607f57b7bc3 Mon Sep 17 00:00:00 2001 From: Paul Okstad Date: Thu, 12 Nov 2020 09:55:59 -0800 Subject: [PATCH 3/4] Update usage of NewRepository to inject config --- internal/git/config_test.go | 3 +- internal/git/remote_test.go | 3 +- internal/git/repository_test.go | 33 ++++++++++--------- internal/git2go/commit_test.go | 2 +- internal/gitaly/service/commit/list_files.go | 3 +- .../service/conflicts/list_conflict_files.go | 2 +- .../conflicts/list_conflict_files_test.go | 3 +- .../service/conflicts/resolve_conflicts.go | 5 +-- .../gitaly/service/operations/branches.go | 4 +-- .../gitaly/service/operations/commit_files.go | 4 +-- internal/gitaly/service/operations/merge.go | 6 ++-- .../gitaly/service/operations/submodules.go | 2 +- internal/gitaly/service/ref/branches.go | 3 +- 13 files changed, 40 insertions(+), 33 deletions(-) diff --git a/internal/git/config_test.go b/internal/git/config_test.go index d3db1187d53..7835ba2e62c 100644 --- a/internal/git/config_test.go +++ b/internal/git/config_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -16,7 +17,7 @@ func TestLocalRepository_Config(t *testing.T) { bareRepo, _, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(bareRepo) + repo := NewRepository(bareRepo, config.Config) require.Equal(t, RepositoryConfig{repo: bareRepo}, repo.Config()) } diff --git a/internal/git/remote_test.go b/internal/git/remote_test.go index a50e2e49d9a..758ef0f7a45 100644 --- a/internal/git/remote_test.go +++ b/internal/git/remote_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "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" @@ -14,7 +15,7 @@ import ( func TestLocalRepository_Remote(t *testing.T) { repository := &gitalypb.Repository{StorageName: "stub", RelativePath: "/stub"} - repo := NewRepository(repository) + repo := NewRepository(repository, config.Config) require.Equal(t, RepositoryRemote{repo: repository}, repo.Remote()) } diff --git a/internal/git/repository_test.go b/internal/git/repository_test.go index 86f3f1920b8..cd3cee35c51 100644 --- a/internal/git/repository_test.go +++ b/internal/git/repository_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "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/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -27,7 +28,7 @@ const ( func TestLocalRepository(t *testing.T) { TestRepository(t, func(t testing.TB, pbRepo *gitalypb.Repository) Repository { t.Helper() - return NewRepository(pbRepo) + return NewRepository(pbRepo, config.Config) }) } @@ -38,7 +39,7 @@ func TestLocalRepository_ContainsRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testcases := []struct { desc string @@ -78,7 +79,7 @@ func TestLocalRepository_GetReference(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testcases := []struct { desc string @@ -127,7 +128,7 @@ func TestLocalRepository_GetBranch(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testcases := []struct { desc string @@ -176,7 +177,7 @@ func TestLocalRepository_GetReferences(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testcases := []struct { desc string @@ -242,7 +243,7 @@ crlf binary lf text `), os.ModePerm)) - repo := NewRepository(pbRepo) + repo := NewRepository(pbRepo, config.Config) for _, tc := range []struct { desc string @@ -306,7 +307,7 @@ func TestLocalRepository_ReadObject(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) for _, tc := range []struct { desc string @@ -341,7 +342,7 @@ func TestLocalRepository_GetBranches(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) refs, err := repo.GetBranches(ctx) require.NoError(t, err) @@ -355,7 +356,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - otherRef, err := NewRepository(testRepo).GetReference(ctx, "refs/heads/gitaly-test-ref") + otherRef, err := NewRepository(testRepo, config.Config).GetReference(ctx, "refs/heads/gitaly-test-ref") require.NoError(t, err) testcases := []struct { @@ -456,7 +457,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) err := repo.UpdateRef(ctx, tc.ref, tc.newrev, tc.oldrev) tc.verify(t, repo, err) @@ -484,11 +485,11 @@ func TestLocalRepository_FetchRemote(t *testing.T) { t.FailNow() } - return NewRepository(testRepo), testRepoPath, cleanup + return NewRepository(testRepo, config.Config), testRepoPath, cleanup } t.Run("invalid name", func(t *testing.T) { - repo := NewRepository(nil) + repo := NewRepository(nil, config.Config) err := repo.FetchRemote(ctx, " ", FetchOpts{}) require.True(t, errors.Is(err, ErrInvalidArg)) @@ -499,7 +500,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, _, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) var stderr bytes.Buffer err := repo.FetchRemote(ctx, "stub", FetchOpts{Stderr: &stderr}) require.Error(t, err) @@ -534,7 +535,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) var stderr bytes.Buffer @@ -549,7 +550,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) @@ -577,7 +578,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo) + repo := NewRepository(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go index 218c5b51db9..87a99977d57 100644 --- a/internal/git2go/commit_test.go +++ b/internal/git2go/commit_test.go @@ -55,7 +55,7 @@ func TestExecutor_Commit(t *testing.T) { pbRepo, repoPath, clean := testhelper.InitBareRepo(t) defer clean() - repo := git.NewRepository(pbRepo) + repo := git.NewRepository(pbRepo, config.Config) originalFile, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("original")) require.NoError(t, err) diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go index 90429d14858..879a3b3a72c 100644 --- a/internal/gitaly/service/commit/list_files.go +++ b/internal/gitaly/service/commit/list_files.go @@ -9,6 +9,7 @@ import ( log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/lstree" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -44,7 +45,7 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit revision = string(defaultBranch) } - containsRef, err := git.NewRepository(in.Repository).ContainsRef(stream.Context(), revision) + containsRef, err := git.NewRepository(in.Repository, config.Config).ContainsRef(stream.Context(), revision) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go index eb7b0f294b7..81532207af7 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files.go +++ b/internal/gitaly/service/conflicts/list_conflict_files.go @@ -25,7 +25,7 @@ func (s *server) listConflictFiles(request *gitalypb.ListConflictFilesRequest, s return helper.ErrInvalidArgument(err) } - repo := git.NewRepository(request.Repository) + repo := git.NewRepository(request.Repository, s.cfg) ours, err := repo.ResolveRefish(ctx, request.OurCommitOid+"^{commit}") if err != nil { diff --git a/internal/gitaly/service/conflicts/list_conflict_files_test.go b/internal/gitaly/service/conflicts/list_conflict_files_test.go index fa18db0db62..9039bcbf393 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files_test.go +++ b/internal/gitaly/service/conflicts/list_conflict_files_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "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" @@ -220,7 +221,7 @@ func buildCommit(t *testing.T, ctx context.Context, repo *gitalypb.Repository, r testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "commit", "-m", "message") - oid, err := git.NewRepository(repo).ResolveRefish(ctx, "HEAD") + oid, err := git.NewRepository(repo, config.Config).ResolveRefish(ctx, "HEAD") require.NoError(t, err) testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "reset", "--hard", "HEAD~") diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go index 390ae99dbef..ed5ee9eea43 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts.go @@ -15,6 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/conflict" "gitlab.com/gitlab-org/gitaly/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/gitalyssh" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -209,7 +210,7 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader return err } - if err := git.NewRepository(header.GetRepository()).UpdateRef( + if err := git.NewRepository(header.GetRepository(), config.Config).UpdateRef( stream.Context(), "refs/heads/"+string(header.GetSourceBranch()), result.CommitID, @@ -253,7 +254,7 @@ func sameRepo(left, right *gitalypb.Repository) bool { func (s *server) repoWithBranchCommit(ctx context.Context, srcRepo, targetRepo *gitalypb.Repository, srcBranch, targetBranch []byte) error { const peelCommit = "^{commit}" - src := git.NewRepository(srcRepo) + src := git.NewRepository(srcRepo, s.cfg) if sameRepo(srcRepo, targetRepo) { _, err := src.ResolveRefish(ctx, string(targetBranch)+peelCommit) return err diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index f1cb37c043f..526816d55bc 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -39,7 +39,7 @@ func (s *server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB return nil, helper.ErrPreconditionFailed(err) } - _, err = git.NewRepository(req.Repository).GetBranch(ctx, string(req.BranchName)) + _, err = git.NewRepository(req.Repository, s.cfg).GetBranch(ctx, string(req.BranchName)) if err == nil { return nil, status.Errorf(codes.FailedPrecondition, "Bad Request (branch exists)") } else if !errors.Is(err, git.ErrReferenceNotFound) { @@ -118,7 +118,7 @@ func (s *server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteB // Implement UserDeleteBranch in Go - revision, err := git.NewRepository(req.Repository).GetBranch(ctx, string(req.BranchName)) + revision, err := git.NewRepository(req.Repository, s.cfg).GetBranch(ctx, string(req.BranchName)) if err != nil { return nil, helper.ErrPreconditionFailed(err) } diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 59ce4467772..470e2afdc0e 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -162,7 +162,7 @@ func (s *server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi return fmt.Errorf("get repo path: %w", err) } - localRepo := git.NewRepository(header.Repository) + localRepo := git.NewRepository(header.Repository, s.cfg) targetBranchName := "refs/heads/" + string(header.BranchName) targetBranchCommit, err := localRepo.ResolveRefish(ctx, targetBranchName+"^{commit}") @@ -375,7 +375,7 @@ func (s *server) resolveParentCommit(ctx context.Context, local git.Repository, } func (s *server) fetchMissingCommit(ctx context.Context, local, remote *gitalypb.Repository, commitID string) error { - if _, err := git.NewRepository(local).ResolveRefish(ctx, commitID+"^{commit}"); err != nil { + if _, err := git.NewRepository(local, s.cfg).ResolveRefish(ctx, commitID+"^{commit}"); err != nil { if !errors.Is(err, git.ErrReferenceNotFound) || remote == nil { return fmt.Errorf("lookup parent commit: %w", err) } diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index e953dd95b72..dad91616f30 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -209,7 +209,7 @@ func (s *server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc return err } - revision, err := git.NewRepository(repo).ResolveRefish(ctx, string(firstRequest.Branch)) + revision, err := git.NewRepository(repo, s.cfg).ResolveRefish(ctx, string(firstRequest.Branch)) if err != nil { return err } @@ -300,7 +300,7 @@ func (s *server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ return s.userFFBranchRuby(ctx, in) } - revision, err := git.NewRepository(in.Repository).ResolveRefish(ctx, string(in.Branch)) + revision, err := git.NewRepository(in.Repository, s.cfg).ResolveRefish(ctx, string(in.Branch)) if err != nil { return nil, helper.ErrInvalidArgument(err) } @@ -400,7 +400,7 @@ func (s *server) userMergeToRef(ctx context.Context, request *gitalypb.UserMerge return nil, err } - repo := git.NewRepository(request.Repository) + repo := git.NewRepository(request.Repository, s.cfg) refName := string(request.Branch) if request.FirstParentRef != nil { diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go index b415bf2b6e9..46d3cae4c0a 100644 --- a/internal/gitaly/service/operations/submodules.go +++ b/internal/gitaly/service/operations/submodules.go @@ -76,7 +76,7 @@ func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest } func (s *server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpdateSubmoduleRequest) (*gitalypb.UserUpdateSubmoduleResponse, error) { - repo := git.NewRepository(req.GetRepository()) + repo := git.NewRepository(req.GetRepository(), s.cfg) branches, err := repo.GetBranches(ctx) if err != nil { return nil, fmt.Errorf("%s: get branches: %w", userUpdateSubmoduleName, err) diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index 67dcaa6a59f..1e873178075 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -7,6 +7,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -26,7 +27,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest repo := req.GetRepository() - branch, err := git.NewRepository(repo).GetBranch(ctx, refName) + branch, err := git.NewRepository(repo, config.Config).GetBranch(ctx, refName) if err != nil { if errors.Is(err, git.ErrReferenceNotFound) { return &gitalypb.FindBranchResponse{}, nil -- GitLab From f8bfe6a9d92f4701ab79f5986aee11c4c4e6acd6 Mon Sep 17 00:00:00 2001 From: Paul Okstad Date: Thu, 12 Nov 2020 10:44:11 -0800 Subject: [PATCH 4/4] Changelog for MR 2779 --- changelogs/unreleased/po-local-repo-inject-config.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/po-local-repo-inject-config.yml diff --git a/changelogs/unreleased/po-local-repo-inject-config.yml b/changelogs/unreleased/po-local-repo-inject-config.yml new file mode 100644 index 00000000000..ca22344d099 --- /dev/null +++ b/changelogs/unreleased/po-local-repo-inject-config.yml @@ -0,0 +1,5 @@ +--- +title: Inject config into local repository +merge_request: 2779 +author: +type: other -- GitLab