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

Regression: parameters are not set on commit notification

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: git-plugin
    • Labels:
    • Environment:
      Jenkins 1.626
      Java 1.8
      CentOS 6.6 (x86_64)
    • Similar Issues:

      Description

      After upgrades (I'm not sure whether it happened after jenkins or jenkins plugin upgrade), jenkins doesn't set configured environment variables with default values in the "This build is parameterized" section on commit notification from SCM (Stash) using /git/notifyCommit request. However it works properly when build is manually triggered with "Build with parameters" button. The only difference I can see is the lack of these configured variables set – everything else works as expected.

      It is a regression – this setup is used for a very long time in our environment.

        Attachments

          Issue Links

            Activity

            emil_upnext Emil Smoleński created issue -
            Hide
            markewaite Mark Waite added a comment -

            Could you compare the behavior of the previous release (2.3.5) and the current release (2.4.0) to see if that is where the change was introduced? There was a change included in 2.4.0 which may have caused this regression, and it would help me to have it narrowed to a specific version that introduced the bug.

            Show
            markewaite Mark Waite added a comment - Could you compare the behavior of the previous release (2.3.5) and the current release (2.4.0) to see if that is where the change was introduced? There was a change included in 2.4.0 which may have caused this regression, and it would help me to have it narrowed to a specific version that introduced the bug.
            Hide
            emil_upnext Emil Smoleński added a comment -

            Thanks for quick reply!

            I downgraded git-plugin to the 2.3.5 version and it in fact solved the problem. Please let me know if you need additional testing or more information in order to fix this.

            Show
            emil_upnext Emil Smoleński added a comment - Thanks for quick reply! I downgraded git-plugin to the 2.3.5 version and it in fact solved the problem. Please let me know if you need additional testing or more information in order to fix this.
            Hide
            markewaite Mark Waite added a comment -

            It would be great to confirm that the problem also occurs when using a commitNotify from something other than Stash. I don't have a Stash installation available, so it will help if I can see it from commitNotify from a simple bare git repository, rather than requiring Stash.

            Show
            markewaite Mark Waite added a comment - It would be great to confirm that the problem also occurs when using a commitNotify from something other than Stash. I don't have a Stash installation available, so it will help if I can see it from commitNotify from a simple bare git repository, rather than requiring Stash.
            Hide
            emil_upnext Emil Smoleński added a comment - - edited

            The actual commit notification from Stash (taken from logs) looks like this:

            GET /git/notifyCommit?url=ssh%3A%2F%2Fgit%40stash.example.com%3A1234%2Fproj%2Frepo.git&branches=branch&sha1=0123456789abcdef0123456789abcdef01234567
            

            In the newest versions of Stash you can also:

            • Omit SHA1 Hash Code
            • Omit Branch Name

            (these options are disabled by default)

            HTH.

            Show
            emil_upnext Emil Smoleński added a comment - - edited The actual commit notification from Stash (taken from logs) looks like this: GET /git/notifyCommit?url=ssh%3A%2F%2Fgit%40stash.example.com%3A1234%2Fproj%2Frepo.git&branches=branch&sha1=0123456789abcdef0123456789abcdef01234567 In the newest versions of Stash you can also: Omit SHA1 Hash Code Omit Branch Name (these options are disabled by default) HTH.
            Hide
            danielbeck Daniel Beck added a comment -

            If a sha1 is provided, the build is triggered without default parameters:
            https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L281...L292

            buildParameters in the above code seems to be fed in
            https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L72
            which means it only contains explicitly passed parameters, and not those defined in the job.

            Haven't confirmed in a debugger, but seems to explain what's going on here.

            Show
            danielbeck Daniel Beck added a comment - If a sha1 is provided, the build is triggered without default parameters: https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L281...L292 buildParameters in the above code seems to be fed in https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitStatus.java#L72 which means it only contains explicitly passed parameters, and not those defined in the job. Haven't confirmed in a debugger, but seems to explain what's going on here.
            Hide
            danielbeck Daniel Beck added a comment -

            Additionally, if there are no such parameters, the build will end up with an empty ParametersAction in the sidepanel anyway. This should really only be set when there actually are parameters IMO.

            Show
            danielbeck Daniel Beck added a comment - Additionally, if there are no such parameters, the build will end up with an empty ParametersAction in the sidepanel anyway. This should really only be set when there actually are parameters IMO.
            fbelzunc Félix Belzunce Arcos made changes -
            Field Original Value New Value
            Assignee Nicolas De Loof [ ndeloof ] Félix Belzunce Arcos [ fbelzunc ]
            fbelzunc Félix Belzunce Arcos made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            jglick Jesse Glick added a comment -

            Part of a general lack of coherent handling of default parameters in Jenkins core.

            Show
            jglick Jesse Glick added a comment - Part of a general lack of coherent handling of default parameters in Jenkins core.
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-13768 [ JENKINS-13768 ]
            Hide
            markewaite Mark Waite added a comment - - edited

            In my tests with git plugin 2.4.0 and git client plugin 1.19.0, I can only duplicate the problem if I include "sha1=" in the notify commit argument list (just as Daniel Beck observed in an earlier comment). When I include no argument, and when I include only the argument "branch=", then the default parameter values are visible to the XShell and Shell build steps in my test job.

            The notify commit I used to show the problem was:

            SHA1=$(git rev-parse HEAD)
            curl -s http://localhost:8080/git/notifyCommit?url=git://mark-pc1.markwaite.net/git/mwaite/bugs/JENKINS-30178.git\&branch=master\&sha1=$SHA1
            

            The notify commit I used that does not show the problem is:

            curl -s http://localhost:8080/git/notifyCommit?url=git://mark-pc1.markwaite.net/git/mwaite/bugs/JENKINS-30178.git\&branch=master
            
            Show
            markewaite Mark Waite added a comment - - edited In my tests with git plugin 2.4.0 and git client plugin 1.19.0, I can only duplicate the problem if I include "sha1=" in the notify commit argument list (just as Daniel Beck observed in an earlier comment). When I include no argument, and when I include only the argument "branch=", then the default parameter values are visible to the XShell and Shell build steps in my test job. The notify commit I used to show the problem was: SHA1=$(git rev-parse HEAD) curl -s http://localhost:8080/git/notifyCommit?url=git://mark-pc1.markwaite.net/git/mwaite/bugs/JENKINS-30178.git\&branch=master\&sha1=$SHA1 The notify commit I used that does not show the problem is: curl -s http://localhost:8080/git/notifyCommit?url=git://mark-pc1.markwaite.net/git/mwaite/bugs/JENKINS-30178.git\&branch=master
            jglick Jesse Glick made changes -
            Labels regression
            Hide
            jglick Jesse Glick added a comment -

            Daniel Beck claims in the PR for JENKINS-27902 that it caused this.

            Show
            jglick Jesse Glick added a comment - Daniel Beck claims in the PR for JENKINS-27902 that it caused this.
            jglick Jesse Glick made changes -
            Link This issue is blocking JENKINS-27902 [ JENKINS-27902 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Mark Waite
            Path:
            src/main/java/hudson/plugins/git/GitStatus.java
            src/test/java/hudson/plugins/git/GitStatusTest.java
            http://jenkins-ci.org/commit/git-plugin/2dfd86d27a7cd4089349fd012d7d70a5e827ed81
            Log:
            Test JENKINS-30178 using GitStatus.toString

            Asserts that job parameter default values are available when a job
            is started by a notifyCommit. If the notifyCommit includes a sha1
            parameter, then the changes for JENKINS-27092 fail to assign parameters
            their default values.

            Modifying the GitStatus object to be more easily tested was simpler
            than using a TestExtension. Should eventually replace the testing
            misuse of the GitStatus.toString() method with a TestExtension of
            GitStatus.Listener.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Mark Waite Path: src/main/java/hudson/plugins/git/GitStatus.java src/test/java/hudson/plugins/git/GitStatusTest.java http://jenkins-ci.org/commit/git-plugin/2dfd86d27a7cd4089349fd012d7d70a5e827ed81 Log: Test JENKINS-30178 using GitStatus.toString Asserts that job parameter default values are available when a job is started by a notifyCommit. If the notifyCommit includes a sha1 parameter, then the changes for JENKINS-27092 fail to assign parameters their default values. Modifying the GitStatus object to be more easily tested was simpler than using a TestExtension. Should eventually replace the testing misuse of the GitStatus.toString() method with a TestExtension of GitStatus.Listener.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: fbelzunc
            Path:
            src/main/java/hudson/plugins/git/GitStatus.java
            http://jenkins-ci.org/commit/git-plugin/0a661ceaa5da94e92df97a2220a792d1eaa34317
            Log:
            [Fix JENKINS-30178] Add default parameters defined in the job

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: fbelzunc Path: src/main/java/hudson/plugins/git/GitStatus.java http://jenkins-ci.org/commit/git-plugin/0a661ceaa5da94e92df97a2220a792d1eaa34317 Log: [Fix JENKINS-30178] Add default parameters defined in the job
            Hide
            emil_upnext Emil Smoleński added a comment -

            I can confirm that this problem is resolved after upgrading git plugin to 2.4.1 and git-client plugin to 1.19.1. This issue can be closed. Thank you very much for your work, guys.

            Show
            emil_upnext Emil Smoleński added a comment - I can confirm that this problem is resolved after upgrading git plugin to 2.4.1 and git-client plugin to 1.19.1. This issue can be closed. Thank you very much for your work, guys.
            emil_upnext Emil Smoleński made changes -
            Status In Progress [ 3 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            markewaite Mark Waite made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 165265 ] JNJira + In-Review [ 209180 ]

              People

              • Assignee:
                fbelzunc Félix Belzunce Arcos
                Reporter:
                emil_upnext Emil Smoleński
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: