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 <git@nroo.de>
Reviewed-on: https://gitea.com/gitea/tea/pulls/300
Reviewed-by: 6543 <6543@obermui.de>
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-Authored-By: Norwin <noerw@noreply.gitea.io>
Co-Committed-By: Norwin <noerw@noreply.gitea.io>

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
+// TeaDeleteLocalBranch removes the given branch locally
-// not empty deletes it at it's remote repo.
+func (r TeaRepo) TeaDeleteLocalBranch(branch *git_config.Branch) error {
-func (r TeaRepo) TeaDeleteBranch(branch *git_config.Branch, remoteBranch string, auth git_transport.AuthMethod) 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))
+	return r.Storer.RemoveReference(git_plumbing.NewBranchReferenceName(branch.Name))
-	if err != nil {
+}
-		return err
-	}
 
-	if remoteBranch != "" {
+// TeaDeleteRemoteBranch removes the given branch on the given remote via git protocol
-		// delete remote branch via git protocol:
+func (r TeaRepo) TeaDeleteRemoteBranch(remoteName, remoteBranch string, auth git_transport.AuthMethod) error {
-		// an empty source in the refspec means remote deletion to git 🙃
+	// delete remote branch via git protocol:
-		refspec := fmt.Sprintf(":%s", git_plumbing.NewBranchReferenceName(remoteBranch))
+	// an empty source in the refspec means remote deletion to git 🙃
-		err = r.Push(&git.PushOptions{
+	refspec := fmt.Sprintf(":%s", git_plumbing.NewBranchReferenceName(remoteBranch))
-			RemoteName: branch.Remote,
+	return r.Push(&git.PushOptions{
-			RefSpecs:   []git_config.RefSpec{git_config.RefSpec(refspec)},
+		RemoteName: remoteName,
-			Prune:      true,
+		RefSpecs:   []git_config.RefSpec{git_config.RefSpec(refspec)},
-			Auth:       auth,
+		Prune:      true,
-		})
+		Auth:       auth,
-	}
+	})
-
-	return err
 }
 
 // TeaFindBranchBySha returns a branch that is at the the given SHA and syncs to the

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 {

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/<idx>/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)
+	fmt.Printf("Deleting local branch %s\n", branch.Name)
-	url, err := r.TeaRemoteURL(branch.Remote)
+	err = r.TeaDeleteLocalBranch(branch)
 	if err != nil {
 		return err
 	}
-	auth, err := local_git.GetAuthForURL(url, login.Token, login.SSHKey, callback)
+
-	if err != nil {
+	if !remoteDeleted && pr.Head.Repository.Permissions.Push {
-		return err
+		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
 }