-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: don't trigger poll unless target ref matches #270
base: master
Are you sure you want to change the base?
fix: don't trigger poll unless target ref matches #270
Conversation
Note: there is likely still discussion and changes needed here, but I wanted to open a PR to re-open discussions as the old one had long been abandoned and closed. |
94da2c1
to
a954bb3
Compare
a954bb3
to
743c2c1
Compare
743c2c1
to
f8eb872
Compare
8709832
to
16bbbbd
Compare
@KostyaSha would you be willing to take a look over this PR? It does fix the problem and has been running in our CI for a while now |
@oleg-nenashev have you taken over as a maintainer on github-plugin now, please could you take a look at this PR? |
That looks like out of scope of current functionality, current functionality is only kick poll. For native branch triggering there is https://github.com/KostyaSha/github-integration-plugin which deals with exact branches and doesn't use gitscm. |
As per the original description from 2 years ago, the intention of this PR is not to provide native branch triggering, the intention is to prevent pushes to unrelated branches triggering a build. i.e., if the Jenkins build is configured to build the default branch of the repo (e.g., normally master or main) then the build should only be triggered when a GitHub push happens to that branch, not when one happens to other branches within the repository. |
16bbbbd
to
069e927
Compare
069e927
to
6efaba0
Compare
I don't know why this PR is stuck but this functionality is definitely needed. Recently we started seeing infinite build loops in our website builds due to GitHub SCM Polling triggering both when @KostyaSha this definitely is an essential functionality, and it does seem to me like it used to work, since this infinite loop behavior is something I only started observing in our setup a few months ago. I don't know what changed but this PR certainly appears to be fixing it. Please reconsider, saying that this is "out of scope" is a bit silly to me, and it should to you if you think about it a bit. If I'm configuring my branch to fetch |
This issue appears to be related: |
@jakubgs that other plugin doesn’t use this plugin, so that linked issue isn’t related But yes, this PR would prevent your gh-pages pushed from triggering a new master branch build (causing endless loop). We’ve run the .hpi from the build of this PR for more than 2 years now |
Thanks for the contribution man. I will try it, because just yesterday Jenkins committed 6000 times to our
So the build was already performed for given commit, and then it just runs again for no good reason. I don't understand it. Will try your fix. Thanks. |
Though I'm not fan of running custom builds as they will not receive fixes for vulnerabilities and other bugs. But it is better than running infinite website build loops... |
I wasn't really on |
@jakubgs sounds like your compile didn't work — just grab this .hpi file from the linked Jenkins jobs on the PR. Direct Link: github-1.38.1-rc825.6efa_b_a_0b_7b_f8.hpi |
@jakubgs make sure your branch specifier is absolute with the refs/heads prefix ^ here's one of mine (for example), make yours refs/heads/develop if that's the only branch you want to build off |
Thanks for the help, but it doesn't seem this branch fixes the loop. My website builds still get stuck in an endless build loop. Need to debug this more. |
In our case this was an issue with Switching to As of this PR, it was causing these log messages, not sure how harmful they were:
|
6efaba0
to
58500c2
Compare
The existing webhook handling code would trigger the GitSCM poll for the default branch regardless of whether or not the target ref of the webhook push event matched the branch specifier of the Job definition. This is problematic and often leads to duplicate builds for the same branch+revision if pushes to other branches happened to occur whilst build(s) were already in progress for that branch. This is because the GitSCM trigger just compares the HEAD revision of any branch matching the branch specifier against the last *completed* build of that branch (ignoring in-progress builds) and schedules a new build for it. The code for this PR is mostly a rebased version of the original changes proposed by @aserrallerios under jenkinsci#44 with some small refinements. Co-authored-by: Albert Serrallé Ríos <[email protected]>
Ref: spotbugs/spotbugs 616
58500c2
to
066df56
Compare
The existing webhook handling code would trigger the GitSCM poll for the default branch regardless of whether or not the target ref of the webhook push event matched the branch specifier of the Job definition. This is problematic and often leads to duplicate builds for the same branch+revision if pushes to other branches happened to occur whilst build(s) were already in progress for that branch. This is because the GitSCM trigger just compares the HEAD revision of any branch matching the branch specifier against the last completed build of that branch (ignoring in-progress builds) and schedules a new build for it.
The code for this PR is mostly a rebased version of the original changes proposed by @aserrallerios under #44 with some small refinements.