Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 500 Error with branch and tag sharing the same name #16040

Merged
merged 3 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions modules/git/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,20 @@ func (err *ErrPushRejected) GenerateMessage() {
}
err.Message = strings.TrimSpace(messageBuilder.String())
}

// ErrMoreThanOne represents an error if pull request fails when there are more than one sources (branch, tag) with the same name
type ErrMoreThanOne struct {
StdOut string
StdErr string
Err error
}

// IsErrMoreThanOne checks if an error is a ErrMoreThanOne
func IsErrMoreThanOne(err error) bool {
_, ok := err.(*ErrMoreThanOne)
return ok
}

func (err *ErrMoreThanOne) Error() string {
return fmt.Sprintf("ErrMoreThanOne Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
}
7 changes: 7 additions & 0 deletions modules/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ func Push(repoPath string, opts PushOptions) error {
}
err.GenerateMessage()
return err
} else if strings.Contains(errbuf.String(), "matches more than one") {
err := &ErrMoreThanOne{
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
}
return err
}
}

Expand Down
14 changes: 13 additions & 1 deletion services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ func checkIfPRContentChanged(pr *models.PullRequest, oldCommitID, newCommitID st
// corresponding branches of base repository.
// FIXME: Only push branches that are actually updates?
func PushToBaseRepo(pr *models.PullRequest) (err error) {
return pushToBaseRepoHelper(pr, "")
}

func pushToBaseRepoHelper(pr *models.PullRequest, prefixHeadBranch string) (err error) {
log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitRefName())

if err := pr.LoadHeadRepo(); err != nil {
Expand All @@ -414,7 +418,7 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {

if err := git.Push(headRepoPath, git.PushOptions{
Remote: baseRepoPath,
Branch: pr.HeadBranch + ":" + gitRefName,
Branch: prefixHeadBranch + pr.HeadBranch + ":" + gitRefName,
Force: true,
// Use InternalPushingEnvironment here because we know that pre-receive and post-receive do not run on a refs/pulls/...
Env: models.InternalPushingEnvironment(pr.Issue.Poster, pr.BaseRepo),
Expand All @@ -427,6 +431,14 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {
rejectErr := err.(*git.ErrPushRejected)
log.Info("Unable to push PR head for %s#%d (%-v:%s) due to rejection:\nStdout: %s\nStderr: %s\nError: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err)
return err
} else if git.IsErrMoreThanOne(err) {
log.Info("Retrying to push with refs/heads/%s", pr.HeadBranch)
6543 marked this conversation as resolved.
Show resolved Hide resolved
if prefixHeadBranch != "" {
log.Info("Can't push with refs/heads/%s", pr.HeadBranch)
6543 marked this conversation as resolved.
Show resolved Hide resolved
return err
}
err = pushToBaseRepoHelper(pr, "refs/heads/")
6543 marked this conversation as resolved.
Show resolved Hide resolved
return err
}
log.Error("Unable to push PR head for %s#%d (%-v:%s) due to Error: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, gitRefName, err)
return fmt.Errorf("Push: %s:%s %s:%s %v", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), gitRefName, err)
Expand Down