search

mendhak / teamcity-stash

TeamCity - Stash integration. Plugin for TeamCity which updates Stash with build statuses
https://code.mendhak.com/teamcity-stash/
54 stars 17 forks source link

Problem with refs/pull-requests/*/merge references #2

Closed aefimov closed 11 years ago

aefimov commented 11 years ago

See https://jira.atlassian.com/browse/STASH-3196 Maybe it a bug within Stash, but maybe you can solve it within Teamcity?

mendhak commented 11 years ago

Hi aefimov, you said

But hash used in refs/pull-requests/123/merge is different to hash on head of pull request branch.

Do you mean the extra commit that gets created after you do a merge? If so, then I think you are talking about git merge vs git rebase. This guy shows how to avoid it.

aefimov commented 11 years ago

No. Look. Then you create Pull Request with in Stash for branch 'feature-1', Stash make automerge commit in special branch referenced refs/pull-requests/1234/merge, there 1234 -- is pull request number. This commit is actually result of merging Pull Request as is it merged from Stash manually. Like dry-run merge for Pull Request.

And better, if you will test not Pull Request HEAD, but actually the result of merging this Pull Request. See https://answers.atlassian.com/questions/144509/what-purpose-of-refs-pull-requests-123-merge-clean-and-other This explain a little this one solution to test Pull Requests.

So, now Stash not associate fake automerge commit with Pull Request, and do not show Build status within. To avoid this you must report hash from previous commit, not from Automerge one. But maybe it must be done on Stash side. As workarround your plugin maybe try to detect Automerged commits and then report build status for both of commits -- for Automerge and for previous before Automerge.

aefimov commented 11 years ago

We discussed with @yaegor details about this. This is not a bug in this plugin and you should not make workarround about i'm ask you. This is wrong solution, because is test runned on Automerge commit, is not guarantied if previous commit also Success or Fail. It totally different commits. So, this bug must be fixed only on Atlassian Stash side.

laurentkempe commented 11 years ago

Too bad I just created a build watching +:refs/pull-requests/*/merge-clean just to realize that the build is working but that the plugin does it job and report the status using the SHA1 of the merge and then stash doesn't show it correctly on the pull request page. Then I found your post, too bad. I really hope it will work soon.

georgy commented 10 years ago

Can't wait for Atlassian to fix it, here is plugin that "solves" this: https://marketplace.atlassian.com/plugins/com.bolyuba.stash.plugin.stash-build-status-plugin, you can contribute here: https://bitbucket.org/bolyuba/stash-build-status-plugin/wiki/Home

aefimov commented 10 years ago

@georgy Two things - first one, there is Build Storm, did you know about this side effect of monitoring merge references? Second one — this plugin to TeamCity is outdated, there are new one from JB team: https://github.com/JetBrains/commit-status-publisher (supports Gerrit, Stash) http://confluence.jetbrains.com/display/TW/Commit+Status+Publisher

georgy commented 10 years ago

Build Storm: I assume you are talking about (N*N)/2 of build for N open pull requests. Scalability will be subject to duration of your build as well as number of open pull requests, true. Any suggestions on how to avoid this and still be confident that result of your merge will build?

Thanks for the link to newer version, should probably create a pull request for this repo with that link on top of README :)