Custom @Symbol for StashBuildTrigger #73

guyv · 2019-04-08T07:47:02Z

Fix java.lang.NoSuchMethodError when using stashBuildTrigger as a pipelineTrigger in a pipeline script

e.g.
properties([ pipelineTriggers([ stashBuildTrigger( projectPath: null, cron: "* * * * *", stashHost: "https://my-stash", credentialsId: 'my-credentialsId', projectCode: "my-project-code", repositoryName: "my-repository", ciSkipPhrases: "NO TEST", ignoreSsl: true, checkDestinationCommit: false, checkMergeable: false, mergeOnSuccess: false, checkNotConflicted: true, onlyBuildOnComment: true, ciBuildPhrases: "test this please", deletePreviousBuildFinishComments: false, targetBranchesToBuild: null, cancelOutdatedJobsEnabled: false ) ]) ])

java.lang.NoSuchMethodError: No such DSL method 'stashBuildTrigger' found among steps [archive, bat, build, catchError, ...

at org.jenkinsci.plugins.workflow.cps.DSL.invokeMethod(DSL.java:199)
at org.jenkinsci.plugins.workflow.cps.CpsScript.invokeMethod(CpsScript.java:122)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
at java.lang.reflect.Method.invoke(Unknown Source)
at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)

…elineTrigger in a pipeline script java.lang.NoSuchMethodError: No such DSL method 'stashBuildTrigger' found among steps [archive, bat, build, catchError, ... at org.jenkinsci.plugins.workflow.cps.DSL.invokeMethod(DSL.java:199) at org.jenkinsci.plugins.workflow.cps.CpsScript.invokeMethod(CpsScript.java:122) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)

jakub-bochenski · 2019-04-08T09:44:42Z

Isn't this a duplicate of #69 ?

guyv · 2019-04-08T11:48:41Z

It is related. I first created a PR to be included in #69 (https://github.com/proski/stash-pullrequest-builder-plugin/pull/2), but the author saw it as a separate issue. @proski
This is not yet fixed in #69

jakub-bochenski · 2019-04-08T11:58:15Z

OK, so it's blocked by #69

proski · 2019-04-08T19:22:23Z

Just like #72, this PR can be applied to the master branch independently, without breaking anything. Unlike #72, this PR needs pipelines to be testable, as it defines a symbol to be used in pipelines. So the "blocked" designation is more reasonable here.

A few questions about the code:

Why is it useful to have a build trigger defined in the pipeline text? It can be defined in the GUI.
Shouldn't we add structs dependency to pom.xml? Other plugins add it.
Why is the descriptor field removed? Some plugins (e.g. Bitbucket PR builder) keep it.

We should also think hard about the name of the command. Other symbols applied to trigger descriptors are bitbucketpr, upstream, cron, pollSCM and loadingWithNPEOnTriggerStart - that's a very diverse set!

To complicate the matter, Stash is being renamed to Bitbucket Server, stash-notifier is already transitioning to the new name, so we should also plan to have "bitbucket" in the name while being distinct from the other Bitbucket plugins.

guyv · 2019-04-09T07:13:27Z

Why is it useful to have a build trigger defined in the pipeline text? It can be defined in the GUI.

We have way too many jobs to configure them all by hand. All our configuration (parameters, triggers, logrotation, ...) is included in the (git-versioned) pipeline Jenkinsfile. Jenkins takes into account this file at first build and all configuration is automatically "added" to the GUI as well.

Shouldn't we add structs dependency to pom.xml? Other plugins add it.

No problem to add, but why do we need that? I don't see it mentioned here: https://github.com/jenkinsci/bitbucket-pullrequest-builder-plugin/blob/master/pom.xml

Why is the descriptor field removed? Some plugins (e.g. Bitbucket PR builder) keep it.

The field is not used, my changes were based on
https://github.com/jenkinsci/jenkins/blob/0795e89b308ec7fcbda097858d58763d8531be8c/core/src/main/java/hudson/triggers/SCMTrigger.java. The "Bitbucket PR builder" references the descriptor field in another static method. As of now, "Stash PR builder" doesn't

proski · 2019-04-10T06:46:32Z

Why is it useful to have a build trigger defined in the pipeline text? It can be defined in the GUI.

We have way too many jobs to configure them all by hand. All our configuration (parameters, triggers, logrotation, ...) is included in the (git-versioned) pipeline Jenkinsfile. Jenkins takes into account this file at first build and all configuration is automatically "added" to the GUI as well.

OK, fair enough.

Shouldn't we add structs dependency to pom.xml? Other plugins add it.

No problem to add, but why do we need that? I don't see it mentioned here: https://github.com/jenkinsci/bitbucket-pullrequest-builder-plugin/blob/master/pom.xml

Ideally to make it compile. But I see that we are importing structs indirectly through git. The git dependency should be removed eventually when #68 is merged. Let's leave it as is for now to minimize changes.

Why is the descriptor field removed? Some plugins (e.g. Bitbucket PR builder) keep it.

The field is not used, my changes were based on
https://github.com/jenkinsci/jenkins/blob/0795e89b308ec7fcbda097858d58763d8531be8c/core/src/main/java/hudson/triggers/SCMTrigger.java. The "Bitbucket PR builder" references the descriptor field in another static method. As of now, "Stash PR builder" doesn't

The field is not used, but you also removed the call to new StashBuildTriggerDescriptor(), which may be needed. Also, the descriptor field was annotated as an @Extension. The change may be OK, but I want to understand it.

I believe the description should be changed. Why would you expect stashBuildTrigger to work? It's not a documented name, it's not in the Pipeline Snippet Generator. This commit doesn't fix a bug, as the description implies, it adds new functionality.

guyv · 2019-04-10T09:02:50Z

The field is not used, but you also removed the call to new StashBuildTriggerDescriptor(), which may be needed. Also, the descriptor field was annotated as an @Extension. The change may be OK, but I want to understand it.

Now the static inner class StashBuildTriggerDescriptor is annotated with @Extension. This is found by https://javadoc.jenkins-ci.org/hudson/ExtensionFinder.Sezpoz.html (See also this and also this)

I believe the description should be changed. Why would you expect stashBuildTrigger to work? It's not a documented name, it's not in the Pipeline Snippet Generator. This commit doesn't fix a bug, as the description implies, it adds new functionality.

Actually, it currently is in the Pipeline Snippet Generator:

Choose Sample Step: properties: Set job properties
Check the option 'Stash pull request builder' under the section 'Build Triggers'
Click 'Generate Pipeline Script'

proski · 2019-04-10T22:29:23Z

Thank you for explaning @Extension, it makes sense to me.

I tried the "properties" in the pipeline snippet generator, I actually had to rename isCancelOutdatedJobsEnabled to getCancelOutdatedJobsEnabled to fix another exception (now in the merge-all branch).

I tried using a different name in Symbol and no Symbol at all.

If I use a different name, say, stashPullRequestBuilder, that's what the snippet generator uses. So we are not required to use stashBuildTrigger.

If I use no Symbol at all, I get properties([pipelineTriggers([[$class: 'StashBuildTrigger', cancelOutdatedJobsEnabled: false,... which also seems reasonable to me. It's not like the snippet generator suggests the name stashBuildTrigger if there is no name.

I think stashPullRequestBuilder would be a better name for the trigger in the pipeline scripts.

…ine scripts

guyv · 2019-04-12T09:26:59Z

I think stashPullRequestBuilder would be a better name for the trigger in the pipeline scripts.

Do you mean only the @Symbol or also the class and the descriptor?

jakub-bochenski · 2019-04-12T14:34:01Z

Ideally to make it compile. But I see that we are importing structs indirectly through git. The git dependency should be removed eventually when #68 is merged. Let's leave it as is for now to minimize changes.

If we use stucts classes in our source code (which we do, e.g Symbol) we should have a direct compile-scope dependency in pom.xml
Not enforcing this by default is a long-standing issue in Maven. Please add it now. Going forward we should add an automatic check for this kind of problems

jakub-bochenski · 2019-04-12T14:35:26Z

If I use no Symbol at all, I get properties([pipelineTriggers([[$class: 'StashBuildTrigger', cancelOutdatedJobsEnabled: false,... which also seems reasonable to me. It's not like the snippet generator suggests the name stashBuildTrigger if there is no name.

This is also how I understand this works. You can use a default identifier derived from the class name or use Symbol to override it.

guyv · 2019-04-14T11:36:47Z

If we use stucts classes in our source code (which we do, e.g Symbol) we should have a direct compile-scope dependency in pom.xml

I fail to see where we depend on structs: org.jenkinsci.Symbol is not part of org.jenkins-ci.plugins:structs, but of org.jenkins-ci:symbol-annotation. So I added the latter to the pom.
There are no imports with structs in the package name.

jakub-bochenski · 2019-04-15T09:16:33Z

You are right, I just did a quick google and found this: https://github.com/jenkinsci/structs-plugin/blob/master/annotation/src/main/java/org/jenkinsci/Symbol.java which made me think it's in the structs plugin.
When I open the project in Idea I see the annotation is indeed imported via org.jenkins-ci:symbol-annotation

jakub-bochenski · 2019-04-15T09:18:08Z

pom.xml

+    </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci</groupId>
+      <artifactId>annotation-indexer</artifactId>


Why do we need annotation-indexer?

by only adding the dependency on org.jenkins-ci:symbol-annotation

I get following build error:

[INFO] --- maven-enforcer-plugin:3.0.0-M1:enforce (display-info) @ stash-pullrequest-builder ---
[INFO] Ignoring requireUpperBoundDeps in com.google.guava:guava
[INFO] Ignoring requireUpperBoundDeps in com.google.code.findbugs:jsr305
[WARNING] Rule 4: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for org.jenkins-ci:annotation-indexer:1.9 paths to dependency are:
+-org.jenkins-ci.plugins:stash-pullrequest-builder:1.9-SNAPSHOT
+-org.jenkins-ci:symbol-annotation:1.3
+-org.jenkins-ci:annotation-indexer:1.9
and
+-org.jenkins-ci.plugins:stash-pullrequest-builder:1.9-SNAPSHOT
+-org.jenkins-ci.main:jenkins-core:2.60.1
+-org.jenkins-ci:annotation-indexer:1.11
and
+-org.jenkins-ci.plugins:stash-pullrequest-builder:1.9-SNAPSHOT
+-org.kohsuke:access-modifier-annotation:1.14
+-org.jenkins-ci:annotation-indexer:1.4
and
+-org.jenkins-ci.plugins:stash-pullrequest-builder:1.9-SNAPSHOT
+-org.jenkins-ci.plugins:git:3.0.0
+-com.infradna.tool:bridge-method-annotation:1.14
+-org.jenkins-ci:annotation-indexer:1.4
and
+-org.jenkins-ci.plugins:stash-pullrequest-builder:1.9-SNAPSHOT
+-org.jenkins-ci.main:jenkins-core:2.60.1
+-org.jenkins-ci:bytecode-compatibility-transformer:1.8
+-org.jenkins-ci:annotation-indexer:1.4
]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

OK. The way to solve this is to lock the version using dependencyManagement, not to add a new direct dependency.
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management

thx for pointing that out. Moved to dependencyManagement

jakub-bochenski · 2019-07-01T16:42:00Z

@proski there's a bunch of conflicts in StashBuildTrigger now. Can you take a look or do we leave it as is until #69 is merged?

proski · 2019-07-02T00:16:30Z

I'm not convinced this PR needs to be applied at all. The description misrepresents the issue as a bug. It is possible to specify the trigger in the properties without an alias. That's what the pipeline snippet generator would produce.

Another issue is that the alias would have the name "stash" in it. Stash is Bitbucket Server now. We should plan how to switch to using the new name. This PR gets us in the opposite direction.

I'm ready to check this PR for the good bits, but only after #69 is merged. Considering that the simpler and non-controversial PR #72 has been in the "blocked" state for months, I'm not interested in spending time on any code that depends on the pipeline support, even indirectly.

jakub-bochenski · 2019-07-21T20:36:48Z

@proski I think we can close this PR unless we want a custom symbol (which I think we don't)?

proski · 2019-07-21T20:50:28Z

I agree. We can get back to it later. I don't care about the custom symbol. The pipeline generator doesn't require it.

jakub-bochenski · 2019-07-22T15:16:13Z

On second thoughts (see https://github.com/jenkinsci/stash-pullrequest-builder-plugin/compare/1ada98a68af0fd89fe227ff9935515261492b646..c58ca972274cc22fcc316c7c1cb56fef5cd2f291#discussion_r305900208) I think we need to settle on a symbol to use in Jenkinsfiles before we do a release

herman-nu · 2019-08-28T11:23:15Z

Thank you for explaning @Extension, it makes sense to me.

I tried the "properties" in the pipeline snippet generator, I actually had to rename isCancelOutdatedJobsEnabled to getCancelOutdatedJobsEnabled to fix another exception (now in the merge-all branch).

I tried using a different name in Symbol and no Symbol at all.

If I use a different name, say, stashPullRequestBuilder, that's what the snippet generator uses. So we are not required to use stashBuildTrigger.

If I use no Symbol at all, I get properties([pipelineTriggers([[$class: 'StashBuildTrigger', cancelOutdatedJobsEnabled: false,... which also seems reasonable to me. It's not like the snippet generator suggests the name stashBuildTrigger if there is no name.

I think stashPullRequestBuilder would be a better name for the trigger in the pipeline scripts.

Can you post an sample of how to use properties in a Jenkinsfile. I'll be glad to update the documentation with an example. I've tried all of the following without success.

Child of `stage > steps`

pipeline {
.
.
   stages {
       stage("Setup Properties") {
           steps {
               properties([
                   pipelineTriggers([
                       [$class: 'StashBuildTrigger', 
                           checkDestinationCommit: true, 
                           checkNotConflicted: true, 
                           credentialsId: 'bitbucket-user', 
                           cron: 'H/10 * * * *', 
                           projectCode: 'KEY', 
                           repositoryName: 'myrepo', 
                           stashHost: 'https://my-bitbucket.com/, 
                           targetBranchesToBuild: 'master']
                   ])
               ])
           }
       }
   }
}

Child of the pipeline step

pipeline {
   .
   .
   properties([
       pipelineTriggers([
           [$class: 'StashBuildTrigger', 
               checkDestinationCommit: true, 
               checkNotConflicted: true, 
               credentialsId: 'bitbucket-user', 
               cron: 'H/10 * * * *', 
               projectCode: 'KEY', 
               repositoryName: 'myrepo', 
               stashHost: 'https://my-bitbucket.com/, 
               targetBranchesToBuild: 'master']
       ])
   ])
   stages {
       stage("Setup Properties") {
           steps {
               ...   
           }
       }
   }
}

Outside the pipeline block


properties([
    pipelineTriggers([
        [$class: 'StashBuildTrigger', 
            checkDestinationCommit: true, 
            checkNotConflicted: true, 
            credentialsId: 'bitbucket-user', 
            cron: 'H/10 * * * *', 
            projectCode: 'KEY', 
            repositoryName: 'myrepo', 
            stashHost: 'https://my-bitbucket.com/, 
            targetBranchesToBuild: 'master']
    ])
])
pipeline {
.
.
   stages {
       stage("Setup Properties") {
           steps {
               ...
           }
       }
   }
}

proski · 2019-09-10T19:59:47Z

@herman-nu In the declarative pipeline (i.e. the one with the top-level pipeline block), the properties should be in the script block:

pipeline {
    stages {
        stage('Setup') {
            steps {
                script {
                    properties...

Some standard properties can be in the options block inside the top-level pipeline block, but trigger configuration cannot be used there.

jakub-bochenski added the blocked label Apr 8, 2019

proski mentioned this pull request Apr 9, 2019

Pipeline support #69

Merged

use stashPullRequestBuilder instead of stashBuildTrigger in the pipel…

77aec87

…ine scripts

jakub-bochenski self-requested a review April 12, 2019 14:34

added compile dependency on symbol-annotation

3d004a0

added upperbound for annotation-indexer

ccee60f

jakub-bochenski reviewed Apr 15, 2019

View reviewed changes

Guy Veraghtert and others added 2 commits April 17, 2019 08:53

moved annotation-indexer to dependencyManagement

2557a1c

Merge branch 'master' into fix_pipeline_dsl_method

338d825

jakub-bochenski removed the blocked label Jul 22, 2019

jakub-bochenski changed the title ~~NoSuchMethodError when using stashBuildTrigger as a pipelineTrigger~~ Custom @Symbol for StashBuildTrigger Aug 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom @Symbol for StashBuildTrigger #73

Custom @Symbol for StashBuildTrigger #73

guyv commented Apr 8, 2019

jakub-bochenski commented Apr 8, 2019

guyv commented Apr 8, 2019 •

edited

Loading

jakub-bochenski commented Apr 8, 2019

proski commented Apr 8, 2019

guyv commented Apr 9, 2019

proski commented Apr 10, 2019

guyv commented Apr 10, 2019

proski commented Apr 10, 2019

guyv commented Apr 12, 2019 •

edited

Loading

jakub-bochenski commented Apr 12, 2019

jakub-bochenski commented Apr 12, 2019

guyv commented Apr 14, 2019 •

edited

Loading

jakub-bochenski commented Apr 15, 2019

jakub-bochenski Apr 15, 2019

guyv Apr 16, 2019

jakub-bochenski Apr 16, 2019

guyv Apr 17, 2019

jakub-bochenski commented Jul 1, 2019

proski commented Jul 2, 2019

jakub-bochenski commented Jul 21, 2019

proski commented Jul 21, 2019

jakub-bochenski commented Jul 22, 2019

herman-nu commented Aug 28, 2019

proski commented Sep 10, 2019

Custom @Symbol for StashBuildTrigger #73

Are you sure you want to change the base?

Custom @Symbol for StashBuildTrigger #73

Conversation

guyv commented Apr 8, 2019

jakub-bochenski commented Apr 8, 2019

guyv commented Apr 8, 2019 • edited Loading

jakub-bochenski commented Apr 8, 2019

proski commented Apr 8, 2019

guyv commented Apr 9, 2019

proski commented Apr 10, 2019

guyv commented Apr 10, 2019

proski commented Apr 10, 2019

guyv commented Apr 12, 2019 • edited Loading

jakub-bochenski commented Apr 12, 2019

jakub-bochenski commented Apr 12, 2019

guyv commented Apr 14, 2019 • edited Loading

jakub-bochenski commented Apr 15, 2019

jakub-bochenski Apr 15, 2019

Choose a reason for hiding this comment

guyv Apr 16, 2019

Choose a reason for hiding this comment

jakub-bochenski Apr 16, 2019

Choose a reason for hiding this comment

guyv Apr 17, 2019

Choose a reason for hiding this comment

jakub-bochenski commented Jul 1, 2019

proski commented Jul 2, 2019

jakub-bochenski commented Jul 21, 2019

proski commented Jul 21, 2019

jakub-bochenski commented Jul 22, 2019

herman-nu commented Aug 28, 2019

Child of stage > steps

Child of the pipeline step

Outside the pipeline block

proski commented Sep 10, 2019

guyv commented Apr 8, 2019 •

edited

Loading

guyv commented Apr 12, 2019 •

edited

Loading

guyv commented Apr 14, 2019 •

edited

Loading

Child of `stage > steps`