bot_update: Rewrite to use properties, and add override options for patches.

recipe_modules/bot_update/api.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 
 """Recipe module to ensure a checkout is consistant on a bot."""
 
 from recipe_engine import recipe_api
 
 
@@ skipping 30 matching lines @@
       ret += ']'
     elif isinstance(spec, basestring):
       ret = repr(str(spec))
     else:
       ret = repr(spec)
   return ret
 
 
 class BotUpdateApi(recipe_api.RecipeApi):
 
-  def __init__(self, *args, **kwargs):
-      self._properties = {}
-      super(BotUpdateApi, self).__init__(*args, **kwargs)
+  def __init__(self, mastername, buildername, slavename, issue, patchset,
+               patch_url, repository, gerrit_ref, rietveld, revision,
+               parent_got_revision, deps_revision_overrides, fail_patch,
+               *args, **kwargs):
+    self._mastername = mastername
+    self._buildername = buildername
+    self._slavename = slavename
+    self._issue = issue
+    self._patchset = patchset
+    self._patch_url = patch_url
+    self._repository = repository
+    self._gerrit_ref = gerrit_ref
+    self._rietveld = rietveld
+    self._revision = revision
+    self._parent_got_revision = parent_got_revision
+    self._deps_revision_overrides = deps_revision_overrides
+    self._fail_patch = fail_patch
+
+    self._properties = {}

[iannucci, 2016/03/01 22:36:22]

can we rename this? like _last_returned_properties or something?

[martiniss, 2016/03/02 01:19:55]

Done. Might have to rename callers though, I'll see.

+    super(BotUpdateApi, self).__init__(*args, **kwargs)
 
   def __call__(self, name, cmd, **kwargs):
     """Wrapper for easy calling of bot_update."""
     assert isinstance(cmd, (list, tuple))
     bot_update_path = self.resource('bot_update.py')
     kwargs.setdefault('infra_step', True)
     kwargs.setdefault('env', {})
     kwargs['env'].setdefault('PATH', '%(PATH)s')
     kwargs['env']['PATH'] = self.m.path.pathsep.join([
         kwargs['env']['PATH'], str(self._module.PACKAGE_DIRECTORY)])
     return self.m.python(name, bot_update_path, cmd, **kwargs)
 
   @property
   def properties(self):
       return self._properties
 
   def ensure_checkout(self, gclient_config=None, suffix=None,
                       patch=True, update_presentation=True,
                       force=False, patch_root=None, no_shallow=False,
                       with_branch_heads=False, refs=None,
                       patch_project_roots=None, patch_oauth2=False,
                       output_manifest=True, clobber=False,
-                      root_solution_revision=None, **kwargs):
+                      root_solution_revision=None, rietveld=None, issue=None,
+                      patchset=None, **kwargs):
+    """
+    Args:
+      gclient_config: The gclient configuration to use when running bot_update.
+        If omitted, the current gclient configuration is used.
+      rietveld: The rietveld server to use. If omitted, will infer from
+        properties.

[iannucci, 2016/03/01 22:36:22]

mention which property for these. document the format of these properties up in
PROPERTIES if possible.

[martiniss, 2016/03/02 01:19:55]

I mentioned which property these refer to. What do you mean document the format?

+      issue: The rietveld issue number to use. If omitted, will infer from
+        properties.
+      patchset: The rietveld issue patchset to use. If omitted, will infer from
+        properties.
+    """
     refs = refs or []
     # We can re-use the gclient spec from the gclient module, since all the
     # data bot_update needs is already configured into the gclient spec.
     cfg = gclient_config or self.m.gclient.c
     spec_string = jsonish_to_python(cfg.as_jsonish(), True)
 
     # Used by bot_update to determine if we want to run or not.
-    master = self.m.properties['mastername']
-    builder = self.m.properties['buildername']
-    slave = self.m.properties['slavename']
+    master = self._mastername
+    builder = self._buildername
+    slave = self._slavename
 
     # Construct our bot_update command.  This basically be inclusive of
     # everything required for bot_update to know:
     root = patch_root
     if root is None:
       root = cfg.solutions[0].name
       additional = self.m.rietveld.calculate_issue_root(patch_project_roots)
       if additional:
         root = self.m.path.join(root, additional)
 
     if patch:
-      issue = self.m.properties.get('issue')
-      patchset = self.m.properties.get('patchset')
-      patch_url = self.m.properties.get('patch_url')
-      gerrit_repo = self.m.properties.get('repository')
-      gerrit_ref = self.m.properties.get('event.patchSet.ref')
+      issue = issue or self._issue
+      patchset = patchset or self._patchset
+      patch_url = self._patch_url
+      gerrit_repo = self._repository
+      gerrit_ref = self._gerrit_ref
     else:
       # The trybot recipe sometimes wants to de-apply the patch. In which case
       # we pretend the issue/patchset/patch_url never existed.
       issue = patchset = patch_url = email_file = key_file = None
       gerrit_repo = gerrit_ref = None
 
     # Issue and patchset must come together.
     if issue:
       assert patchset
     if patchset:
@@ skipping 31 matching lines @@
         # 2. What do we want to check out (spec/root/rev/rev_map).
         ['--spec', spec_string],
         ['--root', root],
         ['--revision_mapping_file', self.m.json.input(rev_map)],
         ['--git-cache-dir', self.m.path['git_cache']],
 
         # 3. How to find the patch, if any (issue/patchset/patch_url).
         ['--issue', issue],
         ['--patchset', patchset],
         ['--patch_url', patch_url],
-        ['--rietveld_server', self.m.properties.get('rietveld')],
+        ['--rietveld_server', rietveld or self._rietveld],
         ['--gerrit_repo', gerrit_repo],
         ['--gerrit_ref', gerrit_ref],
         ['--apply_issue_email_file', email_file],
         ['--apply_issue_key_file', key_file],
 
         # 4. Hookups to JSON output back into recipes.
         ['--output_json', self.m.json.output()],]
 
 
     # Collect all fixed revisions to simulate them in the json output.
     # Fixed revision are the explicit input revisions of bot_update.py, i.e.
     # every command line parameter "--revision name@value".
     fixed_revisions = {}
 
     revisions = {}
     for solution in cfg.solutions:
       if solution.revision:
         revisions[solution.name] = solution.revision
       elif solution == cfg.solutions[0]:
         revisions[solution.name] = (
-            self.m.properties.get('parent_got_revision') or
-            self.m.properties.get('revision') or
+            self._parent_got_revision or
+            self._revision or
             'HEAD')
     if self.m.gclient.c and self.m.gclient.c.revisions:
       revisions.update(self.m.gclient.c.revisions)
     if cfg.solutions and root_solution_revision:
       revisions[cfg.solutions[0].name] = root_solution_revision
     # Allow for overrides required to bisect into rolls.
-    revisions.update(self.m.properties.get('deps_revision_overrides', {}))
+    revisions.update(self._deps_revision_overrides)
     for name, revision in sorted(revisions.items()):
       fixed_revision = self.m.gclient.resolve_revision(revision)
       if fixed_revision:
         fixed_revisions[name] = fixed_revision
         flags.append(['--revision', '%s@%s' % (name, fixed_revision)])
 
     # Add extra fetch refspecs.
     for ref in refs:
       flags.append(['--refs', ref])
 
     # Filter out flags that are None.
     cmd = [item for flag_set in flags
            for item in flag_set if flag_set[1] is not None]
 
     if clobber:
       cmd.append('--clobber')
     if force:
       cmd.append('--force')
     if no_shallow:
       cmd.append('--no_shallow')
     if output_manifest:
       cmd.append('--output_manifest')
     if with_branch_heads or cfg.with_branch_heads:
       cmd.append('--with_branch_heads')
 
     # Inject Json output for testing.
-    git_mode = self.m.properties.get('mastername') not in SVN_MASTERS
+    git_mode = self._mastername not in SVN_MASTERS
     first_sln = cfg.solutions[0].name
     step_test_data = lambda: self.test_api.output_json(
         master, builder, slave, root, first_sln, rev_map, git_mode, force,
-        self.m.properties.get('fail_patch', False),
+        self._fail_patch,
         output_manifest=output_manifest, fixed_revisions=fixed_revisions)
 
     # Add suffixes to the step name, if specified.
     name = 'bot_update'
     if not patch:
       name += ' (without patch)'
     if suffix:
       name += ' - %s' % suffix
 
     # Ah hah! Now that everything is in place, lets run bot_update!
@@ skipping 47 matching lines @@
 
       # bot_update actually just sets root to be the folder name of the
       # first solution.
       if step_result.json.output['did_run']:
         co_root = step_result.json.output['root']
         cwd = kwargs.get('cwd', self.m.path['slave_build'])
         if 'checkout' not in self.m.path:
           self.m.path['checkout'] = cwd.join(*co_root.split(self.m.path.sep))
 
     return step_result