Drone repo's own use of /merge ref

Drone Support
1

So I’ve been following all of the threads and issues about the /merge ref for many months. I consider myself to be pretty well-versed on the nondeterministic behavior of Github, and the advice to use the /head ref instead of the /merge ref.

As a quick summary, the two main issues with the EDIT: /head /merge ref are:

  1. For new PRs, the drone job could fail because the merge ref cannot be found
  2. For existing PRs with new commits being tested, the drone job could run but uses the wrong commit because the FETCH_HEAD is pointing to an older commit.

I see that Drone now has a “retry” feature if fetching the /merge ref fails. Is my understanding correct that this fixes #1, but not #2?

I noticed in drone’s own PR process, it appears that the /merge ref is used. I’m curious about this, because it seems hard to trust that the drone tests are really being run on the right commit as long as #2 is still an issue.

Do you as the drone repo maintainer trust that incoming PRs are not subject to issue #2? Or is this just a risk you’re willing to accept?

The reason I keep coming back to this instead of just using the /push job and turning off the /pr job, is because some of our contributors use the fork model, and others use the branch-off-the-main-repo model. And /push doesn’t work for forks.

But if I turn on /pr then I either have to accept issue #2 as a possible insidious bug, or switch to using /head instead of /merge refs. But if I do that, then for those who use feature branches, they’ll essentially be running two redundant jobs on each PR, since /pr and /push would both be testing the feature branch in isolation right?

I’ve been going around in circles about the right thing to do here, and was hoping that I could get some insight into how you made the choices you did for testing drone’s own PRs, and what your degree of trust is that you’re not testing the wrong commits yourself on incoming PRs.

Thanks!

2

was hoping that I could get some insight into how you made the choices you did for testing drone’s own PRs, and what your degree of trust is that you’re not testing the wrong commits yourself on incoming PRs.

Keep in mind that this was not an issue until recently, within the past 6 months. GitHub must have made an internal change because for ~4 years this was not a problem, and everything started blowing up one day. It is likely that other CI systems are having similar issues, since we based our clone logic off of Travis, which seemed like a safe bet at the time.

So the current state of merge refs was not really a choice we made voluntarily – GitHub changed and forced us into this situation. This change has not had a material impact on Drone, aside from requiring me to restart a few builds from time to time. Most of our customers switch to head ref, which is why there has not been much urgency on my end.

I’ve been going around in circles about the right thing to do here

We have come up with a proposed solution here: Planned change to git clone logic

3

@geekdave see my comment here Planned change to git clone logic

I have been meaning to put together a demo for how the new cloning can work, and took some time today to post a simple, working example. It no longer uses the merge ref, and it rebases to the correct commit sha. It is not battle tested or production ready, but I think with some peer review from the community, it should not take much time.