stash-pullrequest-builder-plugin icon indicating copy to clipboard operation
stash-pullrequest-builder-plugin copied to clipboard

Previous merged commit used when a PR is modified

Open ludrao opened this issue 9 years ago • 3 comments

When a PR is updated with a new commit, sometimes the plugin uses the previous merged commit rather than the new one.

Possible cause of the issue: I understood that Stash is building the merged commit/branch (origin/pr/${pullRequestId}/merge) in the background, so I suspect that there is a bad timing condition that is if the plugin query the PR and find it to be changed, it will pull the origin/pr/${pullRequestId}/merge branch before it is actually updated with the new merge commit.

This is quite annoying as it is hard to be sure that the test are passed with the latest changes!

Other than that, thx for the plugin it is great ! :)

ludrao avatar Sep 02 '16 14:09 ludrao

There are several Issues and posts concerning this problem: Main explanation: https://community.atlassian.com/t5/Bitbucket-questions/Change-pull-request-refs-after-Commit-instead-of-after-Approval/qaq-p/194702 Other links: https://confluence.atlassian.com/stashkb/pull-requests-not-reflecting-changes-pushed-to-remote-branch-after-an-upgrade-385321658.html https://issues.jenkins-ci.org/browse/JENKINS-35219

The current workaround is to enable the Build only if PR is mergeable option in this plugin. This however is no solution for all scenarios: If you set up stash so pull requests need to be approved (by n developers) the PR-Builder is never triggered.

I would propose the integration of the following workaround: Create another option (checkbox) Check Mergablility before building and request the endpoint used to check whether a PR is mergable but build regardless of reply.

u3r avatar May 12 '17 09:05 u3r

I've since looked at the code and Build only if Stash reports no conflicts also triggers a check to the appropriate Stash-endpoint (see StashRepository.java@b2be6792adc2e9e3670189908b09b2331da05d30 Methods isPullRequestMergable Lines 207ff and isBuildTarget Lines 248ff) So you can restrict merging and still always build the latest commit if you check this option.

I would still recommend

  • either always doing a query to this endpoint
  • or making it an explicit option (preferably the default)
  • or documenting the workaround VERY LOUDLY in the pop-up help in jenkins ;-)

u3r avatar May 18 '17 12:05 u3r

For anybody else stumbling over this issue: consider merging locally.

fkaelberer avatar Jun 02 '17 08:06 fkaelberer