From 530a707052e23f8f3225212ec8a2382d9fe4013c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Mon, 3 Feb 2025 10:52:01 +0000 Subject: [PATCH] feat: [CODE-3076]: update several refs on merge (#3355) * resolve pr comments * update several refs on merge --- app/api/controller/pullreq/merge.go | 98 +++++--- app/api/controller/pullreq/pr_create.go | 2 +- app/api/controller/pullreq/pr_state.go | 10 + app/api/controller/repo/rebase.go | 20 +- app/api/controller/repo/squash.go | 20 +- app/services/pullreq/handlers_mergeable.go | 56 ++--- app/services/pullreq/service.go | 2 - git/branch.go | 15 +- git/enum/ref.go | 5 +- git/hook/refupdate.go | 185 +++++++------- git/merge.go | 118 +++++---- git/merge/merge.go | 277 +++++++++------------ git/operations.go | 10 +- git/ref.go | 10 +- git/tag.go | 17 +- 15 files changed, 432 insertions(+), 413 deletions(-) diff --git a/app/api/controller/pullreq/merge.go b/app/api/controller/pullreq/merge.go index 6966e4c1f..afd73ae60 100644 --- a/app/api/controller/pullreq/merge.go +++ b/app/api/controller/pullreq/merge.go @@ -177,13 +177,7 @@ func (c *Controller) Merge( } sourceRepo := targetRepo - sourceWriteParams := targetWriteParams if pr.SourceRepoID != pr.TargetRepoID { - sourceWriteParams, err = controller.CreateRPCInternalWriteParams(ctx, c.urlProvider, session, sourceRepo) - if err != nil { - return nil, nil, fmt.Errorf("failed to create RPC write params: %w", err) - } - sourceRepo, err = c.repoStore.Find(ctx, pr.SourceRepoID) if err != nil { return nil, nil, fmt.Errorf("failed to get source repository: %w", err) @@ -195,7 +189,7 @@ func (c *Controller) Merge( return nil, nil, fmt.Errorf("failed to fetch rules: %w", err) } - checkResults, err := c.checkStore.ListResults(ctx, targetRepo.ID, pr.SourceSHA) + checkResults, err := c.checkStore.ListResults(ctx, targetRepo.ID, in.SourceSHA) if err != nil { return nil, nil, fmt.Errorf("failed to list status checks: %w", err) } @@ -292,7 +286,7 @@ func (c *Controller) Merge( BaseBranch: pr.TargetBranch, HeadRepoUID: sourceRepo.GitUID, HeadBranch: pr.SourceBranch, - RefType: gitenum.RefTypeUndefined, // update no refs -> no commit will be created + Refs: nil, // update no refs -> no commit will be created HeadExpectedSHA: sha.Must(in.SourceSHA), Method: gitenum.MergeMethod(in.Method), }) @@ -425,6 +419,65 @@ func (c *Controller) Merge( log.Ctx(ctx).Debug().Msgf("all pre-check passed, merge PR") + sourceBranchSHA, err := sha.New(in.SourceSHA) + if err != nil { + return nil, nil, fmt.Errorf("failed to convert source SHA: %w", err) + } + + refSourceBranch, err := git.GetRefPath(pr.SourceBranch, gitenum.RefTypeBranch) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate source branch ref name: %w", err) + } + + refTargetBranch, err := git.GetRefPath(pr.TargetBranch, gitenum.RefTypeBranch) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate target branch ref name: %w", err) + } + + prNumber := strconv.FormatInt(pr.Number, 10) + + refPullReqHead, err := git.GetRefPath(prNumber, gitenum.RefTypePullReqHead) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate pull request head ref name: %w", err) + } + + refPullReqMerge, err := git.GetRefPath(prNumber, gitenum.RefTypePullReqMerge) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate pull requert merge ref name: %w", err) + } + + refUpdates := make([]git.RefUpdate, 0, 4) + + // Update the target branch to the result of the merge. + refUpdates = append(refUpdates, git.RefUpdate{ + Name: refTargetBranch, + Old: sha.SHA{}, // don't care about the current commit SHA of the target branch. + New: sha.SHA{}, // update to the result of the merge. + }) + + // Make sure the PR head ref points to the correct commit after the merge. + refUpdates = append(refUpdates, git.RefUpdate{ + Name: refPullReqHead, + Old: sha.SHA{}, // don't care about the old value. + New: sourceBranchSHA, + }) + + // Delete the PR merge reference. + refUpdates = append(refUpdates, git.RefUpdate{ + Name: refPullReqMerge, + Old: sha.SHA{}, + New: sha.Nil, + }) + + if ruleOut.DeleteSourceBranch { + // Delete the source branch. + refUpdates = append(refUpdates, git.RefUpdate{ + Name: refSourceBranch, + Old: sourceBranchSHA, + New: sha.Nil, + }) + } + now := time.Now() mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ WriteParams: targetWriteParams, @@ -436,8 +489,7 @@ func (c *Controller) Merge( CommitterDate: &now, Author: author, AuthorDate: &now, - RefType: gitenum.RefTypeBranch, - RefName: pr.TargetBranch, + Refs: refUpdates, HeadExpectedSHA: sha.Must(in.SourceSHA), Method: gitenum.MergeMethod(in.Method), }) @@ -545,27 +597,13 @@ func (c *Controller) Merge( SourceSHA: mergeOutput.HeadSHA.String(), }) - var branchDeleted bool if ruleOut.DeleteSourceBranch { - errDelete := c.git.DeleteBranch(ctx, &git.DeleteBranchParams{ - WriteParams: sourceWriteParams, - BranchName: pr.SourceBranch, - }) - if errDelete != nil { + pr.ActivitySeq = activitySeqBranchDeleted + if _, errAct := c.activityStore.CreateWithPayload(ctx, pr, mergedBy, + &types.PullRequestActivityPayloadBranchDelete{SHA: in.SourceSHA}, nil); errAct != nil { // non-critical error - log.Ctx(ctx).Err(errDelete).Msgf("failed to delete source branch after merging") - } else { - branchDeleted = true - - // NOTE: there is a chance someone pushed on the branch between merge and delete. - // Either way, we'll use the SHA that was merged with for the activity to be consistent from PR perspective. - pr.ActivitySeq = activitySeqBranchDeleted - if _, errAct := c.activityStore.CreateWithPayload(ctx, pr, mergedBy, - &types.PullRequestActivityPayloadBranchDelete{SHA: in.SourceSHA}, nil); errAct != nil { - // non-critical error - log.Ctx(ctx).Err(errAct). - Msgf("failed to write pull request activity for successful automatic branch delete") - } + log.Ctx(ctx).Err(errAct). + Msgf("failed to write pull request activity for successful automatic branch delete") } } @@ -621,7 +659,7 @@ func (c *Controller) Merge( } return &types.MergeResponse{ SHA: mergeOutput.MergeSHA.String(), - BranchDeleted: branchDeleted, + BranchDeleted: ruleOut.DeleteSourceBranch, RuleViolations: violations, }, nil, nil } diff --git a/app/api/controller/pullreq/pr_create.go b/app/api/controller/pullreq/pr_create.go index 3e2e3123c..e1d56964d 100644 --- a/app/api/controller/pullreq/pr_create.go +++ b/app/api/controller/pullreq/pr_create.go @@ -189,7 +189,7 @@ func (c *Controller) Create( Name: strconv.FormatInt(targetRepo.PullReqSeq, 10), Type: gitenum.RefTypePullReqHead, NewValue: sourceSHA, - OldValue: sha.None, // this is a new pull request, so we expect that the ref doesn't exist + OldValue: sha.None, // we don't care about the old value }) if err != nil { return fmt.Errorf("failed to create PR head ref: %w", err) diff --git a/app/api/controller/pullreq/pr_state.go b/app/api/controller/pullreq/pr_state.go index 54f7cd354..1b17a6ee2 100644 --- a/app/api/controller/pullreq/pr_state.go +++ b/app/api/controller/pullreq/pr_state.go @@ -168,11 +168,21 @@ func (c *Controller) State(ctx context.Context, pr.Closed = &nowMilli pr.MarkAsMergeUnchecked() + // delete the merge pull request reference + err = c.git.UpdateRef(ctx, git.UpdateRefParams{ + WriteParams: targetWriteParams, + Name: strconv.FormatInt(pr.Number, 10), + Type: gitenum.RefTypePullReqMerge, + NewValue: sha.Nil, + OldValue: sha.None, // we don't care about the old value + }) + case changeReopen: pr.SourceSHA = sourceSHA.String() pr.MergeBaseSHA = mergeBaseSHA.String() pr.Closed = nil + // create the head pull request reference err = c.git.UpdateRef(ctx, git.UpdateRefParams{ WriteParams: targetWriteParams, Name: strconv.FormatInt(pr.Number, 10), diff --git a/app/api/controller/repo/rebase.go b/app/api/controller/repo/rebase.go index a9c31771e..3c09b6bbf 100644 --- a/app/api/controller/repo/rebase.go +++ b/app/api/controller/repo/rebase.go @@ -157,11 +157,18 @@ func (c *Controller) Rebase( return nil, nil, fmt.Errorf("failed to create RPC write params: %w", err) } - refType := gitenum.RefTypeBranch - refName := in.HeadBranch - if in.DryRun { - refType = gitenum.RefTypeUndefined - refName = "" + var refs []git.RefUpdate + if !in.DryRun { + headBranchRef, err := git.GetRefPath(in.HeadBranch, gitenum.RefTypeBranch) + if err != nil { + return nil, nil, fmt.Errorf("failed to gerenere ref name: %w", err) + } + + refs = append(refs, git.RefUpdate{ + Name: headBranchRef, + Old: headBranch.Branch.SHA, + New: sha.SHA{}, // update to the result of the merge + }) } mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ @@ -169,8 +176,7 @@ func (c *Controller) Rebase( BaseSHA: baseCommitSHA, HeadRepoUID: repo.GitUID, HeadBranch: in.HeadBranch, - RefType: refType, - RefName: refName, + Refs: refs, HeadExpectedSHA: in.HeadCommitSHA, Method: gitenum.MergeMethodRebase, }) diff --git a/app/api/controller/repo/squash.go b/app/api/controller/repo/squash.go index 910849cff..fb156bcf1 100644 --- a/app/api/controller/repo/squash.go +++ b/app/api/controller/repo/squash.go @@ -135,6 +135,11 @@ func (c *Controller) Squash( in.HeadCommitSHA, headBranch.Branch.Name) } + headBranchRef, err := git.GetRefPath(in.HeadBranch, gitenum.RefTypeBranch) + if err != nil { + return nil, nil, fmt.Errorf("failed to gerenere ref name: %w", err) + } + baseCommitSHA := in.BaseCommitSHA if baseCommitSHA.IsEmpty() { baseBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{ @@ -153,11 +158,13 @@ func (c *Controller) Squash( return nil, nil, fmt.Errorf("failed to create RPC write params: %w", err) } - refType := gitenum.RefTypeBranch - refName := in.HeadBranch - if in.DryRun { - refType = gitenum.RefTypeUndefined - refName = "" + var refs []git.RefUpdate + if !in.DryRun { + refs = append(refs, git.RefUpdate{ + Name: headBranchRef, + Old: headBranch.Branch.SHA, + New: sha.SHA{}, // update to the result of the merge + }) } mergeBase, err := c.git.MergeBase(ctx, git.MergeBaseParams{ @@ -223,8 +230,7 @@ func (c *Controller) Squash( HeadRepoUID: repo.GitUID, HeadBranch: in.HeadBranch, Message: git.CommitMessage(in.Title, in.Message), - RefType: refType, - RefName: refName, + Refs: refs, Committer: committer, CommitterDate: &now, Author: author, diff --git a/app/services/pullreq/handlers_mergeable.go b/app/services/pullreq/handlers_mergeable.go index 00afed9c1..397a5b934 100644 --- a/app/services/pullreq/handlers_mergeable.go +++ b/app/services/pullreq/handlers_mergeable.go @@ -79,46 +79,6 @@ func (s *Service) mergeCheckOnReopen(ctx context.Context, ) } -// mergeCheckOnClosed deletes the merge ref. -func (s *Service) mergeCheckOnClosed(ctx context.Context, - event *events.Event[*pullreqevents.ClosedPayload], -) error { - return s.deleteMergeRef(ctx, event.Payload.SourceRepoID, event.Payload.Number) -} - -// mergeCheckOnMerged deletes the merge ref. -func (s *Service) mergeCheckOnMerged(ctx context.Context, - event *events.Event[*pullreqevents.MergedPayload], -) error { - return s.deleteMergeRef(ctx, event.Payload.SourceRepoID, event.Payload.Number) -} - -func (s *Service) deleteMergeRef(ctx context.Context, repoID int64, prNum int64) error { - repo, err := s.repoGitInfoCache.Get(ctx, repoID) - if err != nil { - return fmt.Errorf("failed to get repo with ID %d: %w", repoID, err) - } - - writeParams, err := createSystemRPCWriteParams(ctx, s.urlProvider, repo.ID, repo.GitUID) - if err != nil { - return fmt.Errorf("failed to generate rpc write params: %w", err) - } - - // TODO: This doesn't work for forked repos - err = s.git.UpdateRef(ctx, git.UpdateRefParams{ - WriteParams: writeParams, - Name: strconv.Itoa(int(prNum)), - Type: gitenum.RefTypePullReqMerge, - NewValue: sha.None, // when NewValue is empty will delete the ref. - OldValue: sha.None, // we don't care about the old value - }) - if err != nil { - return fmt.Errorf("failed to remove PR merge ref: %w", err) - } - - return nil -} - //nolint:funlen // refactor if required. func (s *Service) updateMergeData( ctx context.Context, @@ -186,6 +146,19 @@ func (s *Service) updateMergeData( return fmt.Errorf("failed to generate rpc write params: %w", err) } + refName, err := git.GetRefPath(strconv.Itoa(int(pr.Number)), gitenum.RefTypePullReqMerge) + if err != nil { + return fmt.Errorf("failed to generate pull request merge ref name: %w", err) + } + + refs := []git.RefUpdate{ + { + Name: refName, + Old: sha.SHA{}, // no matter what the value of the reference is + New: sha.SHA{}, // update it to point to result of the merge + }, + } + // call merge and store output in pr merge reference. now := time.Now() mergeOutput, err := s.git.Merge(ctx, &git.MergeParams{ @@ -193,8 +166,7 @@ func (s *Service) updateMergeData( BaseBranch: pr.TargetBranch, HeadRepoUID: sourceRepo.GitUID, HeadBranch: pr.SourceBranch, - RefType: gitenum.RefTypePullReqMerge, - RefName: strconv.Itoa(int(pr.Number)), + Refs: refs, HeadExpectedSHA: sha.Must(newSHA), Force: true, diff --git a/app/services/pullreq/service.go b/app/services/pullreq/service.go index ee5ee15b2..2793a54eb 100644 --- a/app/services/pullreq/service.go +++ b/app/services/pullreq/service.go @@ -194,8 +194,6 @@ func New(ctx context.Context, _ = r.RegisterCreated(service.mergeCheckOnCreated) _ = r.RegisterBranchUpdated(service.mergeCheckOnBranchUpdate) _ = r.RegisterReopened(service.mergeCheckOnReopen) - _ = r.RegisterClosed(service.mergeCheckOnClosed) - _ = r.RegisterMerged(service.mergeCheckOnMerged) return nil }) diff --git a/git/branch.go b/git/branch.go index 75817ea72..7a67546c8 100644 --- a/git/branch.go +++ b/git/branch.go @@ -104,14 +104,14 @@ func (s *Service) CreateBranch(ctx context.Context, params *CreateBranchParams) return nil, fmt.Errorf("failed to get target commit: %w", err) } - branchRef := api.GetReferenceFromBranchName(params.BranchName) - - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, branchRef) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { return nil, fmt.Errorf("failed to create ref updater to create the branch: %w", err) } - err = refUpdater.Do(ctx, sha.Nil, targetCommit.SHA) + branchRef := api.GetReferenceFromBranchName(params.BranchName) + + err = refUpdater.DoOne(ctx, branchRef, sha.Nil, targetCommit.SHA) if errors.IsConflict(err) { return nil, errors.Conflict("branch %q already exists", params.BranchName) } @@ -162,15 +162,16 @@ func (s *Service) DeleteBranch(ctx context.Context, params *DeleteBranchParams) } repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID) - branchRef := api.GetReferenceFromBranchName(params.BranchName) commitSha, _ := sha.NewOrEmpty(params.SHA) - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, branchRef) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { return fmt.Errorf("failed to create ref updater to create the branch: %w", err) } - err = refUpdater.Do(ctx, commitSha, sha.Nil) + branchRef := api.GetReferenceFromBranchName(params.BranchName) + + err = refUpdater.DoOne(ctx, branchRef, commitSha, sha.Nil) if errors.IsNotFound(err) { return errors.NotFound("branch %q does not exist", params.BranchName) } diff --git a/git/enum/ref.go b/git/enum/ref.go index 5c1309a1a..451158399 100644 --- a/git/enum/ref.go +++ b/git/enum/ref.go @@ -17,8 +17,7 @@ package enum type RefType int const ( - RefTypeUndefined RefType = iota - RefTypeRaw + RefTypeRaw RefType = iota RefTypeBranch RefTypeTag RefTypePullReqHead @@ -37,8 +36,6 @@ func (t RefType) String() string { return "head" case RefTypePullReqMerge: return "merge" - case RefTypeUndefined: - fallthrough default: return "" } diff --git a/git/hook/refupdate.go b/git/hook/refupdate.go index ebfe1bd3b..2ca56974e 100644 --- a/git/hook/refupdate.go +++ b/git/hook/refupdate.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "fmt" + "sort" "strings" "github.com/harness/gitness/errors" @@ -32,7 +33,6 @@ func CreateRefUpdater( hookClientFactory ClientFactory, envVars map[string]string, repoPath string, - ref string, ) (*RefUpdater, error) { if repoPath == "" { return nil, errors.Internal(nil, "repo path can't be empty") @@ -44,13 +44,11 @@ func CreateRefUpdater( } return &RefUpdater{ - state: stateInitOld, + state: stateInit, hookClient: client, envVars: envVars, repoPath: repoPath, - ref: ref, - oldValue: sha.None, - newValue: sha.None, + refs: nil, }, nil } @@ -62,9 +60,7 @@ type RefUpdater struct { hookClient Client envVars map[string]string repoPath string - ref string - oldValue sha.SHA - newValue sha.SHA + refs []ReferenceUpdate } // refUpdaterState represents state of the ref updater internal state machine. @@ -72,10 +68,8 @@ type refUpdaterState byte func (t refUpdaterState) String() string { switch t { - case stateInitOld: - return "INIT_OLD" - case stateInitNew: - return "INIT_NEW" + case stateInit: + return "INIT" case statePre: return "PRE" case stateUpdate: @@ -89,8 +83,7 @@ func (t refUpdaterState) String() string { } const ( - stateInitOld refUpdaterState = iota - stateInitNew + stateInit refUpdaterState = iota statePre stateUpdate statePost @@ -98,8 +91,8 @@ const ( ) // Do runs full ref update by executing all methods in the correct order. -func (u *RefUpdater) Do(ctx context.Context, oldValue, newValue sha.SHA) error { - if err := u.Init(ctx, oldValue, newValue); err != nil { +func (u *RefUpdater) Do(ctx context.Context, refs []ReferenceUpdate) error { + if err := u.Init(ctx, refs); err != nil { return fmt.Errorf("init failed: %w", err) } @@ -118,62 +111,70 @@ func (u *RefUpdater) Do(ctx context.Context, oldValue, newValue sha.SHA) error { return nil } -func (u *RefUpdater) Init(ctx context.Context, oldValue, newValue sha.SHA) error { - if err := u.InitOld(ctx, oldValue); err != nil { - return fmt.Errorf("init old failed: %w", err) - } - if err := u.InitNew(ctx, newValue); err != nil { - return fmt.Errorf("init new failed: %w", err) - } - - return nil +// DoOne runs full ref update of only one reference. +func (u *RefUpdater) DoOne(ctx context.Context, ref string, oldValue, newValue sha.SHA) error { + return u.Do(ctx, []ReferenceUpdate{ + { + Ref: ref, + Old: oldValue, + New: newValue, + }, + }) } -func (u *RefUpdater) InitOld(ctx context.Context, oldValue sha.SHA) error { - if u == nil { - return nil - } - - if u.state != stateInitOld { +func (u *RefUpdater) Init(ctx context.Context, refs []ReferenceUpdate) error { + if u.state != stateInit { return fmt.Errorf("invalid operation order: init old requires state=%s, current state=%s", - stateInitOld, u.state) + stateInit, u.state) } - if oldValue.IsEmpty() { - // if no old value was provided, use current value (as required for hooks) - val, err := u.getRef(ctx) - if errors.IsNotFound(err) { //nolint:gocritic - oldValue = sha.Nil - } else if err != nil { - return fmt.Errorf("failed to get current value of reference: %w", err) - } else { - oldValue = val + u.refs = make([]ReferenceUpdate, 0, len(refs)) + for _, ref := range refs { + oldValue := ref.Old + newValue := ref.New + + var oldValueKnown bool + + if oldValue.IsEmpty() { + // if no old value was provided, use current value (as required for hooks) + val, err := u.getRef(ctx, ref.Ref) + if errors.IsNotFound(err) { //nolint:gocritic + oldValue = sha.Nil + } else if err != nil { + return fmt.Errorf("failed to get current value of reference %q: %w", ref.Ref, err) + } else { + oldValue = val + } + + oldValueKnown = true } + + if newValue.IsEmpty() { + // don't break existing interface - user calls with empty value to delete the ref. + newValue = sha.Nil + } + + if oldValueKnown && oldValue == newValue { + // skip the unchanged refs + continue + } + + u.refs = append(u.refs, ReferenceUpdate{ + Ref: ref.Ref, + Old: oldValue, + New: newValue, + }) } - u.state = stateInitNew - u.oldValue = oldValue - - return nil -} - -func (u *RefUpdater) InitNew(_ context.Context, newValue sha.SHA) error { - if u == nil { - return nil + if len(refs) > 0 && len(u.refs) == 0 { + return errors.New("updating zero references") } - if u.state != stateInitNew { - return fmt.Errorf("invalid operation order: init new requires state=%s, current state=%s", - stateInitNew, u.state) - } - - if newValue.IsEmpty() { - // don't break existing interface - user calls with empty value to delete the ref. - newValue = sha.Nil - } + sort.Slice(u.refs, func(i, j int) bool { + return u.refs[i].Ref < u.refs[j].Ref + }) u.state = statePre - u.newValue = newValue return nil } @@ -185,23 +186,13 @@ func (u *RefUpdater) Pre(ctx context.Context, alternateDirs ...string) error { statePre, u.state) } - // fail in case someone tries to delete a reference that doesn't exist. - if u.oldValue.IsEmpty() && u.newValue.IsNil() { - return errors.NotFound("reference %q not found", u.ref) - } - - if u.oldValue.IsNil() && u.newValue.IsNil() { - return fmt.Errorf("provided values cannot be both empty") + if len(u.refs) == 0 { + u.state = stateUpdate + return nil } out, err := u.hookClient.PreReceive(ctx, PreReceiveInput{ - RefUpdates: []ReferenceUpdate{ - { - Ref: u.ref, - Old: u.oldValue, - New: u.newValue, - }, - }, + RefUpdates: u.refs, Environment: Environment{ AlternateObjectDirs: alternateDirs, }, @@ -228,22 +219,33 @@ func (u *RefUpdater) UpdateRef(ctx context.Context) error { stateUpdate, u.state) } - cmd := command.New("update-ref") - if u.newValue.IsNil() { - cmd.Add(command.WithFlag("-d", u.ref)) - } else { - cmd.Add(command.WithArg(u.ref, u.newValue.String())) + if len(u.refs) == 0 { + u.state = statePost + return nil } - cmd.Add(command.WithArg(u.oldValue.String())) + input := bytes.NewBuffer(nil) + for _, ref := range u.refs { + switch { + case ref.New.IsNil(): + _, _ = input.WriteString(fmt.Sprintf("delete %s\000%s\000", ref.Ref, ref.Old)) + case ref.Old.IsNil(): + _, _ = input.WriteString(fmt.Sprintf("create %s\000%s\000", ref.Ref, ref.New)) + default: + _, _ = input.WriteString(fmt.Sprintf("update %s\000%s\000%s\000", ref.Ref, ref.New, ref.Old)) + } + } - if err := cmd.Run(ctx, command.WithDir(u.repoPath)); err != nil { + input.WriteString("commit\000") + + cmd := command.New("update-ref", command.WithFlag("--stdin"), command.WithFlag("-z")) + if err := cmd.Run(ctx, command.WithStdin(input), command.WithDir(u.repoPath)); err != nil { msg := err.Error() if strings.Contains(msg, "reference already exists") { return errors.Conflict("reference already exists") } - return fmt.Errorf("update of ref %q from %q to %q failed: %w", u.ref, u.oldValue, u.newValue, err) + return fmt.Errorf("update of references %v failed: %w", u.refs, err) } u.state = statePost @@ -258,14 +260,13 @@ func (u *RefUpdater) Post(ctx context.Context, alternateDirs ...string) error { statePost, u.state) } + if len(u.refs) == 0 { + u.state = stateDone + return nil + } + out, err := u.hookClient.PostReceive(ctx, PostReceiveInput{ - RefUpdates: []ReferenceUpdate{ - { - Ref: u.ref, - Old: u.oldValue, - New: u.newValue, - }, - }, + RefUpdates: u.refs, Environment: Environment{ AlternateObjectDirs: alternateDirs, }, @@ -282,16 +283,16 @@ func (u *RefUpdater) Post(ctx context.Context, alternateDirs ...string) error { return nil } -func (u *RefUpdater) getRef(ctx context.Context) (sha.SHA, error) { +func (u *RefUpdater) getRef(ctx context.Context, ref string) (sha.SHA, error) { cmd := command.New("show-ref", command.WithFlag("--verify"), command.WithFlag("-s"), - command.WithArg(u.ref), + command.WithArg(ref), ) output := &bytes.Buffer{} err := cmd.Run(ctx, command.WithDir(u.repoPath), command.WithStdout(output)) if cErr := command.AsError(err); cErr != nil && cErr.IsExitCode(128) && cErr.IsInvalidRefErr() { - return sha.None, errors.NotFound("reference %q not found", u.ref) + return sha.None, errors.NotFound("reference %q not found", ref) } if err != nil { diff --git a/git/merge.go b/git/merge.go index f206b4481..a6f9ae115 100644 --- a/git/merge.go +++ b/git/merge.go @@ -26,6 +26,7 @@ import ( "github.com/harness/gitness/git/merge" "github.com/harness/gitness/git/parser" "github.com/harness/gitness/git/sha" + "github.com/harness/gitness/git/sharedrepo" ) // MergeParams is input structure object for merging operation. @@ -56,8 +57,7 @@ type MergeParams struct { // (optional, default: committer date) AuthorDate *time.Time - RefType enum.RefType - RefName string + Refs []RefUpdate // HeadExpectedSHA is commit sha on the head branch, if HeadExpectedSHA is older // than the HeadBranch latest sha then merge will fail. @@ -69,6 +69,19 @@ type MergeParams struct { Method enum.MergeMethod } +type RefUpdate struct { + // Name is the full name of the reference. + Name string + + // Old is the expected current value of the reference. + // If it's empty, the old value of the reference can be any value. + Old sha.SHA + + // New is the desired value for the reference. + // If it's empty, the reference would be set to the resulting commit SHA of the merge. + New sha.SHA +} + func (p *MergeParams) Validate() error { if err := p.WriteParams.Validate(); err != nil { return err @@ -82,9 +95,12 @@ func (p *MergeParams) Validate() error { return errors.InvalidArgument("head branch is mandatory") } - if p.RefType != enum.RefTypeUndefined && p.RefName == "" { - return errors.InvalidArgument("ref name has to be provided if type is defined") + for _, ref := range p.Refs { + if ref.Name == "" { + return errors.InvalidArgument("ref name has to be provided") + } } + return nil } @@ -153,27 +169,6 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, panic("unsupported merge method") } - // set up the target reference - - var refPath string - var refOldValue sha.SHA - - if params.RefType != enum.RefTypeUndefined { - refPath, err = GetRefPath(params.RefName, params.RefType) - if err != nil { - return MergeOutput{}, fmt.Errorf( - "failed to generate full reference for type '%s' and name '%s' for merge operation: %w", - params.RefType, params.RefName, err) - } - - refOldValue, err = s.git.GetFullCommitID(ctx, repoPath, refPath) - if errors.IsNotFound(err) { - refOldValue = sha.Nil - } else if err != nil { - return MergeOutput{}, fmt.Errorf("failed to resolve %q: %w", refPath, err) - } - } - // find the commit SHAs baseCommitSHA := params.BaseSHA @@ -259,35 +254,66 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, message = parser.CleanUpWhitespace(params.Message) } - // merge + // create merge commit and update the references - var refUpdater *hook.RefUpdater - - if params.RefType != enum.RefTypeUndefined { - refUpdater, err = hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, refPath) - if err != nil { - return MergeOutput{}, errors.Internal(err, "failed to create ref updater object") - } - - if err := refUpdater.InitOld(ctx, refOldValue); err != nil { - return MergeOutput{}, errors.Internal(err, "failed to set old reference value for ref updater") - } + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) + if err != nil { + return MergeOutput{}, fmt.Errorf("failed to create reference updater: %w", err) } - mergeCommitSHA, conflicts, err := mergeFunc( - ctx, - refUpdater, - repoPath, s.tmpDir, - &author, &committer, - message, - mergeBaseCommitSHA, baseCommitSHA, headCommitSHA) + var mergeCommitSHA sha.SHA + var conflicts []string + + err = sharedrepo.Run(ctx, refUpdater, s.tmpDir, repoPath, func(s *sharedrepo.SharedRepo) error { + mergeCommitSHA, conflicts, err = mergeFunc( + ctx, + s, + merge.Params{ + Author: &author, + Committer: &committer, + Message: message, + MergeBaseSHA: mergeBaseCommitSHA, + TargetSHA: baseCommitSHA, + SourceSHA: headCommitSHA, + }) + if err != nil { + return fmt.Errorf("failed to create merge commit: %w", err) + } + + if mergeCommitSHA.IsEmpty() || len(conflicts) > 0 { + return refUpdater.Init(ctx, nil) // update nothing + } + + refUpdates := make([]hook.ReferenceUpdate, len(params.Refs)) + for i, ref := range params.Refs { + oldValue := ref.Old + newValue := ref.New + + if newValue.IsEmpty() { // replace all empty new values to the result of the merge + newValue = mergeCommitSHA + } + + refUpdates[i] = hook.ReferenceUpdate{ + Ref: ref.Name, + Old: oldValue, + New: newValue, + } + } + + err = refUpdater.Init(ctx, refUpdates) + if err != nil { + return fmt.Errorf("failed to init values of references (%v): %w", refUpdates, err) + } + + return nil + }) if errors.IsConflict(err) { return MergeOutput{}, fmt.Errorf("failed to merge %q to %q in %q using the %q merge method: %w", params.HeadBranch, params.BaseBranch, params.RepoUID, mergeMethod, err) } if err != nil { - return MergeOutput{}, errors.Internal(err, "failed to merge %q to %q in %q using the %q merge method", - params.HeadBranch, params.BaseBranch, params.RepoUID, mergeMethod) + return MergeOutput{}, fmt.Errorf("failed to merge %q to %q in %q using the %q merge method: %w", + params.HeadBranch, params.BaseBranch, params.RepoUID, mergeMethod, err) } if len(conflicts) > 0 { return MergeOutput{ diff --git a/git/merge/merge.go b/git/merge/merge.go index 0749afd3d..85dd9d2cd 100644 --- a/git/merge/merge.go +++ b/git/merge/merge.go @@ -20,222 +20,175 @@ import ( "github.com/harness/gitness/errors" "github.com/harness/gitness/git/api" - "github.com/harness/gitness/git/hook" "github.com/harness/gitness/git/sha" "github.com/harness/gitness/git/sharedrepo" "github.com/rs/zerolog/log" ) -var ( - // errConflict is used to error out of sharedrepo Run method without erroring out of merge in case of conflicts. - errConflict = errors.New("conflict") -) +type Params struct { + Author, Committer *api.Signature + Message string + MergeBaseSHA, TargetSHA, SourceSHA sha.SHA +} // Func represents a merge method function. The concrete merge implementation functions must have this signature. type Func func( ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - author, committer *api.Signature, - message string, - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, + s *sharedrepo.SharedRepo, + params Params, ) (mergeSHA sha.SHA, conflicts []string, err error) // Merge merges two the commits (targetSHA and sourceSHA) using the Merge method. func Merge( ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - author, committer *api.Signature, - message string, - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, + s *sharedrepo.SharedRepo, + params Params, ) (mergeSHA sha.SHA, conflicts []string, err error) { - return mergeInternal(ctx, - refUpdater, - repoPath, tmpDir, - author, committer, - message, - mergeBaseSHA, targetSHA, sourceSHA, - false) + return mergeInternal(ctx, s, params, false) } // Squash merges two the commits (targetSHA and sourceSHA) using the Squash method. func Squash( ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - author, committer *api.Signature, - message string, - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, + s *sharedrepo.SharedRepo, + params Params, ) (mergeSHA sha.SHA, conflicts []string, err error) { - return mergeInternal(ctx, - refUpdater, - repoPath, tmpDir, - author, committer, - message, - mergeBaseSHA, targetSHA, sourceSHA, - true) + return mergeInternal(ctx, s, params, true) } // mergeInternal is internal implementation of merge used for Merge and Squash methods. -func mergeInternal( - ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - author, committer *api.Signature, - message string, - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, +func mergeInternal(ctx context.Context, + s *sharedrepo.SharedRepo, + params Params, squash bool, ) (mergeSHA sha.SHA, conflicts []string, err error) { - err = sharedrepo.Run(ctx, refUpdater, tmpDir, repoPath, func(s *sharedrepo.SharedRepo) error { - var err error + mergeBaseSHA := params.MergeBaseSHA + targetSHA := params.TargetSHA + sourceSHA := params.SourceSHA - var treeSHA sha.SHA + var treeSHA sha.SHA - treeSHA, conflicts, err = s.MergeTree(ctx, mergeBaseSHA, targetSHA, sourceSHA) - if err != nil { - return fmt.Errorf("merge tree failed: %w", err) - } + treeSHA, conflicts, err = s.MergeTree(ctx, mergeBaseSHA, targetSHA, sourceSHA) + if err != nil { + return sha.None, nil, fmt.Errorf("merge tree failed: %w", err) + } - if len(conflicts) > 0 { - return errConflict - } + if len(conflicts) > 0 { + return sha.None, conflicts, nil + } - parents := make([]sha.SHA, 0, 2) - parents = append(parents, targetSHA) - if !squash { - parents = append(parents, sourceSHA) - } + parents := make([]sha.SHA, 0, 2) + parents = append(parents, targetSHA) + if !squash { + parents = append(parents, sourceSHA) + } - mergeSHA, err = s.CommitTree(ctx, author, committer, treeSHA, message, false, parents...) - if err != nil { - return fmt.Errorf("commit tree failed: %w", err) - } - - if err := refUpdater.InitNew(ctx, mergeSHA); err != nil { - return fmt.Errorf("refUpdater.InitNew failed: %w", err) - } - - return nil - }) - if err != nil && !errors.Is(err, errConflict) { - return sha.None, nil, fmt.Errorf("merge method=merge squash=%t: %w", squash, err) + mergeSHA, err = s.CommitTree(ctx, params.Author, params.Committer, treeSHA, params.Message, false, parents...) + if err != nil { + return sha.None, nil, fmt.Errorf("commit tree failed: %w", err) } return mergeSHA, conflicts, nil } // Rebase merges two the commits (targetSHA and sourceSHA) using the Rebase method. +// Commit author isn't used here - it's copied from every commit. +// Commit message isn't used here // //nolint:gocognit // refactor if needed. func Rebase( ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - _, committer *api.Signature, // commit author isn't used here - it's copied from every commit - _ string, // commit message isn't used here - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, + s *sharedrepo.SharedRepo, + params Params, ) (mergeSHA sha.SHA, conflicts []string, err error) { - err = sharedrepo.Run(ctx, refUpdater, tmpDir, repoPath, func(s *sharedrepo.SharedRepo) error { - sourceSHAs, err := s.CommitSHAsForRebase(ctx, mergeBaseSHA, sourceSHA) - if err != nil { - return fmt.Errorf("failed to find commit list in rebase merge: %w", err) - } + mergeBaseSHA := params.MergeBaseSHA + targetSHA := params.TargetSHA + sourceSHA := params.SourceSHA - lastCommitSHA := targetSHA - lastTreeSHA, err := s.GetTreeSHA(ctx, targetSHA.String()) - if err != nil { - return fmt.Errorf("failed to get tree sha for target: %w", err) - } - - for _, commitSHA := range sourceSHAs { - var treeSHA sha.SHA - - commitInfo, err := api.GetCommit(ctx, s.Directory(), commitSHA.String()) - if err != nil { - return fmt.Errorf("failed to get commit data in rebase merge: %w", err) - } - - // rebase merge preserves the commit author (and date) and the commit message, but changes the committer. - author := &commitInfo.Author - message := commitInfo.Title - if commitInfo.Message != "" { - message += "\n\n" + commitInfo.Message - } - - var mergeTreeMergeBaseSHA sha.SHA - if len(commitInfo.ParentSHAs) > 0 { - // use parent of commit as merge base to only apply changes introduced by commit. - // See example usage of when --merge-base was introduced: - // https://github.com/git/git/commit/66265a693e8deb3ab86577eb7f69940410044081 - // - // NOTE: CommitSHAsForRebase only returns non-merge commits. - mergeTreeMergeBaseSHA = commitInfo.ParentSHAs[0] - } - - treeSHA, conflicts, err = s.MergeTree(ctx, mergeTreeMergeBaseSHA, lastCommitSHA, commitSHA) - if err != nil { - return fmt.Errorf("failed to merge tree in rebase merge: %w", err) - } - if len(conflicts) > 0 { - return errConflict - } - - // Drop any commit which after being rebased would be empty. - // There's two cases in which that can happen: - // 1. Empty commit. - // Github is dropping empty commits, so we'll do the same. - // 2. The changes of the commit already exist on the target branch. - // Git's `git rebase` is dropping such commits on default (and so does Github) - // https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---emptydropkeepask - if treeSHA.Equal(lastTreeSHA) { - log.Ctx(ctx).Debug().Msgf("skipping commit %s as it's empty after rebase", commitSHA) - continue - } - - lastCommitSHA, err = s.CommitTree(ctx, author, committer, treeSHA, message, false, lastCommitSHA) - if err != nil { - return fmt.Errorf("failed to commit tree in rebase merge: %w", err) - } - lastTreeSHA = treeSHA - } - - if err := refUpdater.InitNew(ctx, lastCommitSHA); err != nil { - return fmt.Errorf("refUpdater.InitNew failed: %w", err) - } - - mergeSHA = lastCommitSHA - - return nil - }) - if err != nil && !errors.Is(err, errConflict) { - return sha.None, nil, fmt.Errorf("merge method=rebase: %w", err) + sourceSHAs, err := s.CommitSHAsForRebase(ctx, mergeBaseSHA, sourceSHA) + if err != nil { + return sha.None, nil, fmt.Errorf("failed to find commit list in rebase merge: %w", err) } - return mergeSHA, conflicts, nil + lastCommitSHA := targetSHA + lastTreeSHA, err := s.GetTreeSHA(ctx, targetSHA.String()) + if err != nil { + return sha.None, nil, fmt.Errorf("failed to get tree sha for target: %w", err) + } + + for _, commitSHA := range sourceSHAs { + var treeSHA sha.SHA + + commitInfo, err := api.GetCommit(ctx, s.Directory(), commitSHA.String()) + if err != nil { + return sha.None, nil, fmt.Errorf("failed to get commit data in rebase merge: %w", err) + } + + // rebase merge preserves the commit author (and date) and the commit message, but changes the committer. + author := &commitInfo.Author + message := commitInfo.Title + if commitInfo.Message != "" { + message += "\n\n" + commitInfo.Message + } + + var mergeTreeMergeBaseSHA sha.SHA + if len(commitInfo.ParentSHAs) > 0 { + // use parent of commit as merge base to only apply changes introduced by commit. + // See example usage of when --merge-base was introduced: + // https://github.com/git/git/commit/66265a693e8deb3ab86577eb7f69940410044081 + // + // NOTE: CommitSHAsForRebase only returns non-merge commits. + mergeTreeMergeBaseSHA = commitInfo.ParentSHAs[0] + } + + treeSHA, conflicts, err = s.MergeTree(ctx, mergeTreeMergeBaseSHA, lastCommitSHA, commitSHA) + if err != nil { + return sha.None, nil, fmt.Errorf("failed to merge tree in rebase merge: %w", err) + } + if len(conflicts) > 0 { + return sha.None, conflicts, nil + } + + // Drop any commit which after being rebased would be empty. + // There's two cases in which that can happen: + // 1. Empty commit. + // Github is dropping empty commits, so we'll do the same. + // 2. The changes of the commit already exist on the target branch. + // Git's `git rebase` is dropping such commits on default (and so does Github) + // https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---emptydropkeepask + if treeSHA.Equal(lastTreeSHA) { + log.Ctx(ctx).Debug().Msgf("skipping commit %s as it's empty after rebase", commitSHA) + continue + } + + lastCommitSHA, err = s.CommitTree(ctx, author, params.Committer, treeSHA, message, false, lastCommitSHA) + if err != nil { + return sha.None, nil, fmt.Errorf("failed to commit tree in rebase merge: %w", err) + } + lastTreeSHA = treeSHA + } + + mergeSHA = lastCommitSHA + + return mergeSHA, nil, nil } -// FastForward points the is internal implementation of merge used for Merge and Squash methods. +// FastForward points the is internal implementation of merge used for Merge and Squash methods. +// Commit author and committer aren't used here. Commit message isn't used here. func FastForward( - ctx context.Context, - refUpdater *hook.RefUpdater, - repoPath, tmpDir string, - _, _ *api.Signature, // commit author and committer aren't used here - _ string, // commit message isn't used here - mergeBaseSHA, targetSHA, sourceSHA sha.SHA, + _ context.Context, + _ *sharedrepo.SharedRepo, + params Params, ) (mergeSHA sha.SHA, conflicts []string, err error) { + mergeBaseSHA := params.MergeBaseSHA + targetSHA := params.TargetSHA + sourceSHA := params.SourceSHA + if targetSHA != mergeBaseSHA { return sha.None, nil, errors.Conflict("Target branch has diverged from the source branch. Fast-forward not possible.") } - err = sharedrepo.Run(ctx, refUpdater, tmpDir, repoPath, func(*sharedrepo.SharedRepo) error { - return refUpdater.InitNew(ctx, sourceSHA) - }) - if err != nil { - return sha.None, nil, fmt.Errorf("merge method=fast-forward: %w", err) - } - return sourceSHA, nil, nil } diff --git a/git/operations.go b/git/operations.go index c93fba6a3..e3ddc2712 100644 --- a/git/operations.go +++ b/git/operations.go @@ -144,7 +144,7 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C refOldSHA = commit.SHA } - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, branchRef) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { return CommitFilesResponse{}, fmt.Errorf("failed to create ref updater: %w", err) } @@ -222,7 +222,13 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C refNewSHA = commitSHA - if err := refUpdater.Init(ctx, refOldSHA, refNewSHA); err != nil { + ref := hook.ReferenceUpdate{ + Ref: branchRef, + Old: refOldSHA, + New: refNewSHA, + } + + if err := refUpdater.Init(ctx, []hook.ReferenceUpdate{ref}); err != nil { return fmt.Errorf("failed to init ref updater old=%s new=%s: %w", refOldSHA, refNewSHA, err) } diff --git a/git/ref.go b/git/ref.go index 9afff459c..24b31d787 100644 --- a/git/ref.go +++ b/git/ref.go @@ -89,15 +89,15 @@ func (s *Service) UpdateRef(ctx context.Context, params UpdateRefParams) error { reference, err := GetRefPath(params.Name, params.Type) if err != nil { - return fmt.Errorf("UpdateRef: failed to fetch reference '%s': %w", params.Name, err) + return fmt.Errorf("failed to create reference '%s': %w", params.Name, err) } - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, reference) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { - return fmt.Errorf("UpdateRef: failed to create ref updater: %w", err) + return fmt.Errorf("failed to create ref updater: %w", err) } - if err := refUpdater.Do(ctx, params.OldValue, params.NewValue); err != nil { + if err := refUpdater.DoOne(ctx, reference, params.OldValue, params.NewValue); err != nil { return fmt.Errorf("failed to update ref: %w", err) } @@ -122,8 +122,6 @@ func GetRefPath(refName string, refType enum.RefType) (string, error) { return refPullReqPrefix + refName + refPullReqHeadSuffix, nil case enum.RefTypePullReqMerge: return refPullReqPrefix + refName + refPullReqMergeSuffix, nil - case enum.RefTypeUndefined: - fallthrough default: return "", errors.InvalidArgument("provided reference type '%s' is invalid", refType) } diff --git a/git/tag.go b/git/tag.go index 5f28abfb2..a2be474f7 100644 --- a/git/tag.go +++ b/git/tag.go @@ -278,7 +278,7 @@ func (s *Service) CreateCommitTag(ctx context.Context, params *CreateCommitTagPa // ref updater - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, tagRef) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { return nil, fmt.Errorf("failed to create ref updater to create the tag: %w", err) } @@ -295,7 +295,13 @@ func (s *Service) CreateCommitTag(ctx context.Context, params *CreateCommitTagPa return fmt.Errorf("failed to read annotated tag after creation: %w", err) } - if err := refUpdater.Init(ctx, sha.Nil, tag.Sha); err != nil { + ref := hook.ReferenceUpdate{ + Ref: tagRef, + Old: sha.Nil, + New: tag.Sha, + } + + if err := refUpdater.Init(ctx, []hook.ReferenceUpdate{ref}); err != nil { return fmt.Errorf("failed to init ref updater: %w", err) } @@ -337,14 +343,15 @@ func (s *Service) DeleteTag(ctx context.Context, params *DeleteTagParams) error } repoPath := getFullPathForRepo(s.reposRoot, params.RepoUID) - tagRef := api.GetReferenceFromTagName(params.Name) - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, tagRef) + refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath) if err != nil { return fmt.Errorf("failed to create ref updater to delete the tag: %w", err) } - err = refUpdater.Do(ctx, sha.None, sha.Nil) // delete whatever is there + tagRef := api.GetReferenceFromTagName(params.Name) + + err = refUpdater.DoOne(ctx, tagRef, sha.None, sha.Nil) // delete whatever is there if errors.IsNotFound(err) { return errors.NotFound("tag %q does not exist", params.Name) }