Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-25048

Git plugin ignores included/excluded paths when triggering build on commit notification

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • git-plugin
    • None

      I have a Git repository, which contains of multiple modules, which should be build separately. I have separate Jenkins job for each module. Stash's "Jenkins Webhook plugin" notifies Jenkins about a new commit, which triggers ALL the module job to start.
      To separate job I've used "polling ignores commits in certain paths", selecting included regions. Seems, like the regions are not used during commit notification triggering.

          [JENKINS-25048] Git plugin ignores included/excluded paths when triggering build on commit notification

          ty tanic added a comment -

          Any news on this ? Could I help with it ?

          ty tanic added a comment - Any news on this ? Could I help with it ?

          Mark Waite added a comment -

          tanic you could compile the plugin from the current master branch, merge https://github.com/jenkinsci/git-plugin/pull/318 into that branch, and evaluate it. Once you've seen if that meets your needs, then you could review the issues raised in the pull request by jglick and consider how to provide an alternate implementation that doesn't launch the build from the notifyCommit, but rather schedules polling and allows the polling to then decide if a build is needed.

          Mark Waite added a comment - tanic you could compile the plugin from the current master branch, merge https://github.com/jenkinsci/git-plugin/pull/318 into that branch, and evaluate it. Once you've seen if that meets your needs, then you could review the issues raised in the pull request by jglick and consider how to provide an alternate implementation that doesn't launch the build from the notifyCommit, but rather schedules polling and allows the polling to then decide if a build is needed.

          In terms of pipelines when there is a single Jenkinsfile per branch, it would be useful to redesign this functionality in a way to skip some steps when no related changes detected.
          For example I do not want to run heavy database tests on css changes, or not to run UI tests on migration changes.
          New issue submitted: JENKINS-37978

          Alexander Vorobiev added a comment - In terms of pipelines when there is a single Jenkinsfile per branch, it would be useful to redesign this functionality in a way to skip some steps when no related changes detected. For example I do not want to run heavy database tests on css changes, or not to run UI tests on migration changes. New issue submitted: JENKINS-37978

          Has there been any resolution for this?

          Danrisha Young added a comment - Has there been any resolution for this?

          Mark Waite added a comment -

          No, the proposed pull request is still open. I'd love to have assistance evaluating pull requests. You could take the tip of the current plugin master branch, apply that proposed change to it, and then report your results to the pull request.

          Mark Waite added a comment - No, the proposed pull request is still open. I'd love to have assistance evaluating pull requests. You could take the tip of the current plugin master branch, apply that proposed change to it, and then report your results to the pull request.

          Damien Ruscoe added a comment -

          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

          Damien Ruscoe added a comment - 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

          Piotr Z. added a comment -

          Is there any update here? I have the same problem with included/excluded directories.

          Git plugin: 4.2
          Jenkins: 2.222

          Piotr Z. added a comment - Is there any update here? I have the same problem with included/excluded directories. Git plugin: 4.2 Jenkins: 2.222

          Mark Waite added a comment -

          No update pzs. As far as I know, no one is working on changes in this area.

          Mark Waite added a comment - No update pzs . As far as I know, no one is working on changes in this area.

          Piotr Z. added a comment -

          Is there any working workaround? How do you deal with such a case?

          Piotr Z. added a comment - Is there any working workaround? How do you deal with such a case?

          Mark Waite added a comment -

          A workaround for Pipeline jobs is to include the exclusion or inclusion in the Jenkinsfile that defines the job. When the inclusion or exclusion condition is not met, do an early exit from the pipelines as described in a stackoverflow answer.

          A workaround in a Freestyle job might be possible with a similar technique, adding an early exit from the build scripts if run in a Jenkins environment and if the inclusion or exclusion condition was not satisfied.

          Mark Waite added a comment - A workaround for Pipeline jobs is to include the exclusion or inclusion in the Jenkinsfile that defines the job. When the inclusion or exclusion condition is not met, do an early exit from the pipelines as described in a stackoverflow answer . A workaround in a Freestyle job might be possible with a similar technique, adding an early exit from the build scripts if run in a Jenkins environment and if the inclusion or exclusion condition was not satisfied.

            Unassigned Unassigned
            pbaranchikov Pavel Baranchikov
            Votes:
            30 Vote for this issue
            Watchers:
            37 Start watching this issue

              Created:
              Updated: