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

CpsCallableInvocation.checkMismatch with @NonCPS method calling simple groovy getter (not any step)

    Details

    • Similar Issues:

      Description

      The following document provides an example how to handle native NonCPS methods like toString.

      https://wiki.jenkins.io/display/JENKINS/Pipeline+CPS+method+mismatches#PipelineCPSmethodmismatches-Overridesofnon-CPS-transformedmethods

      class Test {
        @Override
        public String toString() {
          return "Test"
        }
      }
      def builder = new StringBuilder()
      builder.append(new Test())
      echo(builder.toString()) 

       

      But this example is way to trivial, as toString in 99,99% returns variable information usually provided by a couple of handy methods of the class, like in this case with CPS getFullName method

      class Test {
          String name
      
          Test(String name) {
              this.name = name
          }
      
          String getFullName() {
              return 'some prefix ' + name
          }
      
          @NonCPS
          @Override
          public String toString() {
              return this.fullName
          }
      
      }
      
      def builder = new StringBuilder()
      builder.append(new Test('foobar'))
      echo(builder.toString())

      Previously this code worked as it doesn't call any steps as the document suggest, but now it started to raise 

      java.lang.IllegalStateException: expected to call java.lang.StringBuilder.append but wound up catching Test.getFullName; see: https://jenkins.io/redirect/pipeline-cps-method-mismatches

      Obviously it's possible to put in @NonCPS for getFullName but that simply means that almost all methods by default must be @NonCPS, and only those that call steps must be CPS ones.

      Can we have this 
      'org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService.handleMismatch'
      properly fixed as there's actually no mismatch, as even explicit toString

      (new Test('foobar')).toString() 

      doesn't help to work this around?

       

        Attachments

          Issue Links

            Activity

            Hide
            dicomj23 Dicom J added a comment -
            Show
            dicomj23 Dicom J added a comment - CC: Jesse Glick
            Hide
            jglick Jesse Glick added a comment -

            This is not a false warning; your code is actually wrong. You would need to add @NonCPS to getFullName as well. This is just an example of the general principle that a regular method must not call a CPS-transformed method.

            The more typical implementation of toString directly accesses fields, which should not have this issue, e.g.:

            @NonCPS
            @Override
            public String toString() {
                "Test[name=$name]"
            }
            
            Show
            jglick Jesse Glick added a comment - This is not a false warning; your code is actually wrong. You would need to add @NonCPS to getFullName as well. This is just an example of the general principle that a regular method must not call a CPS-transformed method. The more typical implementation of toString directly accesses fields , which should not have this issue, e.g.: @NonCPS @Override public String toString() { "Test[name=$name]" }
            Hide
            dicomj23 Dicom J added a comment -

            Jesse Glick, I figured this out and even put that into description:

            > Obviously it's possible to put in @NonCPS for getFullName but that simply means that almost all methods by default must be @NonCPS, and only those that call steps must be CPS ones.

            Anyways, thanks, it's good to know, I was just wondering as it was working before a new implementation of checkMismatch which doesn't complain about CPS itself but about mismatch of calls which is quite not that obvious but it gives good reference to the page describing all that.

            Show
            dicomj23 Dicom J added a comment - Jesse Glick , I figured this out and even put that into description: > Obviously it's possible to put in @NonCPS for getFullName but that simply means that almost all methods by default must be @NonCPS, and only those that call steps must be CPS ones. Anyways, thanks, it's good to know, I was just wondering as it was working before a new implementation of checkMismatch which doesn't complain about CPS itself but about mismatch of calls which is quite not that obvious but it gives good reference to the page describing all that.
            Hide
            jglick Jesse Glick added a comment -

            almost all methods by default must be @NonCPS

            Well, those that happen to be used from the likes of toString.

            I was just wondering as it was working before…

            The JENKINS-31314 work made no change to runtime behavior other than sometimes printing a warning. If your library code was triggering the warning shown in the issue description, then it was always broken, whether or not you noticed it.

            Show
            jglick Jesse Glick added a comment - almost all methods by default must be @NonCPS Well, those that happen to be used from the likes of toString . I was just wondering as it was working before… The JENKINS-31314 work made no change to runtime behavior other than sometimes printing a warning. If your library code was triggering the warning shown in the issue description, then it was always broken, whether or not you noticed it.

              People

              • Assignee:
                Unassigned
                Reporter:
                dicomj23 Dicom J
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: