janinko / ghprb

github pull requests builder plugin for Jenkins
https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin
MIT License
370 stars 19 forks source link

Checkout fails if base branch has been modified #116

Open michaelpj opened 10 years ago

michaelpj commented 10 years ago

Between a build being queued and it being executed, if one of the following things happens:

then the PR builder will fail with a message like this:

Checking out Revision 535a3ae146eba90c6033042c9772ac1de32a483f (origin/pr/3162/merge)
FATAL: Could not checkout null with start point 535a3ae146eba90c6033042c9772ac1de32a483f
hudson.plugins.git.GitException: Could not checkout null with start point 535a3ae146eba90c6033042c9772ac1de32a483f
    at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.checkoutBranch(CliGitAPIImpl.java:1090)
    at sun.reflect.GeneratedMethodAccessor325.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at hudson.remoting.RemoteInvocationHandler$RPCRequest.perform(RemoteInvocationHandler.java:299)
    at hudson.remoting.RemoteInvocationHandler$RPCRequest.call(RemoteInvocationHandler.java:280)
    at hudson.remoting.RemoteInvocationHandler$RPCRequest.call(RemoteInvocationHandler.java:239)
    at hudson.remoting.UserRequest.perform(UserRequest.java:118)
    at hudson.remoting.UserRequest.perform(UserRequest.java:48)
    at hudson.remoting.Request$2.run(Request.java:328)
    at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
    at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
    at java.util.concurrent.FutureTask.run(FutureTask.java:138)
    at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
    at java.lang.Thread.run(Thread.java:619)
Caused by: hudson.plugins.git.GitException: Command "checkout -f 535a3ae146eba90c6033042c9772ac1de32a483f" returned status code 128:
stdout: 
stderr: fatal: reference is not a tree: 535a3ae146eba90c6033042c9772ac1de32a483f

I believe this is because when the job is queued, the commit to be built is set to the current merge of the PR head and the target branch, but if either the PR head or the target branch change before the actual build, then Github removes the previous merge commit, and so the attempt to check it out fails.

It would perhaps be better if the builder checked for the current merge commit after it fetches from the remote, and then used that.

valdisrigdon commented 10 years ago

I'm not sure it's possible to do what you are asking. How often is this an issue? Do you have a large quiet period?

michaelpj commented 10 years ago

It's quite annoying. We have quite a busy Github Enterprise instance, and often PR builds can get queued up. So then if someone merges something we get a whole load of spurious failures.

I don't know if my diagnosis of the cause is exactly right, but definitely seems to happen after merges, and it fits the symptoms.

Maybe I'm misunderstanding the situation - but you're running some process when the build actually happens, so it seems like you ought to be able to poll Github at that point to figure out what branch to build.

timgraham commented 10 years ago

We are using Jenkins + this plugin for Django continuous integration and have the same problem. We have a matrix of about 20 Python versions/database combinations with 4 touchstone builds that must pass before the rest are attempted. If any commits are pushed to master after one of the initial 4 pull request builds start, the rest will fail with the checkout error. I might be willing to help contribute a fix, but I'd need guidance as 'I'm not sure it's possible to do what you are asking." leaves me thinking it won't be simple.

coder9042 commented 10 years ago

I had a look into this. GhprbCause object creation calls pr.getTarget() and pr.getSource(), which in turn return the static elements: target and source, which are set in the constructor.(Here pr is GhprbPullRequest object)

If instead of returning initially set values, we return the current value by pr.getBase().getRef() and pr.getHead().getRef(), where pr is GHPullRequest object that is passed onto GhprbPullRequest constructor, which I assume gives updated values from its above functions, would that make a difference?

mboersma commented 10 years ago

We also run into this situation often with our continuous integration server in ways similar to what @timgraham described. See deis/deis#1470.

coder9042 commented 10 years ago

@mboersma It seems you have been trying to fix this for a while. Could you try and check if what I suggested above makes any difference ?

mboersma commented 10 years ago

@coder9042, I did actually try the changes you suggested, but it didn't help unfortunately. I got the same "Could not checkout null" errors afterward.

I don't have the code in front of me, but I don't think the pr.getBase().getRef() and the other call actually end up returning different SHA values from pr.getTarget() and pr.getSource() at runtime.

I didn't see any other ways to approach this, but I'm not that familiar with the codebase and my Java is a bit rusty. We are currently just working around it by forcing a rebuild in Jenkins every time this happens, which is tedious.

coder9042 commented 10 years ago

Did you keep in mind that pr is different in both these places, becuase I am of the opinion that if what I suggested does not work then the GHPullRequest that is imported in the ghprb is the culprit.

Memphiz commented 10 years ago

@mboersma we also are hitting this issue on xbmc - can you explain your workaround a bit more? Or are you saying you are manually rebuilding the job once it happend? (i am looking of some sort of automation of this - though i guess the signalling of the PR won't work after rebuild).

mbektchiev commented 10 years ago

I am also affected with this issue and came up with an idea for a workaround, although I haven't tried it, yet. This could be a possible approach for fixing the bug as well:

mbektchiev commented 10 years ago

pre-build:

git checkout -b prmerge-$env:sha1-$env:BUILD_NUMBER
git push origin prmerge-$env:sha1-$env:BUILD_NUMBER

post-build:

git checkout $env:sha1
git branch -d prmerge-$env:sha1-$env:BUILD_NUMBER
git push origin :prmerge-$env:sha1-$env:BUILD_NUMBER

This seems to fix the problem. Maybe it would be enough to make the plugin create a unique tag for the build and not only for the pull request (e.g. origin/pr/<PR#>/merge-<BUILD#> instead of the current origin/pr/<PR#>/merge). Currently if there are multiple Jenkins agents, performing simultaneous builds they overwrite each others' tags.