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

Unexpected behavior of pipelines, possibly due to symbol overlap

      Steps to reproduce

      1. docker run -p 8080:8080 jenkins/jenkins:2.249.1
      2. Select plugins to install: Uncheck all, select "Pipeline" (workflow-aggregator) only
      3. Create a Pipeline with the below script content
      4. Build it

      Pipeline script:

      node {
       git url: 'https://github.com/jenkinsci/workflow-aggregator-plugin/'
       sh 'ls -lA'
      } 

      Expected result:
      GitHub repo to be checked out
      or (more likely, as Git Plugin isn't installed)
      Build to fail, as the 'git' step does not exist

      Actual result:
      The 'git' statement seems to be silently dropped (interpreted as 'git' tool from git-client?). There is no hint to what the problem could be, it's like the pipeline is a different one.

      Started by user admin
      Running in Durability level: MAX_SURVIVABILITY
      [Pipeline] Start of Pipeline
      [Pipeline] node
      Running on Jenkins in /var/jenkins_home/workspace/pl
      [Pipeline] {
      [Pipeline] sh
      + ls -lA
      total 0
      [Pipeline] }
      [Pipeline] // node
      [Pipeline] End of Pipeline
      Finished: SUCCESS

          [JENKINS-63676] Unexpected behavior of pipelines, possibly due to symbol overlap

          Jesse Glick added a comment -

          Known, but unfortunately there is no way to fix this given the design of symbols, a.k.a. $class must die.

          Jesse Glick added a comment - Known, but unfortunately there is no way to fix this given the design of symbols, a.k.a. $class must die .

          Mark Waite added a comment -

          That's an interesting case.

          The git client plugin provides the symbol git that is used to name an implementation of a git tool. That symbol was added in 2016. The git plugin uses the symbol git to define an implementation of an SCM source. That symbol was added in 2017. Both symbols are valid and are used in pipelines.

          In this case, the symbol from the git plugin is not available because the plugin is not loaded. I guess in a perfect world the pipeline would fail with some form of syntax error because the 'git' symbol is loaded but the loaded git symbol does not accept a url: argument.

          Mark Waite added a comment - That's an interesting case. The git client plugin provides the symbol git that is used to name an implementation of a git tool. That symbol was added in 2016. The git plugin uses the symbol git to define an implementation of an SCM source. That symbol was added in 2017. Both symbols are valid and are used in pipelines. In this case, the symbol from the git plugin is not available because the plugin is not loaded. I guess in a perfect world the pipeline would fail with some form of syntax error because the 'git' symbol is loaded but the loaded git symbol does not accept a url: argument.

          Jesse Glick added a comment -

          Inherent limitation of JENKINS-29922.

          Jesse Glick added a comment - Inherent limitation of JENKINS-29922 .

          Jesse Glick added a comment -

          To give a bit of background: @kohsuke and I agreed to go with a symbol syntax like

          someStep someArg: someSymbol(nestedArg: 123)
          

          rather than something using closures like (I do not recall exactly what alternative syntaxes were explored)

          someStep {someArg = {someSymbol {nestedArg = 123}}}
          

          The appearance is cleaner. The problem is that the Groovy runtime will eagerly evaluate someSymbol(…) like a function call. At the time this happens, we have no way of knowing how it will be used—there is no context indicating that it will be passed to someStep. Indeed you could, if you wished, write

          def ss = someSymbol(nestedArg: 123)
          if (runtimeCondition()) {
            someStep someArg: ss
          } else {
            someStep otherArg: true
          }
          

          Therefore when DSL comes across anything that might be a symbol—by checking any registered @Symbol—and does not otherwise have any binding in the current lexical scope (step names, variables, etc.), it assumes that it is in fact a symbol, and creates an UninstantiatedDescribable from it (with no side effects). If and when the result of that “function call” is in fact passed as an argument (directly or indirectly) to some step invocation, the tree of arguments is unpacked and coerced to the expected types. The issue in this case is that the git function was called, produced a result, and then that result was ignored (not assigned to anything) and eventually garbage-collected, but there is no way that I know of to detect this and even warn about it. The only thing I can think of would be to maintain a set of all UninstantiatedDescribable’s created during the course of a build, removing elements when used in DescribableModel’s, and if at program end time there are some unused ones remaining, print a warning to the log.

          markewaite’s notion of checking whether the git symbol accepts a url argument is not feasible in general since there could be multiple symbols of different types with the same name, and we have no way of knowing which is meant to be used at the time. I suppose we could report an error in UninstantiatedDescribable.<init> if none of the known symbols match a Describable with an argument of that name, but this would be pretty fragile and limited; normally the argument checking is done when creating the final DescribableModel for the Step.

          Jesse Glick added a comment - To give a bit of background: @kohsuke and I agreed to go with a symbol syntax like someStep someArg: someSymbol(nestedArg: 123) rather than something using closures like (I do not recall exactly what alternative syntaxes were explored) someStep {someArg = {someSymbol {nestedArg = 123}}} The appearance is cleaner. The problem is that the Groovy runtime will eagerly evaluate someSymbol(…) like a function call. At the time this happens, we have no way of knowing how it will be used—there is no context indicating that it will be passed to someStep . Indeed you could, if you wished, write def ss = someSymbol(nestedArg: 123) if (runtimeCondition()) { someStep someArg: ss } else { someStep otherArg: true } Therefore when DSL comes across anything that might be a symbol—by checking any registered @Symbol —and does not otherwise have any binding in the current lexical scope (step names, variables, etc.), it assumes that it is in fact a symbol, and creates an UninstantiatedDescribable from it (with no side effects). If and when the result of that “function call” is in fact passed as an argument (directly or indirectly) to some step invocation, the tree of arguments is unpacked and coerced to the expected types. The issue in this case is that the git function was called, produced a result, and then that result was ignored (not assigned to anything) and eventually garbage-collected, but there is no way that I know of to detect this and even warn about it. The only thing I can think of would be to maintain a set of all UninstantiatedDescribable ’s created during the course of a build, removing elements when used in DescribableModel ’s, and if at program end time there are some unused ones remaining, print a warning to the log. markewaite ’s notion of checking whether the git symbol accepts a url argument is not feasible in general since there could be multiple symbols of different types with the same name, and we have no way of knowing which is meant to be used at the time. I suppose we could report an error in UninstantiatedDescribable.<init> if none of the known symbols match a Describable with an argument of that name, but this would be pretty fragile and limited; normally the argument checking is done when creating the final DescribableModel for the Step .

            Unassigned Unassigned
            danielbeck Daniel Beck
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: