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

notifyCommit branch parameter can't contain slashes

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • git-plugin
    • Jenkins 1.620, Git Plugin 2.4.0

      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.

          [JENKINS-29603] notifyCommit branch parameter can't contain slashes

          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

          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

          Mark Waite added a comment -

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

          Mark Waite added a comment - If you change the argument value to origin/tests/getSubmodules, does it behave better?

          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.

          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.

          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.

          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.

          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.

          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.

          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 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.

          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

          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

          Mark Waite added a comment -

          warrenseine 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.

          Mark Waite added a comment - warrenseine 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.

          Warren Seine added a comment -

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

          Warren Seine added a comment - It makes a lot of sense. Wouldn't a checkbox in the settings do the job?

          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?

          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?

          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.)

          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.)

          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

          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

          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

          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

          Nicko Glayre added a comment -

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

          Nicko Glayre added a comment - I attempted to solve the merging issue in the PR proposed by coderhugo and it looks like to work as expected. Could you please have a look at the following ?

          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

          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

          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

          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

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

          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/...)

          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)?

          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)?

          Mark Waite added a comment - - edited

          pyrocks 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.

          Mark Waite added a comment - - edited pyrocks 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.

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

              Created:
              Updated: