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

[JENKINS-33016] CVS polling not detected in Pipeline #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

valones
Copy link
Contributor

@valones valones commented Feb 18, 2016

These changes were made to enable the polling functionality in the Pipeline Jobs, as described in JENKINS-33016.

@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@valones
Copy link
Contributor Author

valones commented Feb 18, 2016

Agreed, but the only solution that I see requires that I know of implementations of subclasses, which does not think it's a good idea. The ideal solution would be for the Jenkins-24141 or similar were resolved. Any suggestion?

@mc1arke
Copy link
Member

mc1arke commented Feb 21, 2016

We can't regress the widely used feature of a checkout for a non-pipeline job just to support pipeline features, so I can't accept this Pull Request until we have a suitable solution.

I'd propose implementing both:

protected PollingResult compareRemoteRevisionWith(final Job<?, ?> project, final Launcher launcher, final FilePath workspace, final TaskListener listener, final SCMRevisionState baseline, final CvsRepository[] repositories)

and

protected PollingResult compareRemoteRevisionWith(final AbstractProject<?, ?> project, final Launcher launcher, final FilePath workspace, final TaskListener listener, final SCMRevisionState baseline, final CvsRepository[] repositories)

and have them both delegate to a method with a boolean or enum flag to indicate if this is a pipeline job or not, with the current implementation of compareRemoteRevisionWith contained in this delegated method, and the code you've currently commented out re-enabled and wrapped in a condition that checks this boolean/enum flag. I've not tried running this so can't prove if it would actually work as I hope (I don't know what the actual implementation of Job/AbstractProject that's passed into this method is for pipeline and non-pipeline job), so you'd need to perform some investigation or testing around this.

@valones
Copy link
Contributor Author

valones commented Feb 22, 2016

For a pipeline is a WorkflowJob and for a non-pipeline remains a AbstractProject, but now I shouldn't have to worry about this. Reactivated the check if the Run is a AbstractBuild. Added a TODO for when JENKINS-24141 is resolved.

@valones
Copy link
Contributor Author

valones commented Feb 25, 2016

To clarify the previous comment, the check was reactivated on last commit.

@egorse
Copy link

egorse commented Aug 24, 2017

Any progress on this one?

@valones
Copy link
Contributor Author

valones commented Aug 24, 2017

This plugin no longer has a maintainer to accept the pull request. However, this fix is fully functional, you just have to compile the code. I've been using it in production environment without any problem since my last post.

@egorse
Copy link

egorse commented Sep 15, 2017

@valones Thanks! Works like a charm!

@jglick jglick changed the title Fix JENKINS-33016: Jenkins doesn't detect CVS polling on Pipeline Job [JENKINS-33016] CVS polling not detected in Pipeline Feb 28, 2018
@jglick
Copy link
Member

jglick commented Feb 28, 2018

for when JENKINS-24141 is resolved

It is; can be used most easily with a 2.60.x or newer baseline, though that is not critical. See jenkinsci/mercurial-plugin#102 for example.

@egorse
Copy link

egorse commented Aug 24, 2018

@oleg-nenashev you are last committer to cvs-plugin - do you know who might help on merging this one?

@oleg-nenashev
Copy link
Member

I believe @jglick is a maintainer.
But it's hard to discuss merging the PR while there are merge conflicts

mrmoritz01 added a commit to mrmoritz01/cvs-plugin that referenced this pull request Apr 24, 2019
…/JENKINS-33016

This fix takes the proposed solution of PR#41 jenkinsci#41 with some improvements related to plugin backwards compability
@mrmoritz01
Copy link
Contributor

Please note that I adapted your suggested fixes to create a new PR #50 that is mergable and keeps all old functionality. Thanks for providing your solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants