Avoid computing patch_root in get_files_affected_by_patch.

Context: See https://codereview.chromium.org/1934533002/#msg19 for context.

Allows: https://codereview.chromium.org/1933223002

Next - testing that it works for skia, upgrading old users to new method,
and finally killing old code.

R=machenbach@chromium.org,rmistry@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/1927403003

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

recipe_modules/tryserver/api.py

@@ -148,7 +148,40 @@ class TryserverApi(recipe_api.RecipeApi):
       # Since this method is "maybe", we don't raise an Exception.
       pass
 
-  def get_files_affected_by_patch(self):
+  def get_files_affected_by_patch(self, patch_root=None):
+    """Returns list of paths to files affected by the patch.
+
+    Argument:
+      patch_root: path relative to api.path['root'], usually obtained from
+        api.gclient.calculate_patch_root(patch_project)
+
+    Returned paths will be relative to to patch_root.
+
+    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.
+    """
+    # patch_root must be set! None is for backwards compataibility and will be
+    # removed.
+    if patch_root is None:
+      return self._old_get_files_affected_by_patch()
+    step_result = self.m.git('diff', '--cached', '--name-only',
+                             cwd=self.m.path['slave_build'].join(patch_root),
+                             name='git diff to analyze patch',
+                             stdout=self.m.raw_io.output(),
+                             step_test_data=lambda:
+                               self.m.raw_io.test_api.stream_output('foo.cc'))
+    paths = [self.m.path.join(patch_root, p) for p in
+             step_result.stdout.split()]
+    if self.m.platform.is_win:
+      # Looks like "analyze" wants POSIX slashes even on Windows (since git
+      # uses that format even on Windows).
+      paths = [path.replace('\\', '/') for path in paths]
+    step_result.presentation.logs['files'] = paths
+    return paths
+
+
+  def _old_get_files_affected_by_patch(self):
     git_diff_kwargs = {}
     issue_root = self.m.rietveld.calculate_issue_root()
     if issue_root:

recipe_modules/tryserver/example.expected/with_rietveld_patch_new.json

+[
+  {
+    "cmd": [
+      "python",
+      "-u",
+      "[DEPOT_TOOLS]/apply_issue.py",
+      "-r",
+      "[SLAVE_BUILD]",
+      "-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": [
+      "git",
+      "diff",
+      "--cached",
+      "--name-only"
+    ],
+    "cwd": "[SLAVE_BUILD]/sub/project",
+    "name": "git diff to analyze patch",
+    "stdout": "/path/to/tmp/",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@files@sub/project/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": "Step('fail') failed with return_code 1",
+    "status_code": 1
+  }
+]
\ No newline at end of file

recipe_modules/tryserver/example.expected/with_wrong_patch_new.json

+[
+  {
+    "cmd": [
+      "RECIPE_PACKAGE_REPO[depot_tools]\\bootstrap\\win\\win_tools.bat"
+    ],
+    "cwd": "RECIPE_PACKAGE_REPO[depot_tools]",
+    "name": "ensure git tooling on windows"
+  },
+  {
+    "cmd": [
+      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "diff",
+      "--cached",
+      "--name-only"
+    ],
+    "cwd": "[SLAVE_BUILD]\\sub\\project",
+    "name": "git diff to analyze patch",
+    "stdout": "/path/to/tmp/",
+    "~followup_annotations": [
+      "@@@STEP_LOG_LINE@files@sub/project/foo.cc@@@",
+      "@@@STEP_LOG_END@files@@@"
+    ]
+  },
+  {
+    "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": "Step('fail') failed with return_code 1",
+    "status_code": 1
+  }
+]
\ No newline at end of file

recipe_modules/tryserver/example.py

@@ -14,7 +14,8 @@ DEPS = [
 def RunSteps(api):
   api.path['checkout'] = api.path['slave_build']
   api.tryserver.maybe_apply_issue()
-  api.tryserver.get_files_affected_by_patch()
+  api.tryserver.get_files_affected_by_patch(
+      api.properties.get('test_patch_root'))
 
   if api.tryserver.is_tryserver:
     api.tryserver.set_subproject_tag('v8')
@@ -43,3 +44,10 @@ def GenTests(api):
          api.properties.tryserver())
 
   yield (api.test('with_wrong_patch') + api.platform('win', 32))
+
+
+  yield (api.test('with_rietveld_patch_new') +
+         api.properties.tryserver(test_patch_root='sub/project'))
+
+  yield (api.test('with_wrong_patch_new') + api.platform('win', 32) +
+         api.properties(test_patch_root='sub\\project'))