Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated to correctly describe class behavior #611

Merged
merged 2 commits into from Nov 28, 2017

Conversation

@bitwiseman
Copy link
Contributor

bitwiseman commented Feb 8, 2017

@jglick @rtyler
From discussion on IRC and testing, it looks like this is the current behavior of class declarations.

Is this correct behavior or should the class declaration automatically instantiate a new global variable?

@bitwiseman bitwiseman force-pushed the bitwiseman:bitwiseman-patch-1 branch from dc5fd00 to dc75f6e Feb 8, 2017
Copy link
Contributor

jglick left a comment

No. The whole point of vars, rather than src, is that the variable is created for you automatically. (And it should by convention be lowercase.)

@zbintliff
Copy link
Contributor

zbintliff commented Feb 8, 2017

Hey @jglick i was trying this example verbatim with the following Jenkinsfile and it was erroring out. @bitwiseman was trying to help me out with it.

So far the only thing I have been able to get working was a step call() implemented.

@bitwiseman
Copy link
Contributor Author

bitwiseman commented Feb 8, 2017

@jglick - I made it upper-case because it's a class name and it wasn't automatically creating an instance.

The class example (renamed acmeB) under "Defining Global Variable" produces this error when I run it as the first item in my Jenkinsfile on my Jenkins:

hudson.remoting.ProxyException: groovy.lang.MissingMethodException: No signature of method: acmeB.echo() is applicable for argument types: (org.codehaus.groovy.runtime.GStringImpl) values: [Hello, null! CAUTION: test]

(@abayer - this might interest you too)
When I put acmeB inside a Declarative Pipeline:

pipeline {
    agent any
    stages {
        stage ("Build") {
            steps {
                acmeB.caution('test')
            }
        }
    }
}

I get this error:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
WorkflowScript: 13: Expected a symbol @ line 13, column 17.
                   acmeB.caution('test')
                   ^
@bitwiseman
Copy link
Contributor Author

bitwiseman commented Feb 8, 2017

@jglick @abayer
Ah, wait. It's global variable, so does it need to go in a script block?

pipeline {
    agent any
    stages {
        stage ("Build") {
            steps {
                script {
                    acmeB.caution('test')
                }
            }
        }
    }
}

@abayer - this seems odd are global variables not available inside pipeline aside from in a script block?

@zbintliff
Copy link
Contributor

zbintliff commented Feb 9, 2017

@bitwiseman your latest example runs for me, but then I get the error the echo inside the caution() func does not exist since echo isn't a script but a step.

@zbintliff
Copy link
Contributor

zbintliff commented Feb 9, 2017

For example the following jenkinsfile:

pipeline {
  agent any
  stages{
    stage('build') {
      steps {
        echo 'hello'
        script {
          acme.name = 'foo' 
          echo acme.name //properly prints 'foo'
          acme.caution 'test' //errors with below error
        }
      }
    }
  }
}

Produces this error:

hudson.remoting.ProxyException: groovy.lang.MissingMethodException: No signature of method: acme.echo() is applicable for argument types: (org.codehaus.groovy.runtime.GStringImpl) values: [Hello, foo! CAUTION: test]
@bitwiseman bitwiseman force-pushed the bitwiseman:bitwiseman-patch-1 branch 3 times, most recently from 7bd4cc6 to ef97a32 Mar 1, 2017
@bitwiseman
Copy link
Contributor Author

bitwiseman commented Mar 1, 2017

@zbintliff @jglick @rtyler
Fix for WEBSITE-321.

Copy link
Contributor

jglick left a comment

I also recommend rewriting #defining-a-more-structured-dsl. In practice it is just really hard for people to understand how it works and fix problems that arise. Better to take plain old function calls with a Map of parameters. This syntax works fine and is easy to understand and modify and will not get you into trouble with obscure script-security limitations or Closure semantics issues.

Avoid defining global variables with methods that interact or preserve state.
Use a static class or instantiate a local variable of a class instead.
The example below shows a global variable `customer` which stores a `name` value.

This comment has been minimized.

Copy link
@jglick

jglick Sep 13, 2017

Contributor

Wait, so you warn people not to do this, then show them how to do it?

@duemir
Copy link
Contributor

duemir commented Sep 28, 2017

@bitwiseman would you be able to resurrect this one, please.

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 17, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit 1d1c70defa99e281305124df0bccf19399668185 into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[1d1c70defa99e281305124df0bccf19399668185]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 19, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit 46616390159ac6ab1ef3b2fdbd8edb3c374aa0fb into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[46616390159ac6ab1ef3b2fdbd8edb3c374aa0fb]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 21, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit 018872a61a86a8bc1636d8a5399ce8ccb24b552e into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[018872a61a86a8bc1636d8a5399ce8ccb24b552e]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 23, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit 8dd4b9f9a0da2c5b2735066a722b8bbf02307612 into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[8dd4b9f9a0da2c5b2735066a722b8bbf02307612]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 27, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit e87ab9754bea43e96bbdd9a336407eacdc321942 into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[e87ab9754bea43e96bbdd9a336407eacdc321942]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 27, 2017

The context from the Jenkins Pipeline run is:

Branch indexing
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit f384a8937799440578a33c82e61c60e833027e45 into PR head commit ef97a32c4a9b7cf7be4b4669957dc522c21a4b4a

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[f384a8937799440578a33c82e61c60e833027e45]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@jenkinsadmin
Copy link

jenkinsadmin commented Nov 28, 2017

The context from the Jenkins Pipeline run is:

Pull request #611 updated
Connecting to https://api.github.com using jenkinsadmin/****** (GitHub access token for jenkinsadmin)
Checking out git https://github.com/jenkins-infra/jenkins.io.git into /var/jenkins_home/jobs/Infra/jobs/jenkins-io.7kojb0/branches/PR-611/workspace@script to read Jenkinsfile
Fetching changes from the remote Git repository
Fetching without tags
Merging remotes/origin/master commit 69519b96d66cfefa95c255bb5e3c0a0354d67626 into PR head commit 3f60b86c01beafa51f9e21484dfe62bfa3ffab87

GitHub has been notified of this commit’s build result

hudson.plugins.git.GitException: Failed to merge AnyObjectId[69519b96d66cfefa95c255bb5e3c0a0354d67626]
	at org.jenkinsci.plugins.gitclient.JGitAPIImpl$6.execute(JGitAPIImpl.java:1560)
	at jenkins.plugins.git.MergeWithGitSCMExtension.decorateRevisionToBuild(MergeWithGitSCMExtension.java:122)
	at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1031)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1124)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:113)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:130)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:120)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:263)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:421)
Finished: FAILURE

powered by the Comment Logger

@bitwiseman bitwiseman force-pushed the bitwiseman:bitwiseman-patch-1 branch from 3f60b86 to 13ed7a5 Nov 28, 2017
Copy link
Contributor

jglick left a comment

The #accessing-steps section also contains a bunch of confusing advice (specifically about implementing methods outside the scope of the enclosing class) that you could not really follow unless you were a Groovy expert.

#defining-a-more-structured-dsl should be rewritten to not recommend the crazy DELEGATE_FIRST stuff.

Use a static class or instantiate a local variable of a class instead.
====

=== Defining custom steps

This comment has been minimized.

Copy link
@jglick

jglick Nov 28, 2017

Contributor

I would not advise referring to these as “steps” BTW. They are just functions that might look kind of like steps (or might not).

`acme` object:
Declarative Pipeline does not allow global variable usage outside of a `script`
directive
(link:https://issues.jenkins-ci.org/browse/JENKINS-42360[JENKINS-42360]).

This comment has been minimized.

Copy link
@jglick

jglick Nov 28, 2017

Contributor

@abayer correct me if I am wrong, but I think so long as you declare the method as call, you can call it as if it were a step.

This comment has been minimized.

Copy link
@bitwiseman

bitwiseman Nov 28, 2017

Author Contributor

@jglick I understand that from one perspective everything defined under vars is a "global variable. However, from a user perspective, "global variables" defined with thecall-method do not behave the same way as "global variables" defined with other method names.

Perhaps we could agree to call them "global variables" and "global functions"?

This comment has been minimized.

Copy link
@jglick

jglick Jan 3, 2018

Contributor

Any variable referring to an object with a call method can be called like a function. This is just Groovy sugar. Technically Groovy has no “functions” at all. It has methods, etc.

acme.name = 'Alice'
echo acme.name /* prints: 'Alice' */
acme.caution 'The queen is angry!' /* prints: 'Hello, Alice. CAUTION: The queen is angry!' */
@Library('utils') _

This comment has been minimized.

Copy link
@jglick

jglick Nov 28, 2017

Contributor

There is a dedicated libraries section in Declarative. @abayer can assist.

[WARNING]
====
Avoid defining global variables with methods that interact or preserve state.
Use a static class or instantiate a local variable of a class instead.

This comment has been minimized.

Copy link
@jglick

jglick Nov 28, 2017

Contributor

This is so vague as to be incomprehensible to me at least.

This comment has been minimized.

Copy link
@bitwiseman

bitwiseman Nov 28, 2017

Author Contributor

@jglick You objected to providing an example, but you also feel the text is to vague. Which would you prefer?

@rtyler
rtyler approved these changes Nov 28, 2017
@rtyler rtyler merged commit 0d0dfb8 into jenkins-infra:master Nov 28, 2017
1 check passed
1 check passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
@rtyler
Copy link
Member

rtyler commented Nov 28, 2017

I think there are some criticisms here which apply to content outside of the scope of this pull request specifically.

I suggest taking a discussion to either jenkinsci-docs@ or jenkinsci-dev to discuss the nomenclature challenges between "Global Variables" as they're presented in the Snippet Generator, compared to "global variables" as they're referenced in Shared Libraries, in addition to some other thiings.

@bitwiseman bitwiseman deleted the bitwiseman:bitwiseman-patch-1 branch Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.