SOLUTION:
You can set it to not use the merge ref through an env var.
DRONE_GITHUB_MERGE_REF=true
Set to true to use the refs/pulls/%d/merge vs refs/pulls/%d/head
Hi,
I ran into two issues with the clone plugin using pull/:id/merge as opposed to pull/:id/head on a pull request with merge conflicts.
Drone pulls nexpected/stale code if merge conflict was introduced in the middle of a branch.
There was a merge conflict that was introduced, and the builds for a pull request started pulling from a stale piece of code (probably from before right the conflict).
git fetch --no-tags origin +refs/pull/2648/merge:
resulted in stale code.
git fetch --no-tags origin +refs/pull/2648/head:
resulted in the actual code in the PR, though there might be a merge conflict with the base branch.
This is annoying because instead of an explicit fail, it goes through the whole pipeline and deploys randomly old code into my feature branch testing servers.
Build failures for pull requests if there is a merge conflict to begin with
@bradrydzewski We just got bit by this on 0.8-rc4. We cannot use the aforementioned workaround since we want to test the build with the result of the merge. Is there any other solution or a fix in progress?
I do not think there is anything to fix. The error message indicates the code cannot be merged due to a merge conflict. The github user interface likely displays the same error. One cannot test the merged result if the code cannot be merged in the first place.
So in my opinion this is the desired behavior. It should fail the build with and error message. The user should then resolve the merge conflict and synchronize the pull request, thus triggering a new build.
As mentioned by the author, you could configure drone to use the head ref instead of the merge ref. Your concerns with regard to testing your code against the merge commit could be addressed by enabling protected branches in GitHub, which prevent merging a pull request that is not synchronized with the target branch.
While I agree this is the expected behaviour on merge conflicts, we’ve had several occurrences where there were no conflicts (GitHub was all green) and we still received the error message. So we had to push a “dummy” commit just to basically be able to merge since we already use protected branches and enforce that the build passes.
This is an issue with GitHub. In some cases they send the webhook hook before the code is actually merged, which means the merge ref does not yet exist when trying to clone the repository. There are no plans to create a workaround for this GitHub bug, as we are hoping it is something they will fix on their end.
With that being said, Drone is flexible enough that you can change the clone behavior to meet the exact needs of your organization. If you do not want to wait for a GitHub fix and you do not want to use the head ref, you could create a custom clone plugin that retries on failure, or perhaps checks github and verifies the merge ref is ready before cloning.
UPDATE
However it happens quite rare compared to merge-ref failures.
I use this bash script at the build stage to switch from head to merge with retries.
@bradrydzewski Thanks for the info – for the Github fix is there an issue we can vote on? Or something we could reference in a support inquiry with them?
configure drone to use the head ref instead of the merge ref
What are the real-world effects of making this change? Does it change the desired effect of testing the hypothetical merging of the PR with its target branch?
If you are seeing intermittent errors cloning head refs you would need to contact github support. Drone is just running a simple git clone. If that is failing it needs to be addressed with the upstream system, especially since cloning head refs is something that should always work.
Sorry, not that I am aware of. Providing feedback that pull request hooks should not be sent until the merge ref is available might be a good place to start. This has been the behavior for at least 4 years, so I am not very confident it is something they plan to address any time soon, if ever.
Correct, this is testing the unmerged code. If this is a concern, you can enable protected branches and prevent merging a pull request that is out of sync with the target branch. This forces the pull request author to sync with the target branch before the pull request can be merged, and should therefore mitigate any such concerns.
@allanrenucci you can implement custom clone logic without creating a custom plugin. You can use shell commands in the clone section, instead of a plugin, for example:
@bradrydzewski Do I understand correctly that this is a race condition where drone tries to fetch the /merge result too fast? If so, would a sleep(10) potentially resolve this (albeit in a dirty way)
Yes, github will sometimes send the pull request hook before the pull request can be cloned. If this happens, git returns an error when cloning the ref because it does not exist in github yet. Some workarounds could include:
add a sleep statement
create a custom clone plugin that polls and blocks until the merge ref is ready
create a custom clone plugin that retries the clone step with backoff
configure drone to use head refs with protected branches enabled in github
This issue seems to be causing a good amount of discussion in this thread, the issue track and the slack channel. It looks like github must be having perf issues creating merge refs, causing a lot of builds to fail. I have updated the git plugin to perform a backoff.
@bradrydzewski thank you for the update! However, could you explain steps to update the git plugin, or it will be updated in the next version of drone image? We are using latest docker image (0.8.2, as i know)
$ docker images
REPOSITORY TAG IMAGE ID CREATED SIZE
plugins/git latest 5ed1c3cd7d40 16 hours ago 67.9MB
drone/agent 0.8.1 f48f9317b6e0 4 weeks ago 13.3MB
drone/drone 0.8.1 51d64bcd94b9 4 weeks ago 30.4MB
Builds still occasionally fail to clone:
+ git init
Initialized empty Git repository in /drone/src/github.com/lampepfl/dotty/.git/
+ git remote add origin https://github.com/lampepfl/dotty.git
+ git fetch --no-tags origin +refs/pull/3423/merge:
error: RPC failed; curl 18 transfer closed with outstanding read data remaining
fatal: The remote end hung up unexpectedly
exit status 128
The new version of the plugin only retries when a couldn't find remote ref error is returned in order to address the issue where the github merge ref is not available.
All other failures from the git client or server will continue to result in a failed clone step.
I think the issue I described above is related to the remote not existing yet. I’ve only seen it happen on PRs and every time I restart the build after this failure it succeeds.