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: update FileWorkspace logic to use pull request refs only when PR number is specified #4851

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
6 changes: 6 additions & 0 deletions server/events/vcs/instrumented_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,12 @@ func (c *InstrumentedClient) PullIsMergeable(logger logging.SimpleLogging, repo
}

func (c *InstrumentedClient) UpdateStatus(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {
// If the plan isn't coming from a pull request,
// don't attempt to update the status.
if pull.Num == 0 {
return nil
}

scope := c.StatsScope.SubScope("update_status")
scope = SetGitScopeTags(scope, repo.FullName, pull.Num)

Expand Down
13 changes: 7 additions & 6 deletions server/events/working_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ type FileWorkspace struct {
// TestingOverrideBaseCloneURL can be used during testing to override the
// URL of the base repo to be cloned. If it's empty then we clone normally.
TestingOverrideBaseCloneURL string
// GithubAppEnabled is true if we should fetch the ref "pull/PR_NUMBER/head"
// from the "origin" remote. If this is false, we fetch "+refs/heads/$HEAD_BRANCH"
// from the "head" remote.
// GithubAppEnabled is true and a PR number is supplied, we should fetch
// the ref "pull/PR_NUMBER/head" from the "origin" remote. If this is false,
// we fetch "+refs/heads/$HEAD_BRANCH" from the "head" remote.
GithubAppEnabled bool
// use the global setting without overriding
GpgNoSigningEnabled bool
Expand All @@ -106,12 +106,13 @@ func (w *FileWorkspace) Clone(logger logging.SimpleLogging, headRepo models.Repo
logger.Debug("clone directory '%s' already exists, checking if it's at the right commit", cloneDir)

// We use git rev-parse to see if our repo is at the right commit.
// If just checking out the pull request branch, we can use HEAD.
// If just checking out the pull request branch or if there is no
// pull request (API triggered with a custom git ref), we can use HEAD.
// If doing a merge, then HEAD won't be at the pull request's HEAD
// because we'll already have performed a merge. Instead, we'll check
// HEAD^2 since that will be the commit before our merge.
pullHead := "HEAD"
if w.CheckoutMerge {
if w.CheckoutMerge && c.pr.Num > 0 {
pullHead = "HEAD^2"
}
revParseCmd := exec.Command("git", "rev-parse", pullHead) // #nosec
Expand Down Expand Up @@ -331,7 +332,7 @@ func (w *FileWorkspace) wrappedGit(logger logging.SimpleLogging, c wrappedGitCon
func (w *FileWorkspace) mergeToBaseBranch(logger logging.SimpleLogging, c wrappedGitContext) error {
fetchRef := fmt.Sprintf("+refs/heads/%s:", c.pr.HeadBranch)
fetchRemote := "head"
if w.GithubAppEnabled {
if w.GithubAppEnabled && c.pr.Num > 0 {
fetchRef = fmt.Sprintf("pull/%d/head:", c.pr.Num)
fetchRemote = "origin"
}
Expand Down
Loading