Generalize patch_project to patch root conversion.

IMPORTANT: See bug http://crbug.com/605563#c3 for deployment and emergency roll-back plan.

This patch is first of a series. It provides a cleaner and generic way to apply patches to non-root of first solution.

R=phajdan.jr@chromium.org,machenbach@chromium.org
BUG=605563

Review URL: https://codereview.chromium.org/1917433002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300141 0039d316-1c4b-4281-b951-d872f2087c98

recipe_modules/bot_update/__init__.py

@@ -21,6 +21,7 @@ PROPERTIES = {
   'issue': Property(default=None),
   'patchset': Property(default=None),
   'patch_url': Property(default=None),
+  'patch_project': Property(default=None),
   'repository': Property(default=None),
   'event.patchSet.ref': Property(default=None, param_name="gerrit_ref"),
   'rietveld': Property(default=None),

recipe_modules/bot_update/api.py

@@ -85,6 +85,17 @@ class BotUpdateApi(recipe_api.RecipeApi):
     # Construct our bot_update command.  This basically be inclusive of
     # everything required for bot_update to know:
     root = patch_root
+    if root == 'TODO(TANDRII): REMOVE THIS TRANSITION TO patch_projects':
+      # This special condition is here for initial rollout of this code,
+      # because it's hard to test this change without rolling into build
+      # repository.
+      # After the switch to new code is complete, this special TODOstring will
+      # be removed in favor of "root is None"
+      assert patch_project_roots is None
+      root = self.m.gclient.calculate_patch_root(
+          self.m.properties.get('patch_project'), cfg)
+      # TODO(tandrii): get rid the condition below after transition.
+
     if root is None:
       root = cfg.solutions[0].name
       additional = self.m.rietveld.calculate_issue_root(patch_project_roots)
@@ -128,6 +139,12 @@ class BotUpdateApi(recipe_api.RecipeApi):
     else:
       email_file = key_file = None
 
+    # Allow patch_project's revision if necessary.
+    # This is important for projects which are checked out as DEPS of the
+    # gclient solution.
+    self.m.gclient.set_patch_project_revision(
+        self.m.properties.get('patch_project'), cfg)
+
     rev_map = cfg.got_revision_mapping.as_jsonish()
 
     flags = [

recipe_modules/bot_update/example.expected/gerrit_no_reset.json

@@ -42,4 +42,4 @@
     "recipe_result": null,
     "status_code": 0
   }
-]
+]

recipe_modules/bot_update/example.expected/tryjob_crbug605563.json

@@ -0,0 +1,59 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py",
+      "--master",
+      "tryserver.chromium.linux",
+      "--builder",
+      "linux_rel",
+      "--slave",
+      "totallyaslave-c4",
+      "--spec",
+      "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'svn://svn.chromium.org/chrome/trunk/src'}]",
+      "--root",
+      "src",
+      "--revision_mapping_file",
+      "{\"src\": \"got_cr_revision\"}",
+      "--git-cache-dir",
+      "[GIT_CACHE]",
+      "--patch_url",
+      "http://src.chromium.org/foo/bar",
+      "--output_json",
+      "/path/to/tmp/json",
+      "--revision",
+      "src@HEAD"
+    ],
+    "cwd": "[SLAVE_BUILD]",
+    "env": {
+      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]"
+    },
+    "name": "bot_update",
+    "~followup_annotations": [
+      "@@@STEP_TEXT@Some step text@@@",
+      "@@@STEP_LOG_LINE@json.output@{@@@",
+      "@@@STEP_LOG_LINE@json.output@  \"did_run\": true, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"fixed_revisions\": {@@@",
+      "@@@STEP_LOG_LINE@json.output@    \"src\": \"HEAD\"@@@",
+      "@@@STEP_LOG_LINE@json.output@  }, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_failure\": false, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_root\": \"src\", @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"properties\": {@@@",
+      "@@@STEP_LOG_LINE@json.output@    \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@",
+      "@@@STEP_LOG_LINE@json.output@    \"got_cr_revision_cp\": \"refs/heads/master@{#170242}\"@@@",
+      "@@@STEP_LOG_LINE@json.output@  }, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"root\": \"src\", @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"step_text\": \"Some step text\"@@@",
+      "@@@STEP_LOG_LINE@json.output@}@@@",
+      "@@@STEP_LOG_END@json.output@@@",
+      "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@",
+      "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@"
+    ]
+  },
+  {
+    "name": "$result",
+    "recipe_result": null,
+    "status_code": 0
+  }
+]

recipe_modules/bot_update/example.expected/tryjob_gerrit_angle.json

@@ -0,0 +1,50 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py",
+      "--master",
+      "chromium.testing.master",
+      "--builder",
+      "TestBuilder",
+      "--slave",
+      "TestSlavename",
+      "--spec",
+      "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'svn://svn.chromium.org/chrome/trunk/src'}]",
+      "--root",
+      "src/third_party/angle",
+      "--revision_mapping_file",
+      "{\"src\": \"got_cr_revision\"}",
+      "--git-cache-dir",
+      "[GIT_CACHE]",
+      "--gerrit_repo",
+      "https://chromium.googlesource.com/angle/angle",
+      "--gerrit_ref",
+      "refs/changes/11/338811/3",
+      "--output_json",
+      "/path/to/tmp/json",
+      "--revision",
+      "src@HEAD",
+      "--revision",
+      "src/third_party/angle@HEAD"
+    ],
+    "cwd": "[SLAVE_BUILD]",
+    "env": {
+      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]"
+    },
+    "name": "bot_update",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@json.output@{@@@",
+      "@@@STEP_LOG_LINE@json.output@  \"did_run\": false, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_failure\": false@@@",
+      "@@@STEP_LOG_LINE@json.output@}@@@",
+      "@@@STEP_LOG_END@json.output@@@"
+    ]
+  },
+  {
+    "name": "$result",
+    "recipe_result": null,
+    "status_code": 0
+  }
+]

recipe_modules/bot_update/example.expected/tryjob_v8_head_by_default.json

@@ -0,0 +1,52 @@
+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py",
+      "--master",
+      "chromium.testing.master",
+      "--builder",
+      "TestBuilder",
+      "--slave",
+      "TestSlavename",
+      "--spec",
+      "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'svn://svn.chromium.org/chrome/trunk/src'}]",
+      "--root",
+      "src/v8",
+      "--revision_mapping_file",
+      "{\"src\": \"got_cr_revision\"}",
+      "--git-cache-dir",
+      "[GIT_CACHE]",
+      "--issue",
+      "12853011",
+      "--patchset",
+      "1",
+      "--rietveld_server",
+      "https://codereview.chromium.org",
+      "--output_json",
+      "/path/to/tmp/json",
+      "--revision",
+      "src@HEAD",
+      "--revision",
+      "src/v8@HEAD"
+    ],
+    "cwd": "[SLAVE_BUILD]",
+    "env": {
+      "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]"
+    },
+    "name": "bot_update",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@json.output@{@@@",
+      "@@@STEP_LOG_LINE@json.output@  \"did_run\": false, @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_failure\": false@@@",
+      "@@@STEP_LOG_LINE@json.output@}@@@",
+      "@@@STEP_LOG_END@json.output@@@"
+    ]
+  },
+  {
+    "name": "$result",
+    "recipe_result": null,
+    "status_code": 0
+  }
+]

recipe_modules/bot_update/example.py

@@ -18,8 +18,11 @@ def RunSteps(api):
   soln.url = 'svn://svn.chromium.org/chrome/trunk/src'
   soln.revision = api.properties.get('revision')
   api.gclient.c = src_cfg
-  api.gclient.c.revisions = api.properties.get('revisions', {})
+  api.gclient.c.revisions.update(api.properties.get('revisions', {}))
   api.gclient.c.got_revision_mapping['src'] = 'got_cr_revision'
+  api.gclient.c.patch_projects['v8'] = ('src/v8', 'HEAD')
+  api.gclient.c.patch_projects['angle/angle'] = ('src/third_party/angle',
+                                                 'HEAD')
   patch = api.properties.get('patch', True)
   clobber = True if api.properties.get('clobber') else False
   force = True if api.properties.get('force') else False
@@ -31,6 +34,9 @@ def RunSteps(api):
   root_solution_revision = api.properties.get('root_solution_revision')
   suffix = api.properties.get('suffix')
   gerrit_no_reset = True if api.properties.get('gerrit_no_reset') else False
+  # TODO(tandrii): remove this after transition. http://crbug.com/605563.
+  crbug605563 = ('TODO(TANDRII): REMOVE THIS TRANSITION TO patch_projects'
+                 if api.properties.get('crbug605563') else None)
   api.bot_update.ensure_checkout(force=force,
                                  no_shallow=no_shallow,
                                  patch=patch,
@@ -40,7 +46,8 @@ def RunSteps(api):
                                  clobber=clobber,
                                  root_solution_revision=root_solution_revision,
                                  suffix=suffix,
-                                 gerrit_no_reset=gerrit_no_reset)
+                                 gerrit_no_reset=gerrit_no_reset,
+                                 patch_root=crbug605563)
 
 
 def GenTests(api):
@@ -72,6 +79,15 @@ def GenTests(api):
       patchset=654321,
       patch_url='http://src.chromium.org/foo/bar'
   )
+  yield api.test('tryjob_crbug605563') + api.properties(
+      mastername='tryserver.chromium.linux',
+      buildername='linux_rel',
+      slavename='totallyaslave-c4',
+      issue=12345,
+      patchset=654321,
+      patch_url='http://src.chromium.org/foo/bar',
+      crbug605563=True,
+  )
   yield api.test('trychange') + api.properties(
       mastername='tryserver.chromium.linux',
       buildername='linux_rel',
@@ -161,3 +177,11 @@ def GenTests(api):
       patch_project='v8',
       revisions={'src/v8': 'abc'}
   )
+  yield api.test('tryjob_v8_head_by_default') + api.properties.tryserver(
+      patch_project='v8',
+      crbug605563=True,
+  )
+  yield api.test('tryjob_gerrit_angle') + api.properties.tryserver_gerrit(
+      full_project_name='angle/angle',
+      crbug605563=True,
+  )

recipe_modules/gclient/api.py

@@ -148,6 +148,7 @@ class GclientApi(recipe_api.RecipeApi):
 
   def sync(self, cfg, with_branch_heads=False, **kwargs):
     revisions = []
+    self.set_patch_project_revision(self.m.properties.get('patch_project'), cfg)
     for i, s in enumerate(cfg.solutions):
       if s.safesync_url:  # prefer safesync_url in gclient mode
         continue
@@ -336,3 +337,42 @@ class GclientApi(recipe_api.RecipeApi):
       args=[self.m.path['slave_build']],
       infra_step=True,
     )
+
+  def calculate_patch_root(self, patch_project, gclient_config=None):
+    """Returns path where a patch should be applied to based patch_project.
+
+    Maps "patch_project" to a path of directories relative to checkout's root,
+    which describe where to place the patch.
+
+    For now, considers only first solution (c.solutions[0]), but in theory can
+    be extended to all of them.
+
+    See patch_projects solution config property.
+
+    Returns:
+      Relative path, including solution's root.
+      If patch_project is not given or not recognized, it'll be just first
+      solution root.
+    """
+    cfg = gclient_config or self.c
+    root, _ = cfg.patch_projects.get(patch_project, ('', ''))
+    if root:
+      # Note, that c.patch_projects contains patch roots as
+      # slash(/)-separated path, which are roots of the respective project repos
+      # and include actual solution name in them.
+      return self.m.path.join(*root.split('/'))
+    # Default case - assume patch is for first solution, as this is what most
+    # projects rely on.
+    return cfg.solutions[0].name
+
+  def set_patch_project_revision(self, patch_project, gclient_config=None):
+    """Updates config revision corresponding to patch_project.
+
+    Useful for bot_update only, as this is the only consumer of gclient's config
+    revision map. This doesn't overwrite the revision if it was already set.
+    """
+    assert patch_project is None or isinstance(patch_project, basestring)
+    cfg = gclient_config or self.c
+    path, revision = cfg.patch_projects.get(patch_project, (None, None))
+    if path and revision and path not in cfg.revisions:
+      cfg.revisions[path] = revision

recipe_modules/gclient/config.py

@@ -63,6 +63,20 @@ def BaseConfig(USE_MIRROR=True, GIT_MODE=False, CACHE_DIR=None,
     parent_got_revision_mapping = Dict(hidden=True),
     delete_unversioned_trees = Single(bool, empty_val=True, required=False),
 
+    # Maps patch_project to (solution/path, revision).
+    #  - solution/path is then used to apply patches as patch root in
+    #    bot_update.
+    #  - if revision is given, it's passed verbatim to bot_update for
+    #    corresponding dependency.
+    # This is essentially a whitelist of which projects inside a solution
+    # can be patched automatically by bot_update based on PATCH_PROJECT
+    # property.
+    # For example, bare chromium solution has this entry in patch_projects
+    #     'angle/angle': ('src/third_party/angle', 'HEAD')
+    # then a patch to Angle project can be applied to a chromium src's
+    # checkout after first updating Angle's repo to its master's HEAD.
+    patch_projects = Dict(value_type=tuple, hidden=True),
+
     # Check out refs/branch-heads.
     # TODO (machenbach): Only implemented for bot_update atm.
     with_branch_heads = Single(
@@ -73,6 +87,8 @@ def BaseConfig(USE_MIRROR=True, GIT_MODE=False, CACHE_DIR=None,
 
     GIT_MODE = Static(bool(GIT_MODE)),
     USE_MIRROR = Static(bool(USE_MIRROR)),
+    # TODO(tandrii): remove PATCH_PROJECT field.
+    # DON'T USE THIS. WILL BE REMOVED.
     PATCH_PROJECT = Static(str(PATCH_PROJECT), hidden=True),
     BUILDSPEC_VERSION= Static(BUILDSPEC_VERSION, hidden=True),
   )
@@ -146,18 +162,10 @@ def chromium_bare(c):
   p['parent_got_v8_revision'] = 'v8_revision'
   p['parent_got_webrtc_revision'] = 'webrtc_revision'
 
-  # Patch project revisions are applied whenever patch_project is set. E.g. if
+  p = c.patch_projects
-  # a v8 stand-alone patch is sent to a chromium trybot, patch_project is v8
+  p['v8'] = ('src/v8', 'HEAD')
-  # and can be used to sync v8 to HEAD instead of the pinned chromium
+  p['angle/angle'] = ('src/third_party/angle', None)
-  # version.
+  p['blink'] = ('src/third_party/WebKit', None)
-  patch_project_revisions = {
-    'v8': ('src/v8', 'HEAD'),
-  }
-
-  patch_revision = patch_project_revisions.get(c.PATCH_PROJECT)
-  # TODO(phajdan.jr): Move to proper repo and add coverage.
-  if patch_revision:  # pragma: no cover
-    c.revisions[patch_revision[0]] = patch_revision[1]
 
 @config_ctx(includes=['chromium_bare'])
 def chromium_empty(c):
@@ -538,6 +546,10 @@ def infra(c):
   soln.url = 'https://chromium.googlesource.com/infra/infra.git'
   c.got_revision_mapping['infra'] = 'got_revision'
 
+  p = c.patch_projects
+  p['luci-py'] = ('infra/luci', 'HEAD')
+  p['recipes-py'] = ('infra/recipes-py', 'HEAD')
+
 @config_ctx(config_vars={'GIT_MODE': True})
 def infra_internal(c):  # pragma: no cover
   soln = c.solutions.add()
@@ -572,6 +584,7 @@ def luci_py(c):
   # luci-py is checked out as part of infra just to have appengine
   # pre-installed, as that's what luci-py PRESUBMIT relies on.
   c.revisions['infra'] = 'origin/master'
+  # TODO(tandrii): make use of c.patch_projects.
   c.revisions['infra/luci'] = (
       gclient_api.RevisionFallbackChain('origin/master'))
   m = c.got_revision_mapping
@@ -581,6 +594,7 @@ def luci_py(c):
 @config_ctx(includes=['infra'])
 def recipes_py(c):
   c.revisions['infra'] = 'origin/master'
+  # TODO(tandrii): make use of c.patch_projects.
   c.revisions['infra/recipes-py'] = (
       gclient_api.RevisionFallbackChain('origin/master'))
   m = c.got_revision_mapping
@@ -634,6 +648,7 @@ def angle_top_of_tree(c):  # pragma: no cover
 
   Sets up ToT instead of the DEPS-pinned revision for ANGLE.
   """
+  # TODO(tandrii): I think patch_projects in bare_chromium fixed this.
   c.solutions[0].revision = 'HEAD'
   c.revisions['src/third_party/angle'] = 'HEAD'
 

recipe_modules/rietveld/api.py

@@ -11,6 +11,11 @@ class RietveldApi(recipe_api.RecipeApi):
   def calculate_issue_root(self, extra_patch_project_roots=None):
     """Returns path where a patch should be applied to based on "patch_project".
 
+    YOU SHOULD NOT USE THIS METHOD. Put this into gclient's config as
+    patch_projects instead, and with luck you won't need to use
+    calculate_patch_root from gclient api.
+    TODO(tandrii): remove this method completely. See http://crbug.com/605563.
+
     Maps Rietveld's "patch_project" to a path of directories relative to
     api.gclient.c.solutions[0].name which describe where to place the patch.