Merge branch 'eb/code-280' of _OKE5H2PQKOUfzFFDuD4FA/default/CODE/gitness (#99)

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"
+	headCommit, err := g.GetCommit(ctx, pr.HeadRepoPath, pr.HeadBranch)
-	headBranch := git.BranchPrefix + pr.HeadBranch
+	if err != nil {
-	if err = git.NewCommand(ctx, "fetch", "--no-tags", remoteRepoName, headBranch+":"+trackingBranch).
+		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(

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)

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
+	mergeBaseCommitSHA, _, err := s.adapter.GetMergeBase(ctx, tmpRepo.Path, "origin", baseBranch, trackingBranch)
-	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)
 	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,
+			BaseSha:      tmpRepo.BaseSHA,
-			HeadSha:      headCommitSHA,
+			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,
+		BaseSha:      tmpRepo.BaseSHA,
-		HeadSha:      headCommitSHA,
+		HeadSha:      tmpRepo.HeadSHA,
 		MergeBaseSha: mergeBaseCommitSHA,
 		MergeSha:     mergeCommitSHA,
 	}, nil

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
+}