search

nemccarthy / stash-pullrequest-builder-plugin

A Jenkins plugin for Building Stash Pull Requests
https://wiki.jenkins-ci.org/display/JENKINS/Stash+pullrequest+builder+plugin
Other
64 stars 130 forks source link

In WorkflowJob, fails to delete "build started" comment and add "build success/failure" comment #135

Open ghost opened 6 years ago

ghost commented 6 years ago

This is with an hpi build from the commit https://github.com/nemccarthy/stash-pullrequest-builder-plugin/commit/b2be6792adc2e9e3670189908b09b2331da05d30 manually installed into a jenkins docker container.

The initial build comment is successfully posted, but fails to be deleted once the workflow job completes. I've traced this to the following scenario:

  1. StashPullRequestBuilder run is invoked
  2. StashRepository init is invoked followed by all methods to check if build should be triggered
  3. StashRepository recognizes build should be triggered, posts "build started" comment
  4. StashBuilds onStarted is invoked
  5. StashRespository client (StashApiClient) is finalized
  6. WorkflowRun finish is invoked with SUCCESS result
  7. StashBuilds onCompleted is invoked
  8. StashRespository tries to delete "build started" comment, but StashRespository client has already been finalized and so is null

I'm confused as to why StashRespository.client gets finalized before the StashRespository object itself is finalized. I see nowhere where StashRespository is getting set to null, so StashRespository should still have a reference to the client created in the StashRepository.init method, should it not?

I was able to fix this by simply always allocating a new StashApiClient when needed. This is inefficient so I'd much rather understand why the StashApiClient is being garbage collected so soon.

Dec 20, 2017 4:41:01 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository isBuildTarget
INFO: Building PR: 1
Dec 20, 2017 4:41:01 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository addFutureBuildTasks
INFO: TETO-test-PR client: stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient@613d0a49
Dec 20, 2017 4:41:01 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository getAdditionalParameters
INFO: TETO-test-PR client: stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient@613d0a49
Dec 20, 2017 4:41:01 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository postBuildStartCommentTo
INFO: TETO-test-PR client: stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient@613d0a49
Dec 20, 2017 4:41:02 AM stashpullrequestbuilder.stashpullrequestbuilder.StashBuildListener onStarted
INFO: BuildListener onStarted called.
Dec 20, 2017 4:41:02 AM stashpullrequestbuilder.stashpullrequestbuilder.StashBuilds onStarted
INFO: client: stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient@613d0a49
Dec 20, 2017 4:41:07 AM stashpullrequestbuilder.stashpullrequestbuilder.stash.StashApiClient finalize
INFO: StashApiClient finalize
Dec 20, 2017 4:41:13 AM org.jenkinsci.plugins.workflow.job.WorkflowRun finish
INFO: TETO-test-PR #14 completed: SUCCESS
Dec 20, 2017 4:41:13 AM stashpullrequestbuilder.stashpullrequestbuilder.StashBuilds onCompleted
INFO: client: null
Dec 20, 2017 4:41:13 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository deletePullRequestComment
INFO: TETO-test-PR pullRequestId: 1, commentId: 689695
Dec 20, 2017 4:41:13 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository deletePullRequestComment
INFO: TETO-test-PR this.client: null
Dec 20, 2017 4:41:13 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository deletePullRequestComment
INFO: TETO-test-PR client: null
Dec 20, 2017 4:41:13 AM hudson.model.listeners.RunListener report
WARNING: RunListener failed
java.lang.NullPointerException
    at stashpullrequestbuilder.stashpullrequestbuilder.StashRepository.deletePullRequestComment(StashRepository.java:186)
    at stashpullrequestbuilder.stashpullrequestbuilder.StashBuilds.onCompleted(StashBuilds.java:63)
    at stashpullrequestbuilder.stashpullrequestbuilder.StashBuildListener.onCompleted(StashBuildListener.java:34)
    at hudson.model.listeners.RunListener.fireCompleted(RunListener.java:211)
    at org.jenkinsci.plugins.workflow.job.WorkflowRun.finish(WorkflowRun.java:664)
...
thomasrutkowski commented 6 years ago

I have a similar problem, build status is updated, but the ticker for successful build is not. Could you share your solution, however inelegant?

It looks like this otherwise very fine plugin has become orphaned as of last year. I'm looking for an alternate solution with post-commit hooks, but stash does not make that easy for our use-case.

ghost commented 6 years ago

My poor solution is to remove the internal "client" object in the StashRepository class and add a "getClient" method that returns a new StashClient() every time it is invoked, and then replace all references to the internal "client" object with an invocation of getClient(). It can be seen here: https://github.com/nemccarthy/stash-pullrequest-builder-plugin/compare/master...vproman:issues/135

FWIW my team just ended up utilizing the old-style jenkins jobs for our PR builder since those still work, it's just pipeline-style jobs that don't work.