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

notifyCommit branch parameter can't contain slashes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open (View Workflow)
    • Priority: Major
    • Resolution: Unresolved
    • Component/s: git-plugin
    • Labels:
    • Environment:
      Jenkins 1.620, Git Plugin 2.4.0
    • Similar Issues:

      Description

      I have a simple Jenkins job called "myjob":

      Repository URL: git://github.com/jenkinsci/git-client-plugin.git
      Branches to build: tests/getSubmodules
      Build Triggers > Poll SCM is enabled

      I execute

      curl -s http://myjenkinssrv/jenkins/git/notifyCommit -d url=git://github.com/jenkinsci/git-client-plugin.git -d branches=tests/getSubmodules

      and I get

      No git jobs using repository: git://github.com/jenkinsci/git-client-plugin.git and branches: tests/getSubmodules

      This is wrong! If I configure my job with "Branches to build"=master and call

      curl -s http://myjenkinssrv/jenkins/git/notifyCommit -d url=git://github.com/jenkinsci/git-client-plugin.git -d branches=master

      it works, as expected.

        Attachments

          Activity

          ludwig Ludwig Arnesen created issue -
          Hide
          deepchip Martin d'Anjou added a comment -

          I can reproduce this issue, and it is critical for me. I will attempt to contribute a test, and a fix if I can.

          If someone could help me at first by showing me how to enable the verbosity of all the listener.getLogger() calls that would help me. I already know how to setup the log recorder in jenkins to catch all the hudson.plugins.git messages, and I already know to start jenkins with verbosity enabled java -Dhudson.plugins.git.GitSCM.verbose=true -jar jenkins.war

          Show
          deepchip Martin d'Anjou added a comment - I can reproduce this issue, and it is critical for me. I will attempt to contribute a test, and a fix if I can. If someone could help me at first by showing me how to enable the verbosity of all the listener.getLogger() calls that would help me. I already know how to setup the log recorder in jenkins to catch all the hudson.plugins.git messages, and I already know to start jenkins with verbosity enabled java -Dhudson.plugins.git.GitSCM.verbose=true -jar jenkins.war
          Hide
          markewaite Mark Waite added a comment -

          If you change the argument value to origin/tests/getSubmodules, does it behave better?

          Show
          markewaite Mark Waite added a comment - If you change the argument value to origin/tests/getSubmodules, does it behave better?
          Hide
          deepchip Martin d'Anjou added a comment - - edited

          Setting "Branches to build" to origin/tests/getSubmodules. Result:

          $ curl -s http://localhost:8080/git/notifyCommit \
            -d url=ssh://git@localhost:7999/project/veelox.git \
            -d branches=tests/getSubmodules
          Scheduled polling of VeeloxStash � veeloxstash-prbuild
          

          Your suggestion worked.

          If the notification contains the alias name of the repository, it does not work:

          $ curl -s http://localhost:8080/git/notifyCommit \
            -d url=ssh://git@localhost:7999/project/veelox.git \
            -d branches=origin/tests/getSubmodules
          No git jobs using repository: ssh://git@localhost:7999/project/veelox.git and branches: origin/tests/getSubmodules
          

          So it seems that the BRANCH_NAME in &branches=BRANCH_NAME is prefixed by the alias name of the repository (I have tried origin, upstream, and an empty name), before the Git Plugin scans the jobs for candidates. Looks like this is a documentation issue. I admit I am often confused myself by the Git Plugin documentation.

          Show
          deepchip Martin d'Anjou added a comment - - edited Setting "Branches to build" to origin/tests/getSubmodules . Result: $ curl -s http://localhost:8080/git/notifyCommit \ -d url=ssh://git@localhost:7999/project/veelox.git \ -d branches=tests/getSubmodules Scheduled polling of VeeloxStash � veeloxstash-prbuild Your suggestion worked. If the notification contains the alias name of the repository, it does not work: $ curl -s http://localhost:8080/git/notifyCommit \ -d url=ssh://git@localhost:7999/project/veelox.git \ -d branches=origin/tests/getSubmodules No git jobs using repository: ssh://git@localhost:7999/project/veelox.git and branches: origin/tests/getSubmodules So it seems that the BRANCH_NAME in &branches=BRANCH_NAME is prefixed by the alias name of the repository (I have tried origin , upstream , and an empty name), before the Git Plugin scans the jobs for candidates. Looks like this is a documentation issue. I admit I am often confused myself by the Git Plugin documentation.
          Hide
          markewaite Mark Waite added a comment -

          Yes, the documentation has a very difficult task, describing something with more edge cases and conditionals than make me comfortable. I've preferred to focus my efforts on writing tests to enumerate the edge cases rather than writing documentation to describe the edge cases.

          This is a most interesting condition for which (I believe) there is no unit test to show the behavioral oddity you've detected. I'm not entirely sure how to write a test for this case.

          Show
          markewaite Mark Waite added a comment - Yes, the documentation has a very difficult task, describing something with more edge cases and conditionals than make me comfortable. I've preferred to focus my efforts on writing tests to enumerate the edge cases rather than writing documentation to describe the edge cases. This is a most interesting condition for which (I believe) there is no unit test to show the behavioral oddity you've detected. I'm not entirely sure how to write a test for this case.
          Hide
          deepchip Martin d'Anjou added a comment - - edited

          This code prefixes the repository name to the branch name:
          https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L266

          It makes sense to NOT use the remote repository name when composing the notifyCommit HTTP GET branches parameter. The external world is not supposed to know what names will be used inside the Git Plugin configuration.

          On the other hand, there are eight documented formats for the branches to build:

          1. <branchName>
          2. refs/heads/<branchName>
          3. <remoteRepoName>/<branchName>
          4. remotes/<remoteRepoName>/<branchName>
          5. refs/remotes/<remoteRepoName>/<branchName>
          6. ${ENV_VARIABLE}
          7. <Wildcards>
          8. :<regular expression>

          According to the code in BranchSpec.java and GitStatus.java, I think the first two will never work with notifyCommit. The last three may work if constructed properly. So is this issue a bug? I think it can be addressed with proper documentation in the wiki and perhaps in the help text of the plugin. As for writing tests, I agree that it is necessary.

          Show
          deepchip Martin d'Anjou added a comment - - edited This code prefixes the repository name to the branch name: https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L266 It makes sense to NOT use the remote repository name when composing the notifyCommit HTTP GET branches parameter. The external world is not supposed to know what names will be used inside the Git Plugin configuration. On the other hand, there are eight documented formats for the branches to build: <branchName> refs/heads/<branchName> <remoteRepoName>/<branchName> remotes/<remoteRepoName>/<branchName> refs/remotes/<remoteRepoName>/<branchName> ${ENV_VARIABLE} <Wildcards> :<regular expression> According to the code in BranchSpec.java and GitStatus.java , I think the first two will never work with notifyCommit . The last three may work if constructed properly. So is this issue a bug? I think it can be addressed with proper documentation in the wiki and perhaps in the help text of the plugin. As for writing tests, I agree that it is necessary.
          Hide
          ludwig Ludwig Arnesen added a comment -

          Is it possible to fix this problem? I use /ref/heads/... or /ref/tags/... in the "Branches to build" input field to build from branches or tags. It can happen that a user moves a tag which means the git server calls notifyCommit to trigger a new build. If I have to use "origin" as a prefix (which is not very intuitive) in the "Branches to build" field I cannot build from tags.

          Show
          ludwig Ludwig Arnesen added a comment - Is it possible to fix this problem? I use /ref/heads/... or /ref/tags/... in the "Branches to build" input field to build from branches or tags. It can happen that a user moves a tag which means the git server calls notifyCommit to trigger a new build. If I have to use "origin" as a prefix (which is not very intuitive) in the "Branches to build" field I cannot build from tags.
          ludwig Ludwig Arnesen made changes -
          Field Original Value New Value
          Status Open [ 1 ] In Progress [ 3 ]
          ludwig Ludwig Arnesen made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Hide
          warrenseine Warren Seine added a comment - - edited

          I'm also experiencing this issue.

          I already had this issue with The BFG (a Java tool to filter a Git repository) and it was fixed. Are you using the same Git library?

          See the issue here: https://github.com/rtyley/bfg-repo-cleaner/issues/34

          Show
          warrenseine Warren Seine added a comment - - edited I'm also experiencing this issue. I already had this issue with The BFG (a Java tool to filter a Git repository) and it was fixed . Are you using the same Git library? See the issue here: https://github.com/rtyley/bfg-repo-cleaner/issues/34
          Hide
          markewaite Mark Waite added a comment -

          Warren Seine the plugin is not using the same library. I believe the root reason the notifyCommit branch parameter has difficulty with some forms of branch names is do the legacy compatibility retained for imprecisely declared branch names. With 60 000+ installations, compatibility matters very much.

          Show
          markewaite Mark Waite added a comment - Warren Seine the plugin is not using the same library. I believe the root reason the notifyCommit branch parameter has difficulty with some forms of branch names is do the legacy compatibility retained for imprecisely declared branch names. With 60 000+ installations, compatibility matters very much.
          Hide
          warrenseine Warren Seine added a comment -

          It makes a lot of sense. Wouldn't a checkbox in the settings do the job?

          Show
          warrenseine Warren Seine added a comment - It makes a lot of sense. Wouldn't a checkbox in the settings do the job?
          Hide
          coderhugo Robin Müller added a comment - - edited

          I created a Pull-Request a few days ago for this. Can you review it and merge it back if you are comfortable with it?

          Show
          coderhugo Robin Müller added a comment - - edited I created a Pull-Request a few days ago for this. Can you review it and merge it back if you are comfortable with it?
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jesse Glick
          Path:
          multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java
          http://jenkins-ci.org/commit/workflow-plugin/fca115faca36a62968c1500ffd89e06b982a56f4
          Log:
          Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603.
          (Multibranch Git webhooks rely only on the url parameter to detect changes.)

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java http://jenkins-ci.org/commit/workflow-plugin/fca115faca36a62968c1500ffd89e06b982a56f4 Log: Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603 . (Multibranch Git webhooks rely only on the url parameter to detect changes.)
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jesse Glick
          Path:
          multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java
          http://jenkins-ci.org/commit/workflow-plugin/521099bd8a31963ea6149de0a4f4ab9fe5617e97
          Log:
          Merge pull request #276 from jglick/test-JENKINS-29603

          Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603

          Compare: https://github.com/jenkinsci/workflow-plugin/compare/3840d8ca847e...521099bd8a31

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java http://jenkins-ci.org/commit/workflow-plugin/521099bd8a31963ea6149de0a4f4ab9fe5617e97 Log: Merge pull request #276 from jglick/test- JENKINS-29603 Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603 Compare: https://github.com/jenkinsci/workflow-plugin/compare/3840d8ca847e...521099bd8a31
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jesse Glick
          Path:
          multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java
          http://jenkins-ci.org/commit/workflow-multibranch-plugin/c9738eb45a25c1ae6a6e55df0d9688654d328725
          Log:
          Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603.
          (Multibranch Git webhooks rely only on the url parameter to detect changes.)
          Originally-Committed-As: fca115faca36a62968c1500ffd89e06b982a56f4

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowBranchProjectFactoryTest.java http://jenkins-ci.org/commit/workflow-multibranch-plugin/c9738eb45a25c1ae6a6e55df0d9688654d328725 Log: Extend JENKINS-30744 test to confirm that it is unaffected by JENKINS-29603 . (Multibranch Git webhooks rely only on the url parameter to detect changes.) Originally-Committed-As: fca115faca36a62968c1500ffd89e06b982a56f4
          rtyler R. Tyler Croy made changes -
          Workflow JNJira [ 164485 ] JNJira + In-Review [ 181635 ]
          Hide
          zonart Nicko Glayre added a comment -

          I attempted to solve the merging issue in the PR proposed by Robin Müller and it looks like to work as expected.
          Could you please have a look at the following ?

          Show
          zonart Nicko Glayre added a comment - I attempted to solve the merging issue in the PR proposed by Robin Müller and it looks like to work as expected. Could you please have a look at the following ?
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Nicolas Glayre
          Path:
          src/main/java/hudson/plugins/git/BranchSpec.java
          src/main/java/hudson/plugins/git/GitSCM.java
          src/main/java/hudson/plugins/git/GitStatus.java
          src/test/java/hudson/plugins/git/GitStatusTest.java
          http://jenkins-ci.org/commit/git-plugin/1db52c4b7fa03eb6c7d61a0af86a2b3e18343051
          Log:
          JENKINS-29603 Fix notifyCommit for branches that contain slashes

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nicolas Glayre Path: src/main/java/hudson/plugins/git/BranchSpec.java src/main/java/hudson/plugins/git/GitSCM.java src/main/java/hudson/plugins/git/GitStatus.java src/test/java/hudson/plugins/git/GitStatusTest.java http://jenkins-ci.org/commit/git-plugin/1db52c4b7fa03eb6c7d61a0af86a2b3e18343051 Log: JENKINS-29603 Fix notifyCommit for branches that contain slashes
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Nicolas Glayre
          Path:
          src/test/java/hudson/plugins/git/GitStatusTest.java
          http://jenkins-ci.org/commit/git-plugin/b184f2b46eb2b0cb6e0274b28e138f39c7be1c68
          Log:
          JENKINS-29603 Fixing failing tests added on master branch

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nicolas Glayre Path: src/test/java/hudson/plugins/git/GitStatusTest.java http://jenkins-ci.org/commit/git-plugin/b184f2b46eb2b0cb6e0274b28e138f39c7be1c68 Log: JENKINS-29603 Fixing failing tests added on master branch
          Hide
          jbochenski Jakub Bochenski added a comment -

          I confirm using the origin/<slashyBranch> workaround helps. No other forms seem to work though (e.g refs/heads/... or refs/remotes/...)

          Show
          jbochenski Jakub Bochenski added a comment - I confirm using the origin/<slashyBranch> workaround helps. No other forms seem to work though (e.g refs/heads/... or refs/remotes/...)
          Hide
          pyrocks Mor L added a comment -

          Workaround works in our case too - but seeing the fix already merged to a beta of version 4 of the Git plugin - when is it due to be released officially (as in available through Jenkins update center)?

          Show
          pyrocks Mor L added a comment - Workaround works in our case too - but seeing the fix already merged to a beta of version 4 of the Git plugin - when is it due to be released officially (as in available through Jenkins update center)?
          Hide
          markewaite Mark Waite added a comment - - edited

          Mor L my current goal is to release git plugin 4 before the end of Feb 2019. There are still several things that must be completed before that release. You can see the progress on the pull requests that need to precede release in the git client plugin 3.0 milestone and the git plugin 4.0 milestone.

          You're welcome (and encouraged) to download and install the pre-release versions of the git client plugin 3.0.0 and the git plugin 4.0.0. Those versions are the most recent builds of the master branch. Report any issues with those versions and you'll be helping the community have a better release.

          Show
          markewaite Mark Waite added a comment - - edited Mor L my current goal is to release git plugin 4 before the end of Feb 2019. There are still several things that must be completed before that release. You can see the progress on the pull requests that need to precede release in the git client plugin 3.0 milestone and the git plugin 4.0 milestone . You're welcome (and encouraged) to download and install the pre-release versions of the git client plugin 3.0.0 and the git plugin 4.0.0 . Those versions are the most recent builds of the master branch. Report any issues with those versions and you'll be helping the community have a better release.
          markewaite Mark Waite made changes -
          Assignee Nicolas De Loof [ ndeloof ]

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            ludwig Ludwig Arnesen
            Votes:
            5 Vote for this issue
            Watchers:
            14 Start watching this issue

              Dates

              Created:
              Updated: