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

Support to skip the build wrapper for multiple SCM #54

Closed
lianghu opened this issue Jul 30, 2015 · 40 comments
Closed

Support to skip the build wrapper for multiple SCM #54

lianghu opened this issue Jul 30, 2015 · 40 comments
Assignees

Comments

@lianghu
Copy link

lianghu commented Jul 30, 2015

This plugin is great.

However, I have a project with multiple git repos with its own directory layout.
It seems PhabricatorBuildWrapper.java assumes only one git repository case.

I would like to add one more parameter in my Jenkins job to get the repo's Callsign from Phabricator.
The Callsign is unique for each repo. With that, I should be able to handle the apply differential and build in Jenkin job's build section via Execute Shell commands.
After build finished, I will use post-build script to reset the repo with patch to master branch again.

Thus, I need this plugin has an option to do nothing in the build wrapper part but just pass the values for arguments, and keep current directory as the workspace, direct go to Jenkins job's own build scripts.

Thanks,
Lianghu

@ascandella
Copy link
Contributor

Hello there,

Could you clarify a little bit more what you'd like us to do with these callsigns? You want them passed as an environment variable?

I'm definitely interested in supporting your use case, but I don't really know what it would look like to have multiple repos. Could you post a screenshot maybe of what your job configuration looks like? Do you have a base workspace with multiple subdirectories, each one of those a separate repo?

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

Hi,

Yes. you're right, we have a base workspace with multiple subdirectories, each one of those a separate repo, the top directory is also a repo. Because we need to know the subdirectory path of a repo to apply the specific differential which is identified by the callsign.

So the simple solution is have a mapping between the repos and the directory pathes. I would like to do that in the build shell script directly since the build wrapper can't do that yet.

@ascandella
Copy link
Contributor

Ok, makes sense. So what should the behavior of this plugin be, then?

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

I used the Multiple SCM (https://wiki.jenkins-ci.org/display/JENKINS/Multiple+SCMs+Plugin) to manage the git repos in Jenkins. However, since we don't need the Multiple SCM plugin to trigger building after we have the phabricator plugin to do that. I'll clone the repos in the start of the buiding shell script, and apply the diff to proper directory/repo, then build it (similar to https://gist.github.com/dctrwatson/4669113#file-apply-diff-sh, but here it's for multiple repos).

so the plugin should just pass the values to DIFF_ID PHID, CALLSIGN and keep the current directory as top of workspace then goes to Jenkin's own build process, also the post action should be OK since it targets the Differential number.

@ascandella
Copy link
Contributor

In that case, can you just remove the "Apply Phabricator Differential" step and skip the BuildWrapper? Or are you asking for some different functionality in the build wrapper?

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

If I skip the "Apply Phabricator Differential" step, will the arguments still get the values from phabricator? and the feedback part still works?

@ascandella
Copy link
Contributor

What do you mean "will the arguments still get the values from phabricator?"? Which "arguments"?

It should work, although you won't get a "build started" message.

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

I mean the parameters, like DIFF_ID, PHID, CALLSIGN

@ascandella
Copy link
Contributor

Those are just environment variables, which both the BuildWrapper and Notifier use, but are provided by Harbormaster (presumably, or else some custom cURL hackery like JenkinsDiffEventListener.php) -- those will be present regardless of whether this plugin is installed or not.

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

Thanks, I'll have a try skipping the "Apply Phabricator Differential" step

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

Hi,
There is a no Dxxxx label in the build history as previous after skipping the step.
It's nice to have it back, any idea?

@ascandella
Copy link
Contributor

Ah, good catch. Is that the only thing you're missing? If so, we can check for a missing badge on the Notifier stage and add it.

@ascandella ascandella self-assigned this Jul 31, 2015
@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

so far, it's the only thing. actually, I think you can try to add multiple repos support.
or try to be able to work together with The Multiple SCM (https://wiki.jenkins-ci.org/display/JENKINS/Multiple+SCMs+Plugin)

@ascandella
Copy link
Contributor

What would it mean to support multiple repos? Have a bunch of parameters and arc patch them individually? I'm open to the idea, it's just not clear to me what your workflow is that you're supporting. If you're not comfortable sharing a screenshot of your job config on public github I can send you my email.

@lianghu
Copy link
Author

lianghu commented Jul 31, 2015

For each build there is only one DIFF_ID, one PHID and one CALLSIGN_ID because there is only one Differential. However, the differential need to be built togehter with other repos as a whole.

A differential can be any of the repos, but just build one differential each time (if the phabricator can merge a few diffs in a short period would be great though). I have two screenshot of two of the repos ( we have about 10) for a rough idea.

image
image

@ascandella
Copy link
Contributor

I see, thanks for the clarification. I just left work for he day, I'll see
what I can come up with tomorrow/this weekend.
On Thu, Jul 30, 2015 at 8:45 PM lianghu notifications@github.com wrote:

For each build there is only one DIFF_ID, one PHID and one CALLSIGN_ID
because there is only one Differential. However, the differential need to
be built togehter with other repos as a whole.

A differential can be any of the repos, but just build one differential
each time (if the phabricator can merge a few diffs in a short period would
be great though). I have two screenshot of two of the repos ( we have about
10) for a rough idea.

[image: image]
https://cloud.githubusercontent.com/assets/3190996/9000371/f2e21b50-3778-11e5-9b76-d16b0584ba23.png
[image: image]
https://cloud.githubusercontent.com/assets/3190996/9000384/2fa2f622-3779-11e5-80a8-ba7ed8749ba4.png


Reply to this email directly or view it on GitHub
#54 (comment)
.

@ascandella
Copy link
Contributor

@lianghu in your build script do you just have a big switch statement on the callsign_id to determine which directory/repo to arc patch?

@lianghu
Copy link
Author

lianghu commented Aug 3, 2015

yes, just like that

@ascandella
Copy link
Contributor

Got it. It sounds to me then like the most elegant and flexible solution, to help both you and others, would be to pass an additional parameter, something like DIFF_SUBDIR that we would cd into before running arc patch. That way, you could remove your arc patch custom build script, get the badge decoration (and start notifications) and this wouldn't be tied to any specific plugin implementation.

Does that seem reasonable? If so, I'll go ahead and implement it.

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

I think a option for user to add <DIFF_SUBIR, CALLSIGN> pairs would be great.
With this input from user, the plugin can get the DIFF_SUBDIR info once it knows the CALLSIGN.
We can't get the DIFF_SBUDIR from Phabricator.

@ascandella
Copy link
Contributor

How would we get the $DIFF_SUBDIR from callsign? Based on the dirname of querying phabricator for the CALLSIGN? Why not just pass the SUBDIR directly? That way this is usable for more uses than just yours, e.g. somebody has a submodule that they want to test that's not checked out at the directory specified by the callsign

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

How to pass the SUBDIR from Phabricator?

I think the SUBDIR should be come from the user.
The plugin can get the user input where the CALLSIGN to SUBDIR map is provided.

If it's doable via Phabricator, I'm also fine.

@ascandella
Copy link
Contributor

If you're using Harbormaster, just add it as a new parameter in your build job. If you're using the outdated jenkinsphoo PHP approach, add it into the .arcanist folder there. That way, this plugin doesn't have to know/care about CALLSIGN, it cas just look at $DIFF_SUBDIR and arc patch in that specific directory.

@ascandella
Copy link
Contributor

Harbormaster: https://github.com/uber/phabricator-jenkins-plugin#harbormaster

Just like DIFF_ID is a parameter in the URL there, you could add DIFF_SUBDIR for what you have configured in the checkout of your multi-SCM git plugin.

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

but how does Harbormaster know the DIFF_SUBDIR value?

@ascandella
Copy link
Contributor

Assuming you have a harbormaster rule per-repo, you would configure it in the repo's harbormaster job. Are you using harbormaster?

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

Yes. but I just use 1 harbormaster job for all repos, use the "is any of " settings

image

@ascandella
Copy link
Contributor

I see, thanks for the info. I guess you could pass the repository.uri as a variable and we could take the suffix of that, but it's starting to feel like less clean of a solution. Then again, I'm not seeing a clean conduit call to map Callsign ID to directory name.

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

that's why I prefer to let user provide the map just like the multi-SCM git plugin. the plugin gets the mapping from the user, CALLSIGN from harbomaster and then find the SUBDIR mapped.

@ascandella
Copy link
Contributor

Interesting. I'm open to supporting that, but am not sure how the UI would be presented. Here are the configuration option we get from Jelly (the templating/configuration engine for Jenkins plugins):

https://wiki.jenkins-ci.org/display/JENKINS/Jelly+form+controls

@lianghu
Copy link
Author

lianghu commented Aug 4, 2015

I'm not familar with the Jelly for UI. however, I think UI is good as long as it's clear.

ascandella added a commit that referenced this issue Aug 4, 2015
@ascandella
Copy link
Contributor

I just pushed a branch (map-subdir) at https://github.com/uber/phabricator-jenkins-plugin/tree/map-subdir

Could you pull it down and give it a spin? run mvn package to generate an hpi file that you can install via the "advanced" section of the plugin manager.

If the approach looks good, I will add tests and make a PR.

@ascandella
Copy link
Contributor

There is now an "advanced" button that expands like so:

image

ascandella added a commit that referenced this issue Aug 4, 2015
@ascandella
Copy link
Contributor

Also, if you'd like to test a self-contained instance without updating your "real" jenkins install, you can pull down the repo and run mvn hpi:run to start an isolated jenkins on localhost:8080

@lianghu
Copy link
Author

lianghu commented Aug 5, 2015

Is it possible to run some prebuild script before this plugin take control? I need to clone the repos (I don't want to use the SCM plugin for simple configure).

@lianghu
Copy link
Author

lianghu commented Aug 5, 2015

there are multipe repos, need to reset the current repo to the base commit (i.e., unapply the diff) after the build finished, can the plugin support that?

@ascandella
Copy link
Contributor

I think you have some very reasonable request, but due to work I don't have time to implement them right now. I'll leave this branch up and issue open until I (or you, or somebody else) can get a solution that works for you. PRs welcome.

@lianghu
Copy link
Author

lianghu commented Aug 6, 2015

Currently, I think I can stick with the script method (More flexible).
Could you add the check for a missing badge on the Notifier stage and add it as we talked in the beginning? That's a missing feature.

@ascandella
Copy link
Contributor

Sure, the only reason that I haven't implemented that is that there is no way to share configuration between the two plugins, and the BuildWrapper (the thing that sets up the build) is where you configure your Phabricator URL, so if we add the badge in the Notifier (post-build) step, we don't know how to link to your differential.

@ascandella
Copy link
Contributor

This is now released in 1.7 of the plugin. Note that conduit credentials have moved into the "Credentials" plugin so you'll need to follow the instructions at https://github.com/uber/phabricator-jenkins-plugin#configuration

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

No branches or pull requests

2 participants