I have tried applying your patch but due to API changes it no longer cleanly applies.
isRevExcluded accepts a AbstractProject as its parameter although there is no guarantee that the project variable is an instance of this type.
https://github.com/jenkinsci/git-plugin/commit/057acb2767d3988c6d5d33a08ae253bac6241325
This added the check
- if (!project.isDisabled()) {
+ if (!(project instanceof AbstractProject && ((AbstractProject) project).isDisabled())) {
The condition is true if project is an AbstractProject and enabled, or, project is any other type. This allows WorkflowJob objects to also be triggered by NotifyCommits.
Ive tried modifying the patch to accept higher levels of abstraction but this leads to more issues; higher level abstractions are not guaranteed to have a workspace which makes me agree with @jglick https://github.com/jenkinsci/git-plugin/pull/318
Triggering a polling notification would be the better solution here. The fact that supplying a SHA1 bypasses polling completely feels a little hacky to me. If a notification was triggered then we get the behaviour implemented here for free.
A further argument for a polling solution is security. http://kohsuke.org/2011/12/01/polling-must-die-triggering-jenkins-builds-from-a-git-hook/
Kohsuke touched on this stating that this hook could be sent over HTTP (not HTTPS). “the server does not directly use anything the client is sending. It runs polling to verify there is a change.” As it currently stands this route could lead to a DDoS on public Jenkins servers as there is no verification of the request.
I’ve investigated the polling solution but I don’t have the java/jenkins knowledge to create a patch. The interface between Jenkins and the Git plugin loses the information regarding which SHA1 commit to actually build. Jenkins then builds the latest commit when that job is picked from the queue which may not be the SHA1 requested to be built. This incidentally leads to a race condition which is described here: https://issues.jenkins-ci.org/browse/JENKINS-20518
Damien
Any news on this ? Could I help with it ?