From 8b588f5313e36ec09ba63d6c33ab214f18431e5f Mon Sep 17 00:00:00 2001 From: Norwin Date: Thu, 17 Dec 2020 22:00:16 +0800 Subject: [PATCH] make PR workflow helpers more robust (#300) improve handling of remote deleted branches split git.TeaDeleteBranch only delete remote branch if we have permission add missing err check Co-authored-by: Norwin Roosen Reviewed-on: https://gitea.com/gitea/tea/pulls/300 Reviewed-by: 6543 <6543@obermui.de> Reviewed-by: Lunny Xiao Co-Authored-By: Norwin Co-Committed-By: Norwin --- modules/git/branch.go | 35 ++++++++++++++------------------ modules/task/pull_checkout.go | 4 ++++ modules/task/pull_clean.go | 38 ++++++++++++++++++++++++++--------- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/modules/git/branch.go b/modules/git/branch.go index 7773843..f359868 100644 --- a/modules/git/branch.go +++ b/modules/git/branch.go @@ -47,33 +47,28 @@ func (r TeaRepo) TeaCheckout(branchName string) error { return tree.Checkout(&git.CheckoutOptions{Branch: localBranchRefName}) } -// TeaDeleteBranch removes the given branch locally, and if `remoteBranch` is -// not empty deletes it at it's remote repo. -func (r TeaRepo) TeaDeleteBranch(branch *git_config.Branch, remoteBranch string, auth git_transport.AuthMethod) error { +// TeaDeleteLocalBranch removes the given branch locally +func (r TeaRepo) TeaDeleteLocalBranch(branch *git_config.Branch) error { err := r.DeleteBranch(branch.Name) // if the branch is not found that's ok, as .git/config may have no entry if // no remote tracking branch is configured for it (eg push without -u flag) if err != nil && err.Error() != "branch not found" { return err } - err = r.Storer.RemoveReference(git_plumbing.NewBranchReferenceName(branch.Name)) - if err != nil { - return err - } + return r.Storer.RemoveReference(git_plumbing.NewBranchReferenceName(branch.Name)) +} - if remoteBranch != "" { - // delete remote branch via git protocol: - // an empty source in the refspec means remote deletion to git 🙃 - refspec := fmt.Sprintf(":%s", git_plumbing.NewBranchReferenceName(remoteBranch)) - err = r.Push(&git.PushOptions{ - RemoteName: branch.Remote, - RefSpecs: []git_config.RefSpec{git_config.RefSpec(refspec)}, - Prune: true, - Auth: auth, - }) - } - - return err +// TeaDeleteRemoteBranch removes the given branch on the given remote via git protocol +func (r TeaRepo) TeaDeleteRemoteBranch(remoteName, remoteBranch string, auth git_transport.AuthMethod) error { + // delete remote branch via git protocol: + // an empty source in the refspec means remote deletion to git 🙃 + refspec := fmt.Sprintf(":%s", git_plumbing.NewBranchReferenceName(remoteBranch)) + return r.Push(&git.PushOptions{ + RemoteName: remoteName, + RefSpecs: []git_config.RefSpec{git_config.RefSpec(refspec)}, + Prune: true, + Auth: auth, + }) } // TeaFindBranchBySha returns a branch that is at the the given SHA and syncs to the diff --git a/modules/task/pull_checkout.go b/modules/task/pull_checkout.go index 92262a6..fda9ec3 100644 --- a/modules/task/pull_checkout.go +++ b/modules/task/pull_checkout.go @@ -27,6 +27,10 @@ func PullCheckout(login *config.Login, repoOwner, repoName string, index int64, if err != nil { return err } + remoteDeleted := pr.Head.Ref == fmt.Sprintf("refs/pull/%d/head", pr.Index) + if remoteDeleted { + return fmt.Errorf("Can't checkout: remote head branch was already deleted") + } remoteURL := pr.Head.Repository.CloneURL if len(login.SSHKey) != 0 { diff --git a/modules/task/pull_clean.go b/modules/task/pull_clean.go index fc7e20a..775748b 100644 --- a/modules/task/pull_clean.go +++ b/modules/task/pull_clean.go @@ -19,6 +19,9 @@ func PullClean(login *config.Login, repoOwner, repoName string, index int64, ign client := login.Client() repo, _, err := client.GetRepo(repoOwner, repoName) + if err != nil { + return err + } defaultBranch := repo.DefaultBranch if len(defaultBranch) == 0 { defaultBranch = "master" @@ -33,7 +36,13 @@ func PullClean(login *config.Login, repoOwner, repoName string, index int64, ign return fmt.Errorf("PR is still open, won't delete branches") } - // IDEA: abort if PR.Head.Repository.CloneURL does not match login.URL? + // if remote head branch is already deleted, pr.Head.Ref points to "pulls//head" + remoteBranch := pr.Head.Ref + remoteDeleted := remoteBranch == fmt.Sprintf("refs/pull/%d/head", pr.Index) + if remoteDeleted { + remoteBranch = pr.Head.Name // this still holds the original branch name + fmt.Printf("Remote branch '%s' already deleted.\n", remoteBranch) + } r, err := local_git.RepoForWorkdir() if err != nil { @@ -43,7 +52,7 @@ func PullClean(login *config.Login, repoOwner, repoName string, index int64, ign // find a branch with matching sha or name, that has a remote matching the repo url var branch *git_config.Branch if ignoreSHA { - branch, err = r.TeaFindBranchByName(pr.Head.Ref, pr.Head.Repository.CloneURL) + branch, err = r.TeaFindBranchByName(remoteBranch, pr.Head.Repository.CloneURL) } else { branch, err = r.TeaFindBranchBySha(pr.Head.Sha, pr.Head.Repository.CloneURL) } @@ -52,12 +61,12 @@ func PullClean(login *config.Login, repoOwner, repoName string, index int64, ign } if branch == nil { if ignoreSHA { - return fmt.Errorf("Remote branch %s not found in local repo", pr.Head.Ref) + return fmt.Errorf("Remote branch %s not found in local repo", remoteBranch) } return fmt.Errorf(`Remote branch %s not found in local repo. Either you don't track this PR, or the local branch has diverged from the remote. If you still want to continue & are sure you don't loose any important commits, -call me again with the --ignore-sha flag`, pr.Head.Ref) +call me again with the --ignore-sha flag`, remoteBranch) } // prepare deletion of local branch: @@ -73,14 +82,23 @@ call me again with the --ignore-sha flag`, pr.Head.Ref) } // remove local & remote branch - fmt.Printf("Deleting local branch %s and remote branch %s\n", branch.Name, pr.Head.Ref) - url, err := r.TeaRemoteURL(branch.Remote) + fmt.Printf("Deleting local branch %s\n", branch.Name) + err = r.TeaDeleteLocalBranch(branch) if err != nil { return err } - auth, err := local_git.GetAuthForURL(url, login.Token, login.SSHKey, callback) - if err != nil { - return err + + if !remoteDeleted && pr.Head.Repository.Permissions.Push { + fmt.Printf("Deleting remote branch %s\n", remoteBranch) + url, err := r.TeaRemoteURL(branch.Remote) + if err != nil { + return err + } + auth, err := local_git.GetAuthForURL(url, login.Token, login.SSHKey, callback) + if err != nil { + return err + } + err = r.TeaDeleteRemoteBranch(branch.Remote, remoteBranch, auth) } - return r.TeaDeleteBranch(branch, pr.Head.Ref, auth) + return err }