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
Allow merge requests to trigger multibranch builds. #857
Conversation
@pdudits if you have some time, could you review this? I was just feeling my way along a bit blindly here :) |
Without a test case it's surely hard to tell the effect. Since this is entirely new branch of code, it might be safe to integrate. I have some proposal to cleaning that up, though, I'll post a review. |
@pdudits thanks very much! |
private final static Logger LOGGER = Logger.getLogger(PushBuildAction.class.getName()); | ||
private final Item project; | ||
|
||
public SCMSourceOwnerNotifier(Item project) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class only makes sense when given an SCMSourceOwner. At the call site this is already verified, so it's nicer to just allow these as projects
public SCMSourceOwnerNotifier(Item project) { | |
public SCMSourceOwnerNotifier(SCMSourceOwner project) { |
} | ||
|
||
public void run() { | ||
for (SCMSource scmSource : ((SCMSourceOwner) project).getSCMSources()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you can remove any casts to SCMSourceOwner in the method
if (scmSource instanceof GitSCMSource) { | ||
GitSCMSource gitSCMSource = (GitSCMSource) scmSource; | ||
try { | ||
if (new URIish(gitSCMSource.getRemote()).equals(new URIish(gitSCMSource.getRemote()))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read this as if (a.equals(a))
. Both the constructor and getRemote() return the same thing. So the try block, and the if block can be removed.
Are there any specific reasons to why this MR is not being accepted? |
+1 to accept this MR, currently I am using two separate job, one multibranch (created using DSL) and one normal pipeline for the MR, so the ideal solution for me would be to cover MR with the multibranch plugin. |
This will in the end create an unnecessary work... If it won't get merged that's what I will have to do also.. |
Sure. We used Gitflow workflow with develop and master branches. I defined a normal pipeline job with this configuration (same gitlab repository) including MR to master (see configuration in the second snapshot): You also need to add a new MR webhook in GitLab to the jenkins job I realized just now, that it is possible to modify the jenkins jobs multibranch pipeline is generating, so probably it is not needed to add a new job, just only modify the one you need to Pipeline script from SCM: |
@carloscaverobarca Thank you so much for the reply! Unfortunately I don't have that option in a normal Pipeline job. I wonder if you have installed a plugin to get that option. I use Jenkins from stable image inside Docker. Maybe I should use latest? |
@erenatas, I guess you need to install also the pipeline plugins, my installation is this one: Hope it helps! |
@carloscaverobarca you are the best! Thank you so much. I will try and let you know when I'm back at my laptop. Cheers! |
@pdudits Are there any possibilities for reviewing and accepting this soon? Otherwise I will work on a workaround. Thanks! |
@erenatas I am not the member if the repo, just helping out with the code reviews. So merging fully depends on time @omehegan is able to allocate for this. What I can suggest though is:
|
@erenatas BTW found this interesting blog to install the plugins you need (even adapt your git repo) to generate your Jenkins developer docker image |
I haven't had the time to try the plugin yet due to the exams of my master's degree. Instead of doing it like this, I thought of getting backups of I need to build this plugin and try it on Monday. I will definitely let you know. |
Any plans to merge this PR? |
Sorry I have forgotten to reply to this ticket. I couldn't make it work unfortunately. |
I could not make it work. I installed the version and Merge Requests are not launched. Currently I have no time to help with the source code. For the moment I am using the multibranch job and another pipeline job for the Merge Requests. |
Fixes #416.
The plugin's original implementation of Pipeline Multibranch job triggering only supported triggering from GitLab push webhooks. My understanding is that this was because, in the case of Multibranch jobs, the plugin just triggers branch indexing and lets Jenkins trigger the builds themselves for whichever branches have changes. As a result, there is no way to pass any of the data from the GitLab webhook to a Multibranch job, and so when building from an MR the job would not have any of the data about source and target branch which could be used for performing a local merge and so on.
However, enough people have complained about the fact that you cannot even trigger a Multibranch build from a Merge Request that I have tried to remedy that. A better long-term solution would be to add Jenkins SCM Branch Source support to the plugin, and refactor the way Multibranch jobs are triggered in the process.