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

Mishandling of binary methods accepting Closure

    Details

    • Similar Issues:

      Description

      At least some closures are executed only once inside of Groovy CPS DSL scripts managed by the workflow plugin.

      Steps to reproduce:
      1. Create a new workflow with the following script:
      node {
      [1, 2, 3].each

      { println it }

      println "abc".replaceAll(/[a-z]/)

      { it.toUpperCase() }

      }
      2. Build the workflow

      Actual output:
      Started by user anonymous
      Running: Allocate node : Start
      Running on master in /var/lib/jenkins/jobs/testflow/workspace
      Running: Allocate node : Body : Start
      Running: Print Message
      1
      Running: Print Message
      A
      Running: Allocate node : Body : End
      Running: Allocate node : End
      Running: End of Workflow
      Finished: SUCCESS

      Expected output:
      Started by user anonymous
      Running: Allocate node : Start
      Running on master in /var/lib/jenkins/jobs/testflow/workspace
      Running: Allocate node : Body : Start
      Running: Print Message
      1
      Running: Print Message
      2
      Running: Print Message
      3
      Running: Print Message
      ABC
      Running: Allocate node : Body : End
      Running: Allocate node : End
      Running: End of Workflow
      Finished: SUCCESS

        Attachments

          Issue Links

            Activity

            Hide
            jglick Jesse Glick added a comment -

            Ulad Muraveika good catch. Could you file an issue for this in the WEBSITE project? Of course if you have the time to do it, we welcome PRs updating documentation. (In the case of code samples, this means testing that the proposed syntax actually works in current releases.)

            Show
            jglick Jesse Glick added a comment - Ulad Muraveika good catch. Could you file an issue for this in the WEBSITE project? Of course if you have the time to do it, we welcome PRs updating documentation. (In the case of code samples, this means testing that the proposed syntax actually works in current releases.)
            Hide
            batmat Baptiste Mathus added a comment -

            Ulad Muraveika or anyone having the time to fix this, please note that https://jenkins.io is fully backed by https://github.com/jenkins-infra/jenkins.io, so a PR there is the only thing needed to fix it.

            (Side note: not only guys can fix this issue )

            Show
            batmat Baptiste Mathus added a comment - Ulad Muraveika or anyone having the time to fix this, please note that https://jenkins.io is fully backed by https://github.com/jenkins-infra/jenkins.io , so a PR there is the only thing needed to fix it. (Side note: not only guys can fix this issue )
            Hide
            murme Ulad Muraveika added a comment -

            Baptiste Mathus, I'll do this, because I had funny time with digging around NonSerializable exceptions and have created small documentation file with all such cases for my team. Now it is time to share it

            Thanks for additional info and reminder!

            Show
            murme Ulad Muraveika added a comment - Baptiste Mathus , I'll do this, because I had funny time with digging around NonSerializable exceptions and have created small documentation file with all such cases for my team. Now it is time to share it Thanks for additional info and reminder!
            Hide
            ilatypov Ilguiz Latypov added a comment - - edited

            Another limit is still annoying because it silently returns from a NonCPS method calling a CPS method.

             

                @NonCPS
                public static String readCurrentVersion(String fileContent) {
                    def xml = new XmlSlurper().parseText(deBOM(fileContent))
                    String retval = "${xml.metadata.version}"
                    xml = null
                    return retval
                }
            
                // Omitted @NonCPS here
                public static CharSequence deBOM(CharSequence s) {
                    [...]
                    return s
                }
            

            Calling readCurrentVersion from the above unexpectedly returned the entire fileContent.  This is probably another unexpected behaviour but seems worth mentioning in the documentation.

             

            Show
            ilatypov Ilguiz Latypov added a comment - - edited Another limit is still annoying because it silently returns from a NonCPS method calling a CPS method.   @NonCPS public static String readCurrentVersion( String fileContent) { def xml = new XmlSlurper().parseText(deBOM(fileContent)) String retval = "${xml.metadata.version}" xml = null return retval } // Omitted @NonCPS here public static CharSequence deBOM(CharSequence s) { [...] return s } Calling readCurrentVersion from the above unexpectedly returned the entire fileContent.  This is probably another unexpected behaviour but seems worth mentioning in the documentation.  
            Hide
            jglick Jesse Glick added a comment -

            Ilguiz Latypov this ought to have been addressed (in the form of a warning) by JENKINS-31314.

            Show
            jglick Jesse Glick added a comment - Ilguiz Latypov this ought to have been addressed (in the form of a warning) by JENKINS-31314 .

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                dtschan Daniel Tschan
              • Votes:
                108 Vote for this issue
                Watchers:
                136 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: