Make the recipe roller tryjob accept tryjobs.

scripts/slave/recipes/infra/recipe_roll_tryjob.py

Index: scripts/slave/recipes/infra/recipe_roll_tryjob.py
diff --git a/scripts/slave/recipes/infra/recipe_roll_tryjob.py b/scripts/slave/recipes/infra/recipe_roll_tryjob.py
index f272d542856896f3a8dd88353e8a0c97eb5eebaa..179127793d3047bf3039e9cc71f40058268610ee 100644
--- a/scripts/slave/recipes/infra/recipe_roll_tryjob.py
+++ b/scripts/slave/recipes/infra/recipe_roll_tryjob.py
@@ -148,7 +148,7 @@ def get_url_mapping(api, headers=None):
   """Fetch the mapping of project id to url from luci-config.
 
   Args:
-    auth_token: an OAuth2 access token for querying luci-config.
+    headers: Optional authentication headers to pass to luci-config.
 
   Returns:
     A dictionary mapping project id to its luci-config project spec (among
@@ -184,17 +184,21 @@ RietveldPatch = collections.namedtuple(
     'RietveldPatch', 'project server issue patchset')
 
 
-def parse_patches(api, patches_raw):
+def parse_patches(api, patches_raw, rietveld, issue, patchset, patch_project):

[iannucci, 2016/03/23 19:08:40]

hm.. this seems a bit awkward of an API to me. What if you had a different
function 'extract_patches_from_rietveld', and then fed the result of that into
parse_patches?

And I still maintain that 'patches_raw' should be a real json dictionary instead
of this string thing... what was the reason for not doing that again?

I would also consider having the rietveld syntax look like:

DOWNSTREAM_PATCH=recipes:<project_name>:<url>
DOWNSTREAM_PATCH=recipes:<project_name2>:<url>

Instead of cramming it all into one line. I'd also make sure that rietveld's
url-finder regex can correctly link the url portions as well.

The 'recipes' literal would allow us to use this syntax and roller mechanism for
applying other sorts of patches too (e.g. be able to have a roller which tries
to roll generic upstream changes downstream). Something like
`DOWNSTREAM_PATCH=DEPS:chromium_src:<url>`. The rollers would need to
disambiguate the type of dependency (recipes.cfg, DEPS, submodules, repo, etc.)
in order to know how to update the downstream patch with the roll information
(bumping the pinned version).

Just a thought.

[martiniss, 2016/03/23 20:38:43]

Looks great!

This syntax is only for command line invocations. I figured that people wouldn't
want to do something like patches='[{\"project\": \"build\", ......' on the
command line. It's a bit ugly but it does work. I think eventually it'd be
mostly replaced by just passing in one patch on the command line, and having
this look at the downstream patches and do smart things™ from there.

   """
   gives mapping of project to patch
     expect input of
     project1:https://a.b.c/1342342#ps1,project2:https://d.ce.f/1231231#ps1
   """
-  if not patches_raw:
-    return {}
-
   result = {}
 
+  if rietveld and issue and patchset and patch_project:
+    result[patch_project] = RietveldPatch(
+        patch_project, rietveld, issue, patchset)
+
+  if not patches_raw:
+    return result
+
   for patch_raw in patches_raw.split(','):
     project, url = patch_raw.split(':', 1)
     server, issue_and_patchset = url.rsplit('/', 1)
@@ -302,6 +306,8 @@ def checkout_project(api, proj, proj_config, root_dir, patch=None):
     kwargs['rietveld'] = patch.server
     kwargs['issue'] = patch.issue
     kwargs['patchset'] = patch.patchset
+  else:
+    kwargs['patch'] = False
 
   api.bot_update.ensure_checkout(**kwargs)
   return repo_path
@@ -321,12 +327,26 @@ PROPERTIES = {
                       "url.to.codereview is the address of the code review site"
                       ", 123456 is the issue number, and ps01 is the patchset"
                       "number"),
+  # This recipe can be used as a tryjob by setting the rietveld, issue, and
+  # patchset properties, like a normal tryjob. If those are set, it will use
+  # those, as well as any data sent in the regular properties, as patches to
+  # apply.
+  "rietveld": Property(kind=str, default="",
+                       help="The code review site the tryjob patch isfrom"),

[iannucci, 2016/03/23 19:08:40]

This is very rietveld specific: "The Rietveld instance the issue is from"

[martiniss, 2016/03/23 20:38:43]

Done.

+  "issue": Property(kind=int, default=None,
+                    help="The issue number this patch is from"),

[iannucci, 2016/03/23 19:08:40]

The Rietveld issue number to pull data from.

[martiniss, 2016/03/23 20:38:43]

Done.

+  "patchset": Property(kind=int, default=None,
+                       help="The patchset to apply from this patch"),

[iannucci, 2016/03/23 19:08:40]

The patchset number for the supplied issue.

Though honestly why can't we just have it be one url?

[martiniss, 2016/03/23 20:38:43]

Done, and because CQ sets these properties when they send a tryjob, so we need
to accept them... AFAIK

+  "patch_project": Property(
+      kind=str, default=None,
+      help="The luci-config name of the project this patch belongs to"),

[iannucci, 2016/03/23 19:08:40]

Isn't this retrievable from rietveld directly?

[martiniss, 2016/03/23 20:38:43]

Probably, yeah. It's set by CQ though, so I figure we might as well use it.

 }
 
-def RunSteps(api, patches_raw):
+def RunSteps(api, patches_raw, rietveld, issue, patchset, patch_project):
   headers = {'Authorization': 'Bearer %s' % get_auth_token(api)}
 
-  patches = parse_patches(api, patches_raw)
+  patches = parse_patches(
+      api, patches_raw, rietveld, issue, patchset, patch_project)
 
   root_dir = api.path['slave_build']
 
@@ -341,7 +361,7 @@ def RunSteps(api, patches_raw):
   recipe_configs = {
       p: get_project_config(api, p, headers) for p in all_projects}
 
-  downstream_projects, deps = get_deps_info(all_projects, recipe_configs)
+  deps, downstream_projects = get_deps_info(all_projects, recipe_configs)
 
   projs_to_test, locations = checkout_projects(
       api, all_projects, url_mapping, downstream_projects, root_dir, patches)
@@ -410,3 +430,15 @@ def GenTests(api):
       api.step_data("Get recipe_engine deps", project('recipe_engine'))
   )
 
+  yield (
+      api.test('tryjob') +
+      api.properties(
+        rietveld="https://fake.code.review",
+        issue=12345678,
+        patchset=1,
+        patch_project="build",
+      ) +
+      api.step_data("Get build deps", project('build', ['recipe_engine'])) +
+      api.step_data("Get recipe_engine deps", project('recipe_engine'))
+  )
+