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

Git plugin polls incorrect branch

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Won't Fix
    • Component/s: git-plugin
    • Labels:
    • Environment:
      Git Plugin 2.2.7, Jenkins 1.509.2
    • Similar Issues:

      Description

      I have a job configured with "branches to build" as "origin/release/flow". The polling log shows that the git plugin is erroneously only using the last component of the branch name to poll:

      git ls-remote -h git@github.mycompany.lan:org/Repo.git flow # timeout=10

      This matches six branches on the server. The plugin uses the first one, which is "refs/heads/appstore/flow". The hashes never match, so the plugin triggers the build every time.

      As a workaround I've changed "branches to build" to "refs/heads/release/flow", but this should be fixed.

        Attachments

          Activity

          Hide
          markewaite Mark Waite added a comment - - edited

          Thanks for your comments. Unfortunately, it can't be fixed without a significant risk of breaking compatibility with many of the existing users of the git plugin.

          That simplified syntax is used by many jobs in many locations and changing its behavior is likely to have many unintended side effects. That is why Alexander Link's changes to allow refs/heads syntax were used instead of altering the behavior of the existing syntax. The refs/heads syntax allows unambiguous specification of the branch name without altering the meaning of the existing simplified syntax.

          That is also why the description of the regex form of branch specification was added to the online help about the same time. The regex form of branch specification had been in the code for quite a while, without online help, when Alexander and I discovered it lurking there. Since it was in the code, we included it in the online help as another way of handling this condition.

          There are too many users of the git plugin using it in too many diverse and interesting ways for us to change so fundamental a behavior, even if the behavior seems flawed.

          For a good discussion of the challenges associated with the branches to build specification, refer to commit 1, commit 2, commit 3, commit 4, and commit 5

          Show
          markewaite Mark Waite added a comment - - edited Thanks for your comments. Unfortunately, it can't be fixed without a significant risk of breaking compatibility with many of the existing users of the git plugin. That simplified syntax is used by many jobs in many locations and changing its behavior is likely to have many unintended side effects. That is why Alexander Link's changes to allow refs/heads syntax were used instead of altering the behavior of the existing syntax. The refs/heads syntax allows unambiguous specification of the branch name without altering the meaning of the existing simplified syntax. That is also why the description of the regex form of branch specification was added to the online help about the same time. The regex form of branch specification had been in the code for quite a while, without online help, when Alexander and I discovered it lurking there. Since it was in the code, we included it in the online help as another way of handling this condition. There are too many users of the git plugin using it in too many diverse and interesting ways for us to change so fundamental a behavior, even if the behavior seems flawed. For a good discussion of the challenges associated with the branches to build specification, refer to commit 1 , commit 2 , commit 3 , commit 4 , and commit 5
          Hide
          n8gray Nathan Gray added a comment - - edited

          As somebody who has done way too much git scripting I can appreciate how tricky this stuff is, but the plugin is just plain doing the wrong thing. I've specified an unambiguous branch and it's randomly dropped part of the name. I've asked it to poll "origin/release/flow" and it's polling "flow". The "origin" part is correctly removed but what happened to "release"?

          Note that this is a regression from the last version of the plugin I used, where "origin/release/flow" did the right thing. I'm not asking for new behavior, I'm asking for previously working branch names to work again.

          Looking at the code, normalizeBranchSpec is broken (and there's even a comment in the code saying so). Taking the last component of the branch name is going to be wrong a lot. I wonder if you're aware that git already provides branch normalization with "git rev-parse --symbolic-full-name":

          [nathangray@n8bookpro]% git rev-parse --symbolic-full-name release/flow
          refs/heads/release/flow
          [nathangray@n8bookpro]% git rev-parse --symbolic-full-name heads/release/flow
          refs/heads/release/flow
          [nathangray@n8bookpro]% git rev-parse --symbolic-full-name origin/release/flow
          refs/remotes/origin/release/flow
          [nathangray@n8bookpro]% git rev-parse --symbolic-full-name remotes/origin/release/flow
          refs/remotes/origin/release/flow
          

          Couldn't you use that instead of trying to do it yourself?

          Show
          n8gray Nathan Gray added a comment - - edited As somebody who has done way too much git scripting I can appreciate how tricky this stuff is, but the plugin is just plain doing the wrong thing. I've specified an unambiguous branch and it's randomly dropped part of the name. I've asked it to poll "origin/release/flow" and it's polling "flow". The "origin" part is correctly removed but what happened to "release"? Note that this is a regression from the last version of the plugin I used, where "origin/release/flow" did the right thing. I'm not asking for new behavior, I'm asking for previously working branch names to work again. Looking at the code, normalizeBranchSpec is broken (and there's even a comment in the code saying so). Taking the last component of the branch name is going to be wrong a lot . I wonder if you're aware that git already provides branch normalization with "git rev-parse --symbolic-full-name": [nathangray@n8bookpro]% git rev-parse --symbolic-full-name release/flow refs/heads/release/flow [nathangray@n8bookpro]% git rev-parse --symbolic-full-name heads/release/flow refs/heads/release/flow [nathangray@n8bookpro]% git rev-parse --symbolic-full-name origin/release/flow refs/remotes/origin/release/flow [nathangray@n8bookpro]% git rev-parse --symbolic-full-name remotes/origin/release/flow refs/remotes/origin/release/flow Couldn't you use that instead of trying to do it yourself?
          Hide
          markewaite Mark Waite added a comment - - edited

          Whether we use git rev-parse or some other form, we still have the compatibility problem. The unfortunate definition of that field is that it takes the text from the right of the rightmost slash and uses that as the branch name. I wish it didn't do that, but that is what it does. With over 40 000 installations of the git plugin, I'm confident there are many cases where that behavior is crucial to their use of the plugin. I can't justify breaking their use of the plugin to resolve my concern that the branch specification they used is not doing the right thing.

          Users (like you and me) who need more precise (or accurate) specification of the branch name can either use the refs/heads syntax or can use a regular expression to define the branch to build. Both are described in the help. and both are available for use, without breaking compatibility for existing users. Those existing users may just be "lucky" that the current behavior is what they want, but even if they are just "lucky", they will be dissatisfied if the behavior changes.

          Note that this is a regression from the last version of the plugin I used, where "origin/release/flow" did the right thing. I'm not asking for new behavior, I'm asking for previously working branch names to work again.

          Which version of the git plugin did the right thing with "origin/release/flow"? I haven't investigated the entire history of the git plugin, but as far as I know, the plugin has behaved this way since at least git plugin 2.0.

          Show
          markewaite Mark Waite added a comment - - edited Whether we use git rev-parse or some other form, we still have the compatibility problem. The unfortunate definition of that field is that it takes the text from the right of the rightmost slash and uses that as the branch name. I wish it didn't do that, but that is what it does. With over 40 000 installations of the git plugin, I'm confident there are many cases where that behavior is crucial to their use of the plugin. I can't justify breaking their use of the plugin to resolve my concern that the branch specification they used is not doing the right thing. Users (like you and me) who need more precise (or accurate) specification of the branch name can either use the refs/heads syntax or can use a regular expression to define the branch to build. Both are described in the help. and both are available for use, without breaking compatibility for existing users. Those existing users may just be "lucky" that the current behavior is what they want, but even if they are just "lucky", they will be dissatisfied if the behavior changes. Note that this is a regression from the last version of the plugin I used, where "origin/release/flow" did the right thing. I'm not asking for new behavior, I'm asking for previously working branch names to work again. Which version of the git plugin did the right thing with "origin/release/flow"? I haven't investigated the entire history of the git plugin, but as far as I know, the plugin has behaved this way since at least git plugin 2.0.
          Hide
          n8gray Nathan Gray added a comment -

          It's your plugin and you can do whatever you want, but if there's somebody out there who's relying on a branch spec of "remoteName/bar/baz" to build "bar/baz" but match a randomly selected branch ending in "baz" for polling then they are seriously deranged. There is no rational use for this behavior. It's a good and noble thing to provide stability and compatibility for your users, but locking in senseless bugs is just pouring cement around your own feet.

          If you really must keep this bizarre behavior (which is completely different from the way git branch names work in every other context outside your plugin) you really should do more to document it. The current documentation says this:

          <remoteRepoName>/<branchName>
          Tracks/checks out the specified branch. If ambiguous the first result is taken, which is not necessarily the expected one.
          Better use refs/heads/<branchName>.
          E.g. origin/master
          

          This is only partially true. The branch I specified was unambiguous and worked fine for building but caused polling to fail. In fact, it takes the last "path" component of the branch and uses that, but only for polling it would appear. Good luck trying to explain that. Your best bet is probably to just display a bright red warning if the branch spec doesn't start with one of your "good" prefixes.

          RE your question, the version that worked was a 1.x version. I'm not sure exactly which because we went through a few 2.x versions trying to debug this polling problem.

          Show
          n8gray Nathan Gray added a comment - It's your plugin and you can do whatever you want, but if there's somebody out there who's relying on a branch spec of "remoteName/bar/baz" to build "bar/baz" but match a randomly selected branch ending in "baz" for polling then they are seriously deranged. There is no rational use for this behavior. It's a good and noble thing to provide stability and compatibility for your users, but locking in senseless bugs is just pouring cement around your own feet. If you really must keep this bizarre behavior (which is completely different from the way git branch names work in every other context outside your plugin) you really should do more to document it. The current documentation says this: <remoteRepoName>/<branchName> Tracks/checks out the specified branch. If ambiguous the first result is taken, which is not necessarily the expected one. Better use refs/heads/<branchName>. E.g. origin/master This is only partially true. The branch I specified was unambiguous and worked fine for building but caused polling to fail. In fact, it takes the last "path" component of the branch and uses that, but only for polling it would appear. Good luck trying to explain that. Your best bet is probably to just display a bright red warning if the branch spec doesn't start with one of your "good" prefixes. RE your question, the version that worked was a 1.x version. I'm not sure exactly which because we went through a few 2.x versions trying to debug this polling problem.

            People

            • Assignee:
              ndeloof Nicolas De Loof
              Reporter:
              n8gray Nathan Gray
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: