From 331221a2ef4343b095a59d03884fe1323bb7b350 Mon Sep 17 00:00:00 2001 From: Enver Bisevac Date: Fri, 26 May 2023 15:47:20 +0000 Subject: [PATCH] Merge branch 'eb/code-280' of _OKE5H2PQKOUfzFFDuD4FA/default/CODE/gitness (#99) --- gitrpc/internal/gitea/merge.go | 53 ++++++++++++++--------- gitrpc/internal/service/interface.go | 3 +- gitrpc/internal/service/merge.go | 64 ++++++++++++---------------- gitrpc/internal/types/types.go | 6 +++ 4 files changed, 68 insertions(+), 58 deletions(-) diff --git a/gitrpc/internal/gitea/merge.go b/gitrpc/internal/gitea/merge.go index 4f8bb4a3a..5e09620f6 100644 --- a/gitrpc/internal/gitea/merge.go +++ b/gitrpc/internal/gitea/merge.go @@ -31,7 +31,9 @@ func (g Adapter) CreateTemporaryRepoForPR( ctx context.Context, reposTempPath string, pr *types.PullRequest, -) (string, error) { + baseBranch string, + trackingBranch string, +) (types.TempRepository, error) { if pr.BaseRepoPath == "" && pr.HeadRepoPath != "" { pr.BaseRepoPath = pr.HeadRepoPath } @@ -41,11 +43,11 @@ func (g Adapter) CreateTemporaryRepoForPR( } if pr.BaseBranch == "" { - return "", errors.New("empty base branch") + return types.TempRepository{}, errors.New("empty base branch") } if pr.HeadBranch == "" { - return "", errors.New("empty head branch") + return types.TempRepository{}, errors.New("empty head branch") } baseRepoPath := pr.BaseRepoPath @@ -54,17 +56,15 @@ func (g Adapter) CreateTemporaryRepoForPR( // Clone base repo. tmpBasePath, err := tempdir.CreateTemporaryPath(reposTempPath, "pull") if err != nil { - return "", err + return types.TempRepository{}, err } if err = g.InitRepository(ctx, tmpBasePath, false); err != nil { - // log.Error("git init tmpBasePath: %v", err) _ = tempdir.RemoveTemporaryPath(tmpBasePath) - return "", err + return types.TempRepository{}, err } remoteRepoName := "head_repo" - baseBranch := "base" // Add head repo remote. addCacheRepo := func(staging, cache string) error { @@ -84,7 +84,7 @@ func (g Adapter) CreateTemporaryRepoForPR( if err = addCacheRepo(tmpBasePath, baseRepoPath); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) - return "", fmt.Errorf("unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepoPath, err) + return types.TempRepository{}, fmt.Errorf("unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepoPath, err) } var outbuf, errbuf strings.Builder @@ -96,14 +96,20 @@ func (g Adapter) CreateTemporaryRepoForPR( }); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to add base repository as origin "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to add base repository as origin "+ "[%s -> tmpBasePath]:\n%s\n%s", pr.BaseRepoPath, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() + // Fetch base branch + baseCommit, err := g.GetCommit(ctx, pr.BaseRepoPath, pr.BaseBranch) + if err != nil { + return types.TempRepository{}, fmt.Errorf("failed to get commit of %s branch: %w", baseBranch, err) + } + baseID := baseCommit.SHA if err = git.NewCommand(ctx, "fetch", "origin", "--no-tags", "--", - pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch). + baseID+":"+baseBranch, baseID+":original_"+baseBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -111,7 +117,7 @@ func (g Adapter) CreateTemporaryRepoForPR( }); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to fetch origin base branch "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to fetch origin base branch "+ "[%s:%s -> base, original_base in tmpBasePath].\n%s\n%s", pr.BaseRepoPath, pr.BaseBranch, outbuf.String(), errbuf.String()) } @@ -126,7 +132,7 @@ func (g Adapter) CreateTemporaryRepoForPR( }); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to set HEAD as base "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to set HEAD as base "+ "branch [tmpBasePath]:\n%s\n%s", outbuf.String(), errbuf.String()) } outbuf.Reset() @@ -135,7 +141,7 @@ func (g Adapter) CreateTemporaryRepoForPR( if err = addCacheRepo(tmpBasePath, headRepoPath); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to head base repository "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to head base repository "+ "to temporary repo [%s -> tmpBasePath]", pr.HeadRepoPath) } @@ -147,15 +153,18 @@ func (g Adapter) CreateTemporaryRepoForPR( }); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to add head repository as head_repo "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to add head repository as head_repo "+ "[%s -> tmpBasePath]:\n%s\n%s", pr.HeadRepoPath, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() - trackingBranch := "tracking" - headBranch := git.BranchPrefix + pr.HeadBranch - if err = git.NewCommand(ctx, "fetch", "--no-tags", remoteRepoName, headBranch+":"+trackingBranch). + headCommit, err := g.GetCommit(ctx, pr.HeadRepoPath, pr.HeadBranch) + if err != nil { + return types.TempRepository{}, fmt.Errorf("failed to get commit of %s branch: %w", trackingBranch, err) + } + headID := headCommit.SHA + if err = git.NewCommand(ctx, "fetch", "--no-tags", remoteRepoName, headID+":"+trackingBranch). Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: &outbuf, @@ -163,14 +172,18 @@ func (g Adapter) CreateTemporaryRepoForPR( }); err != nil { _ = tempdir.RemoveTemporaryPath(tmpBasePath) giteaErr := &giteaRunStdError{err: err, stderr: errbuf.String()} - return "", processGiteaErrorf(giteaErr, "unable to fetch head_repo head branch "+ + return types.TempRepository{}, processGiteaErrorf(giteaErr, "unable to fetch head_repo head branch "+ "[%s:%s -> tracking in tmpBasePath]:\n%s\n%s", - pr.HeadRepoPath, headBranch, outbuf.String(), errbuf.String()) + pr.HeadRepoPath, pr.HeadBranch, outbuf.String(), errbuf.String()) } outbuf.Reset() errbuf.Reset() - return tmpBasePath, nil + return types.TempRepository{ + Path: tmpBasePath, + BaseSHA: baseID, + HeadSHA: headID, + }, nil } func runMergeCommand( diff --git a/gitrpc/internal/service/interface.go b/gitrpc/internal/service/interface.go index a8c091f8e..a61c349c1 100644 --- a/gitrpc/internal/service/interface.go +++ b/gitrpc/internal/service/interface.go @@ -48,7 +48,8 @@ type GitAdapter interface { GetRef(ctx context.Context, repoPath string, name string, refType enum.RefType) (string, error) GetRefPath(refName string, refType enum.RefType) (string, error) UpdateRef(ctx context.Context, repoPath, refName string, refType enum.RefType, newValue, oldValue string) error - CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest) (string, error) + CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest, + baseBranch, trackingBranch string) (types.TempRepository, error) Merge(ctx context.Context, pr *types.PullRequest, mergeMethod enum.MergeMethod, baseBranch, trackingBranch string, tmpBasePath string, mergeMsg string, env []string, identity *types.Identity) error GetMergeBase(ctx context.Context, repoPath, remote, base, head string) (string, string, error) diff --git a/gitrpc/internal/service/merge.go b/gitrpc/internal/service/merge.go index ead1fa8a9..ab74eafb6 100644 --- a/gitrpc/internal/service/merge.go +++ b/gitrpc/internal/service/merge.go @@ -52,6 +52,9 @@ func (s MergeService) Merge( base := request.Base repoPath := getFullPathForRepo(s.reposRoot, base.RepoUid) + baseBranch := "base" + trackingBranch := "tracking" + pr := &types.PullRequest{ BaseRepoPath: repoPath, BaseBranch: request.BaseBranch, @@ -59,90 +62,77 @@ func (s MergeService) Merge( } // Clone base repo. - tmpBasePath, err := s.adapter.CreateTemporaryRepoForPR(ctx, s.reposTempDir, pr) + tmpRepo, err := s.adapter.CreateTemporaryRepoForPR(ctx, s.reposTempDir, pr, baseBranch, trackingBranch) if err != nil { return nil, err } defer func() { - rmErr := tempdir.RemoveTemporaryPath(tmpBasePath) + rmErr := tempdir.RemoveTemporaryPath(tmpRepo.Path) if rmErr != nil { - log.Ctx(ctx).Warn().Msgf("Removing temporary location %s for merge operation was not successful", tmpBasePath) + log.Ctx(ctx).Warn().Msgf("Removing temporary location %s for merge operation was not successful", tmpRepo.Path) } }() - // no error check needed, all branches were created when creating the temporary repo - baseBranch := "base" - trackingBranch := "tracking" - headCommit, err := s.adapter.GetCommit(ctx, tmpBasePath, trackingBranch) - if err != nil { - return nil, fmt.Errorf("failed to get commit of tracking branch (head): %w", err) - } - headCommitSHA := headCommit.SHA - baseCommit, err := s.adapter.GetCommit(ctx, tmpBasePath, baseBranch) - if err != nil { - return nil, fmt.Errorf("failed to get commit of base branch: %w", err) - } - baseCommitSHA := baseCommit.SHA - mergeBaseCommitSHA, _, err := s.adapter.GetMergeBase(ctx, tmpBasePath, "origin", baseBranch, trackingBranch) + mergeBaseCommitSHA, _, err := s.adapter.GetMergeBase(ctx, tmpRepo.Path, "origin", baseBranch, trackingBranch) if err != nil { return nil, fmt.Errorf("failed to get merge base: %w", err) } - if headCommitSHA == mergeBaseCommitSHA { + if tmpRepo.HeadSHA == mergeBaseCommitSHA { return nil, ErrInvalidArgumentf("no changes between head branch %s and base branch %s", request.HeadBranch, request.BaseBranch) } - if request.HeadExpectedSha != "" && request.HeadExpectedSha != headCommitSHA { + if request.HeadExpectedSha != "" && request.HeadExpectedSha != tmpRepo.HeadSHA { return nil, status.Errorf( codes.FailedPrecondition, "head branch '%s' is on SHA '%s' which doesn't match expected SHA '%s'.", request.HeadBranch, - headCommitSHA, + tmpRepo.HeadSHA, request.HeadExpectedSha) } var outbuf, errbuf strings.Builder // Enable sparse-checkout - sparseCheckoutList, err := s.adapter.GetDiffTree(ctx, tmpBasePath, baseBranch, trackingBranch) + sparseCheckoutList, err := s.adapter.GetDiffTree(ctx, tmpRepo.Path, baseBranch, trackingBranch) if err != nil { return nil, fmt.Errorf("execution of GetDiffTree failed: %w", err) } - infoPath := filepath.Join(tmpBasePath, ".git", "info") + infoPath := filepath.Join(tmpRepo.Path, ".git", "info") if err = os.MkdirAll(infoPath, 0o700); err != nil { - return nil, fmt.Errorf("unable to create .git/info in tmpBasePath: %w", err) + return nil, fmt.Errorf("unable to create .git/info in tmpRepo.Path: %w", err) } sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") if err = os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0o600); err != nil { return nil, - fmt.Errorf("unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) + fmt.Errorf("unable to write .git/info/sparse-checkout file in tmpRepo.Path: %w", err) } // Switch off LFS process (set required, clean and smudge here also) - if err = s.adapter.Config(ctx, tmpBasePath, "filter.lfs.process", ""); err != nil { + if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.process", ""); err != nil { return nil, err } - if err = s.adapter.Config(ctx, tmpBasePath, "filter.lfs.required", "false"); err != nil { + if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.required", "false"); err != nil { return nil, err } - if err = s.adapter.Config(ctx, tmpBasePath, "filter.lfs.clean", ""); err != nil { + if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.clean", ""); err != nil { return nil, err } - if err = s.adapter.Config(ctx, tmpBasePath, "filter.lfs.smudge", ""); err != nil { + if err = s.adapter.Config(ctx, tmpRepo.Path, "filter.lfs.smudge", ""); err != nil { return nil, err } - if err = s.adapter.Config(ctx, tmpBasePath, "core.sparseCheckout", "true"); err != nil { + if err = s.adapter.Config(ctx, tmpRepo.Path, "core.sparseCheckout", "true"); err != nil { return nil, err } // Read base branch index - if err = s.adapter.ReadTree(ctx, tmpBasePath, "HEAD", io.Discard); err != nil { + if err = s.adapter.ReadTree(ctx, tmpRepo.Path, "HEAD", io.Discard); err != nil { return nil, fmt.Errorf("failed to read tree: %w", err) } outbuf.Reset() @@ -180,7 +170,7 @@ func (s MergeService) Merge( enum.MergeMethodFromRPC(request.Method), baseBranch, trackingBranch, - tmpBasePath, + tmpRepo.Path, mergeMsg, env, &types.Identity{ @@ -190,7 +180,7 @@ func (s MergeService) Merge( return nil, processGitErrorf(err, "merge failed") } - mergeCommitSHA, err := s.adapter.GetFullCommitID(ctx, tmpBasePath, baseBranch) + mergeCommitSHA, err := s.adapter.GetFullCommitID(ctx, tmpRepo.Path, baseBranch) if err != nil { return nil, fmt.Errorf("failed to get full commit id for the new merge: %w", err) } @@ -198,8 +188,8 @@ func (s MergeService) Merge( refType := enum.RefFromRPC(request.RefType) if refType == enum.RefTypeUndefined { return &rpc.MergeResponse{ - BaseSha: baseCommitSHA, - HeadSha: headCommitSHA, + BaseSha: tmpRepo.BaseSHA, + HeadSha: tmpRepo.HeadSHA, MergeBaseSha: mergeBaseCommitSHA, MergeSha: mergeCommitSHA, }, nil @@ -212,7 +202,7 @@ func (s MergeService) Merge( } pushRef := baseBranch + ":" + refPath - if err = s.adapter.Push(ctx, tmpBasePath, types.PushOptions{ + if err = s.adapter.Push(ctx, tmpRepo.Path, types.PushOptions{ Remote: "origin", Branch: pushRef, Force: request.Force, @@ -222,8 +212,8 @@ func (s MergeService) Merge( } return &rpc.MergeResponse{ - BaseSha: baseCommitSHA, - HeadSha: headCommitSHA, + BaseSha: tmpRepo.BaseSHA, + HeadSha: tmpRepo.HeadSHA, MergeBaseSha: mergeBaseCommitSHA, MergeSha: mergeCommitSHA, }, nil diff --git a/gitrpc/internal/types/types.go b/gitrpc/internal/types/types.go index 4e4222a56..1b97e9b3d 100644 --- a/gitrpc/internal/types/types.go +++ b/gitrpc/internal/types/types.go @@ -305,3 +305,9 @@ type CommitFilter struct { Until int64 Committer string } + +type TempRepository struct { + Path string + BaseSHA string + HeadSHA string +}