Revert "Revert "[tryserver] Remove unused methods.""

This reverts commit cc27ecb0.

Reason for revert: Published this method downstream to internal repo.

Original change's description:
> Revert "[tryserver] Remove unused methods."
> 
> This reverts commit 133ac1ab.
> 
> Reason for revert: Turns out these are used by internal recipes.
> 
> Original change's description:
> > [tryserver] Remove unused methods.
> > 
> > R=​agable@chromium.org, dnj@chromium.org, hinoka@chromium.org
> > 
> > Bug:
> > Change-Id: I82a11f31c8c1c4c4a2b461090e5aee637f8821c2
> > Reviewed-on: https://chromium-review.googlesource.com/569019
> > Reviewed-by: Nodir Turakulov <nodir@chromium.org>
> > Reviewed-by: Aaron Gable <agable@chromium.org>
> > Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
> 
> TBR=iannucci@chromium.org,hinoka@chromium.org,agable@chromium.org,dnj@chromium.org,nodir@chromium.org
> 
> Change-Id: Ib1d4192520a36f649f1f9b31e2928027667311d4
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/570988
> Reviewed-by: Daniel Jacques <dnj@chromium.org>
> Commit-Queue: Daniel Jacques <dnj@chromium.org>

TBR=iannucci@chromium.org,hinoka@chromium.org,agable@chromium.org,dnj@chromium.org,nodir@chromium.org

Change-Id: Id7ac3555d40162e4204ceac5e96c2e3864c67aba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/570781Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Daniel Jacques <dnj@chromium.org>

recipes/README.recipes.md

@@ -658,29 +658,21 @@ Returns:
 
 [DEPS](/recipes/recipe_modules/tryserver/__init__.py#5): [gerrit](#recipe_modules-gerrit), [git](#recipe_modules-git), [git\_cl](#recipe_modules-git_cl), [rietveld](#recipe_modules-rietveld), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/raw\_io][recipe_engine/recipe_modules/raw_io], [recipe\_engine/step][recipe_engine/recipe_modules/step]
 
-#### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#16)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
+#### **class [TryserverApi](/recipes/recipe_modules/tryserver/api.py#12)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
 
-— **def [\_\_init\_\_](/recipes/recipe_modules/tryserver/api.py#17)(self, \*args, \*\*kwargs):**
+— **def [\_\_init\_\_](/recipes/recipe_modules/tryserver/api.py#13)(self, \*args, \*\*kwargs):**
 
-— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#224)(self, reason):**
+— **def [add\_failure\_reason](/recipes/recipe_modules/tryserver/api.py#146)(self, reason):**
 
 Records a more detailed reason why build is failing.
 
 The reason can be any JSON-serializable object.
 
-— **def [apply\_from\_git](/recipes/recipe_modules/tryserver/api.py#67)(self, cwd):**
-
-Downloads patch from given git repo and ref and applies it
-
-&emsp; **@property**<br>&mdash; **def [can\_apply\_issue](/recipes/recipe_modules/tryserver/api.py#27)(self):**
+&emsp; **@property**<br>&mdash; **def [can\_apply\_issue](/recipes/recipe_modules/tryserver/api.py#23)(self):**
 
 Returns true iff the properties exist to apply_issue from rietveld.
 
-&mdash; **def [determine\_patch\_storage](/recipes/recipe_modules/tryserver/api.py#94)(self):**
-
-Determines patch_storage automatically based on properties.
-
-&mdash; **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#124)(self, patch_root=None, \*\*kwargs):**
+&mdash; **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#46)(self, patch_root=None, \*\*kwargs):**
 
 Returns list of paths to files affected by the patch.
 
@@ -694,44 +686,34 @@ TODO(tandrii): remove this doc.
 Unless you use patch_root=None, in which case old behavior is used
 which returns paths relative to checkout aka solution[0].name.
 
-&mdash; **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#287)(self, tag, patch_text=None):**
+&mdash; **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#209)(self, tag, patch_text=None):**
 
 Gets a specific tag from a CL description
 
-&mdash; **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#260)(self, patch_text=None):**
+&mdash; **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#182)(self, patch_text=None):**
 
 Retrieves footers from the patch description.
 
 footers are machine readable tags embedded in commit messages. See
 git-footers documentation for more information.
 
-&emsp; **@property**<br>&mdash; **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#34)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_gerrit\_issue](/recipes/recipe_modules/tryserver/api.py#30)(self):**
 
 Returns true iff the properties exist to match a Gerrit issue.
 
-&emsp; **@property**<br>&mdash; **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#44)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_patch\_in\_git](/recipes/recipe_modules/tryserver/api.py#40)(self):**
 
-&emsp; **@property**<br>&mdash; **def [is\_tryserver](/recipes/recipe_modules/tryserver/api.py#21)(self):**
+&emsp; **@property**<br>&mdash; **def [is\_tryserver](/recipes/recipe_modules/tryserver/api.py#17)(self):**
 
 Returns true iff we can apply_issue or patch.
 
-&mdash; **def [maybe\_apply\_issue](/recipes/recipe_modules/tryserver/api.py#103)(self, cwd=None, authentication=None):**
-
-If we're a trybot, apply a codereview issue.
-
-Args:
-  cwd: If specified, apply the patch from the specified directory.
-  authentication: authentication scheme whenever apply_issue.py is called.
-    This is only used if the patch comes from Rietveld. Possible values:
-    None, 'oauth2' (see also api.rietveld.apply_issue.)
-
-&mdash; **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#291)(self, footer):**
+&mdash; **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#213)(self, footer):**
 
-&mdash; **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#203)(self):**
+&mdash; **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#125)(self):**
 
 Mark the tryjob result as a compile failure.
 
-&emsp; **@contextlib.contextmanager**<br>&mdash; **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#233)(self):**
+&emsp; **@contextlib.contextmanager**<br>&mdash; **def [set\_failure\_hash](/recipes/recipe_modules/tryserver/api.py#155)(self):**
 
 Context manager that sets a failure_hash build property on StepFailure.
 
@@ -740,7 +722,7 @@ for the same reason. For example, if a patch is bad (breaks something),
 we'd expect it to always break in the same way. Different failures
 for the same patch are usually a sign of flakiness.
 
-&mdash; **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#215)(self):**
+&mdash; **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#137)(self):**
 
 Mark the tryjob result as having invalid test results.
 
@@ -748,18 +730,18 @@ This means we run some tests, but the results were not valid
 (e.g. no list of specific test cases that failed, or too many
 tests failing, etc).
 
-&mdash; **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#199)(self):**
+&mdash; **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#121)(self):**
 
 Mark the tryjob result as failure to apply the patch.
 
-&mdash; **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#181)(self, subproject_tag):**
+&mdash; **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#103)(self, subproject_tag):**
 
 Adds a subproject tag to the build.
 
 This can be used to distinguish between builds that execute different steps
 depending on what was patched, e.g. blink vs. pure chromium patches.
 
-&mdash; **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#207)(self):**
+&mdash; **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#129)(self):**
 
 Mark the tryjob result as a test failure.
 

recipes/recipe_modules/tryserver/api.py

@@ -9,10 +9,6 @@ import hashlib
 from recipe_engine import recipe_api
 
 
-PATCH_STORAGE_RIETVELD = 'rietveld'
-PATCH_STORAGE_GIT = 'git'
-
-
 class TryserverApi(recipe_api.RecipeApi):
   def __init__(self, *args, **kwargs):
     super(TryserverApi, self).__init__(*args, **kwargs)
@@ -43,84 +39,10 @@ class TryserverApi(recipe_api.RecipeApi):
 
   @property
   def is_patch_in_git(self):
-    return (self.m.properties.get('patch_storage') == PATCH_STORAGE_GIT and
+    return (self.m.properties.get('patch_storage') == 'git' and
             self.m.properties.get('patch_repo_url') and
             self.m.properties.get('patch_ref'))
 
-  def _apply_patch_step(self, patch_file=None, patch_content=None, root=None):
-    assert not (patch_file and patch_content), (
-        'Please only specify either patch_file or patch_content, not both!')
-    patch_cmd = [
-        'patch',
-        '--dir', root or self.m.path['checkout'],
-        '--force',
-        '--forward',
-        '--remove-empty-files',
-        '--strip', '0',
-    ]
-    if patch_file:
-      patch_cmd.extend(['--input', patch_file])
-
-    self.m.step('apply patch', patch_cmd,
-                      stdin=patch_content)
-
-  def apply_from_git(self, cwd):
-    """Downloads patch from given git repo and ref and applies it"""
-    # TODO(nodir): accept these properties as parameters
-    patch_repo_url = self.m.properties['patch_repo_url']
-    patch_ref = self.m.properties['patch_ref']
-
-    patch_dir = self.m.path.mkdtemp('patch')
-    try:
-      build_path = self.m.path['build']
-    except KeyError:
-      raise self.m.step.StepFailure(
-          'path["build"] is not defined. '
-          'Possibly this is a LUCI build. '
-          'tryserver.apply_from_git is not supported in LUCI builds.')
-
-    git_setup_py = build_path.join('scripts', 'slave', 'git_setup.py')
-    git_setup_args = ['--path', patch_dir, '--url', patch_repo_url]
-    patch_path = patch_dir.join('patch.diff')
-
-    self.m.python('patch git setup', git_setup_py, git_setup_args)
-    with self.m.context(cwd=patch_dir):
-      self.m.git('fetch', 'origin', patch_ref, name='patch fetch')
-      self.m.git('clean', '-f', '-d', '-x', name='patch clean')
-      self.m.git('checkout', '-f', 'FETCH_HEAD', name='patch git checkout')
-    self._apply_patch_step(patch_file=patch_path, root=cwd)
-    self.m.step('remove patch', ['rm', '-rf', patch_dir])
-
-  def determine_patch_storage(self):
-    """Determines patch_storage automatically based on properties."""
-    storage = self.m.properties.get('patch_storage')
-    if storage:
-      return storage
-
-    if self.can_apply_issue:
-      return PATCH_STORAGE_RIETVELD
-
-  def maybe_apply_issue(self, cwd=None, authentication=None):
-    """If we're a trybot, apply a codereview issue.
-
-    Args:
-      cwd: If specified, apply the patch from the specified directory.
-      authentication: authentication scheme whenever apply_issue.py is called.
-        This is only used if the patch comes from Rietveld. Possible values:
-        None, 'oauth2' (see also api.rietveld.apply_issue.)
-    """
-    storage = self.determine_patch_storage()
-
-    if storage == PATCH_STORAGE_RIETVELD:
-      return self.m.rietveld.apply_issue(
-          self.m.rietveld.calculate_issue_root(),
-          authentication=authentication)
-    elif storage == PATCH_STORAGE_GIT:
-      return self.apply_from_git(cwd)
-    else:
-      # Since this method is "maybe", we don't raise an Exception.
-      pass
-
   def get_files_affected_by_patch(self, patch_root=None, **kwargs):
     """Returns list of paths to files affected by the patch.
 

recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch.json

 [
-  {
-    "cmd": [
-      "python",
-      "-u",
-      "[BUILD]/scripts/slave/git_setup.py",
-      "--path",
-      "[TMP_BASE]/patch_tmp_1",
-      "--url",
-      "http://patch.url/"
-    ],
-    "name": "patch git setup"
-  },
-  {
-    "cmd": [
-      "git",
-      "fetch",
-      "origin",
-      "johndoe#123.diff"
-    ],
-    "cwd": "[TMP_BASE]/patch_tmp_1",
-    "infra_step": true,
-    "name": "patch fetch"
-  },
-  {
-    "cmd": [
-      "git",
-      "clean",
-      "-f",
-      "-d",
-      "-x"
-    ],
-    "cwd": "[TMP_BASE]/patch_tmp_1",
-    "infra_step": true,
-    "name": "patch clean"
-  },
-  {
-    "cmd": [
-      "git",
-      "checkout",
-      "-f",
-      "FETCH_HEAD"
-    ],
-    "cwd": "[TMP_BASE]/patch_tmp_1",
-    "infra_step": true,
-    "name": "patch git checkout"
-  },
-  {
-    "cmd": [
-      "patch",
-      "--dir",
-      "[START_DIR]",
-      "--force",
-      "--forward",
-      "--remove-empty-files",
-      "--strip",
-      "0",
-      "--input",
-      "[TMP_BASE]/patch_tmp_1/patch.diff"
-    ],
-    "name": "apply patch"
-  },
-  {
-    "cmd": [
-      "rm",
-      "-rf",
-      "[TMP_BASE]/patch_tmp_1"
-    ],
-    "name": "remove patch"
-  },
   {
     "cmd": [
       "git",

recipes/recipe_modules/tryserver/examples/full.expected/with_git_patch_luci.json

 [
+  {
+    "cmd": [
+      "git",
+      "diff",
+      "--cached",
+      "--name-only"
+    ],
+    "cwd": "[START_DIR]/v8",
+    "infra_step": true,
+    "name": "git diff to analyze patch",
+    "stdout": "/path/to/tmp/",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@files@v8/foo.cc@@@",
+      "@@@STEP_LOG_END@files@@@",
+      "@@@SET_BUILD_PROPERTY@failure_type@\"INVALID_TEST_RESULTS\"@@@",
+      "@@@SET_BUILD_PROPERTY@subproject_tag@\"v8\"@@@"
+    ]
+  },
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "import sys; sys.exit(1)"
+    ],
+    "name": "fail",
+    "~followup_annotations": [
+      "step returned non-zero exit code: 1",
+      "@@@STEP_TEXT@foo@@@",
+      "@@@STEP_FAILURE@@@",
+      "@@@SET_BUILD_PROPERTY@failure_hash@\"c2311ad770732eade3d2f48247abd147e40a70e7\"@@@"
+    ]
+  },
   {
     "name": "$result",
-    "reason": "path[\"build\"] is not defined. Possibly this is a LUCI build. tryserver.apply_from_git is not supported in LUCI builds.",
+    "reason": "Step('fail') failed with return_code 1",
     "recipe_result": null,
     "status_code": 1
   }

recipes/recipe_modules/tryserver/examples/full.expected/with_rietveld_patch.json

 [
-  {
-    "cmd": [
-      "python",
-      "-u",
-      "RECIPE_PACKAGE_REPO[depot_tools]/apply_issue.py",
-      "-r",
-      "[START_DIR]",
-      "-i",
-      "12853011",
-      "-p",
-      "1",
-      "-s",
-      "https://codereview.chromium.org",
-      "--no-auth"
-    ],
-    "name": "apply_issue",
-    "~followup_annotations": [
-      "@@@STEP_LINK@Applied issue 12853011@https://codereview.chromium.org/12853011@@@"
-    ]
-  },
   {
     "cmd": [
       "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py",

recipes/recipe_modules/tryserver/examples/full.expected/with_rietveld_patch_new.json

 [
-  {
-    "cmd": [
-      "python",
-      "-u",
-      "RECIPE_PACKAGE_REPO[depot_tools]/apply_issue.py",
-      "-r",
-      "[START_DIR]",
-      "-i",
-      "12853011",
-      "-p",
-      "1",
-      "-s",
-      "https://codereview.chromium.org",
-      "--no-auth"
-    ],
-    "name": "apply_issue",
-    "~followup_annotations": [
-      "@@@STEP_LINK@Applied issue 12853011@https://codereview.chromium.org/12853011@@@"
-    ]
-  },
   {
     "cmd": [
       "RECIPE_PACKAGE_REPO[depot_tools]/git_cl.py",

recipes/recipe_modules/tryserver/examples/full.py

@@ -28,7 +28,6 @@ def RunSteps(api):
             'Foo', api.properties['patch_text']))])
     return
 
-  api.tryserver.maybe_apply_issue()
   if api.tryserver.can_apply_issue or api.tryserver.is_gerrit_issue:
     api.tryserver.get_footers()
   api.tryserver.get_files_affected_by_patch(