[git] Remove hard-coded "depot_tools" Git.

Several tools, including the "git" recipe module, hard-code a
checkout-relative "git.bat" path. Git is a feature that is provided by
the system, both to tooling and recipes:

1) For users, "depot_tools" must be on PATH, and during setup it will
   have installed "git.bat", ensuring that Git tooling is available in
   PATH.
2) For bots, the system is responsible for providing "git.bat" on PATH.
   This is typically done at "/b/depot_tools/git.bat", which is sync'd
   through the "update_scripts" step.

By formally treating Git as a system resource, we absolve Windows bots
and users from manually installing a depot_tools-local Git, bringing
them in line with other platforms.

BUG=chromium:590806
TEST=local

Change-Id: I93e89855cdd330a2ba7a8cfb8117a1789d1ab54e
Reviewed-on: https://chromium-review.googlesource.com/568694
Commit-Queue: Daniel Jacques <dnj@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

fetch.py

@@ -26,6 +26,8 @@ import subprocess
 import sys
 import textwrap
 
+import git_common
+
 from distutils import spawn
 
 
@@ -88,11 +90,8 @@ class GclientCheckout(Checkout):
 class GitCheckout(Checkout):
 
   def run_git(self, *cmd, **kwargs):
-    if sys.platform == 'win32' and not spawn.find_executable('git'):
+    print 'Running: git %s' % (' '.join(pipes.quote(x) for x in cmd))
-      git_path = os.path.join(SCRIPT_PATH, 'git.bat')
+    return git_common.run(*cmd, **kwargs)
-    else:
-      git_path = 'git'
-    return self.run((git_path,) + cmd, **kwargs)
 
 
 class GclientGitCheckout(GclientCheckout, GitCheckout):

git_common.py

@@ -37,9 +37,21 @@ from StringIO import StringIO
 
 ROOT = os.path.abspath(os.path.dirname(__file__))
 IS_WIN = sys.platform == 'win32'
-GIT_EXE = ROOT+'\\git.bat' if IS_WIN else 'git'
 TEST_MODE = False
 
+
+def win_find_git():
+  for elem in os.environ.get('PATH', '').split(os.pathsep):
+    for candidate in ('git.exe', 'git.bat'):
+      path = os.path.join(elem, candidate)
+      if os.path.isfile(path):
+        return path
+  raise ValueError('Could not find Git on PATH.')
+
+
+GIT_EXE = 'git' if not IS_WIN else win_find_git()
+
+
 FREEZE = 'FREEZE'
 FREEZE_SECTIONS = {
   'indexed': 'soft',

recipes/README.recipes.md

@@ -366,13 +366,11 @@ Returns:
 
 #### **class [GitApi](/recipes/recipe_modules/git/api.py#10)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
 
-— **def [\_\_call\_\_](/recipes/recipe_modules/git/api.py#17)(self, \*args, \*\*kwargs):**
+— **def [\_\_call\_\_](/recipes/recipe_modules/git/api.py#13)(self, \*args, \*\*kwargs):**
 
 Return a git command step.
 
-— **def [\_\_init\_\_](/recipes/recipe_modules/git/api.py#13)(self, \*args, \*\*kwargs):**
+— **def [bundle\_create](/recipes/recipe_modules/git/api.py#366)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
-
-— **def [bundle\_create](/recipes/recipe_modules/git/api.py#391)(self, bundle_path, rev_list_args=None, \*\*kwargs):**
 
 Run 'git bundle create' on a Git repository.
 
@@ -382,11 +380,11 @@ Args:
       refs in the Git checkout will be bundled.
   kwargs: Forwarded to '__call__'.
 
-— **def [cat\_file\_at\_commit](/recipes/recipe_modules/git/api.py#58)(self, file_path, commit_hash, remote_name=None, \*\*kwargs):**
+— **def [cat\_file\_at\_commit](/recipes/recipe_modules/git/api.py#38)(self, file_path, commit_hash, remote_name=None, \*\*kwargs):**
 
 Outputs the contents of a file at a given revision.
 
-— **def [checkout](/recipes/recipe_modules/git/api.py#130)(self, url, ref=None, dir_path=None, recursive=False, submodules=True, submodule_update_force=False, keep_paths=None, step_suffix=None, curl_trace_file=None, can_fail_build=True, set_got_revision=False, remote_name=None, display_fetch_size=None, file_name=None, submodule_update_recursive=True, use_git_cache=False):**
+— **def [checkout](/recipes/recipe_modules/git/api.py#110)(self, url, ref=None, dir_path=None, recursive=False, submodules=True, submodule_update_force=False, keep_paths=None, step_suffix=None, curl_trace_file=None, can_fail_build=True, set_got_revision=False, remote_name=None, display_fetch_size=None, file_name=None, submodule_update_recursive=True, use_git_cache=False):**
 
 Performs a full git checkout and returns sha1 of checked out revision.
 
@@ -423,7 +421,7 @@ Args:
 Returns: If the checkout was successful, this returns the commit hash of
   the checked-out-repo. Otherwise this returns None.
 
-— **def [config\_get](/recipes/recipe_modules/git/api.py#364)(self, prop_name, \*\*kwargs):**
+— **def [config\_get](/recipes/recipe_modules/git/api.py#339)(self, prop_name, \*\*kwargs):**
 
 Returns: (str) The Git config output, or None if no output was generated.
 
@@ -431,7 +429,7 @@ Args:
   prop_name: (str) The name of the config property to query.
   kwargs: Forwarded to '__call__'.
 
-— **def [count\_objects](/recipes/recipe_modules/git/api.py#66)(self, previous_result=None, can_fail_build=False, \*\*kwargs):**
+— **def [count\_objects](/recipes/recipe_modules/git/api.py#46)(self, previous_result=None, can_fail_build=False, \*\*kwargs):**
 
 Returns `git count-objects` result as a dict.
 
@@ -444,15 +442,11 @@ Args:
 Returns:
   A dict of count-object values, or None if count-object run failed.
 
-— **def [ensure\_win\_git\_tooling](/recipes/recipe_modules/git/api.py#39)(self):**
+— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#32)(self, remote_name=None, \*\*kwargs):**
-
-Ensures that depot_tools/git.bat actually exists.
-
-— **def [fetch\_tags](/recipes/recipe_modules/git/api.py#52)(self, remote_name=None, \*\*kwargs):**
 
 Fetches all tags from the remote.
 
-— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#381)(self, remote_name=None, \*\*kwargs):**
+— **def [get\_remote\_url](/recipes/recipe_modules/git/api.py#356)(self, remote_name=None, \*\*kwargs):**
 
 Returns: (str) The URL of the remote Git repository, or None.
 
@@ -460,11 +454,11 @@ Args:
   remote_name: (str) The name of the remote to query, defaults to 'origin'.
   kwargs: Forwarded to '__call__'.
 
-— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#336)(self, commit='HEAD', test_data=None, \*\*kwargs):**
+— **def [get\_timestamp](/recipes/recipe_modules/git/api.py#311)(self, commit='HEAD', test_data=None, \*\*kwargs):**
 
 Find and return the timestamp of the given commit.
 
-— **def [new\_branch](/recipes/recipe_modules/git/api.py#404)(self, branch, name=None, upstream=None, \*\*kwargs):**
+— **def [new\_branch](/recipes/recipe_modules/git/api.py#379)(self, branch, name=None, upstream=None, \*\*kwargs):**
 
 Runs git new-branch on a Git repository, to be used before git cl upload.
 
@@ -474,7 +468,7 @@ Args:
   upstream (str): to origin/master.
   kwargs: Forwarded to '__call__'.
 
-— **def [rebase](/recipes/recipe_modules/git/api.py#345)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
+— **def [rebase](/recipes/recipe_modules/git/api.py#320)(self, name_prefix, branch, dir_path, remote_name=None, \*\*kwargs):**
 
 Run rebase HEAD onto branch
 Args:

recipes/recipe_modules/git/api.py

@@ -10,18 +10,11 @@ from recipe_engine import recipe_api
 class GitApi(recipe_api.RecipeApi):
   _GIT_HASH_RE = re.compile('[0-9a-f]{40}', re.IGNORECASE)
 
-  def __init__(self, *args, **kwargs):
-    super(GitApi, self).__init__(*args, **kwargs)
-    self.initialized_win_git = False
-
   def __call__(self, *args, **kwargs):
     """Return a git command step."""
     name = kwargs.pop('name', 'git ' + args[0])
     infra_step = kwargs.pop('infra_step', True)
     git_cmd = ['git']
-    if self.m.platform.is_win:
-      self.ensure_win_git_tooling()
-      git_cmd = [self.package_repo_resource('git.bat')]
     options = kwargs.pop('git_config_options', {})
     for k, v in sorted(options.iteritems()):
       git_cmd.extend(['-c', '%s=%s' % (k, v)])
@@ -36,19 +29,6 @@ class GitApi(recipe_api.RecipeApi):
       else:
         return f.result
 
-  def ensure_win_git_tooling(self):
-    """Ensures that depot_tools/git.bat actually exists."""
-    if not self.m.platform.is_win or self.initialized_win_git:
-      return
-    with self.m.context(cwd=self.package_repo_resource()):
-      self.m.python(
-          'ensure git tooling on windows',
-          self.package_repo_resource('bootstrap', 'win', 'git_bootstrap.py'),
-          ['--verbose'],
-          infra_step=True,
-          timeout=300)
-    self.initialized_win_git = True
-
   def fetch_tags(self, remote_name=None, **kwargs):
     """Fetches all tags from the remote."""
     kwargs.setdefault('name', 'git fetch tags')
@@ -197,11 +177,6 @@ class GitApi(recipe_api.RecipeApi):
     else:
       remote_name = 'origin'
 
-    if self.m.platform.is_win:
-      self.ensure_win_git_tooling()
-      git_setup_args += [
-          '--git_cmd_path', self.package_repo_resource('git.bat')]
-
     step_suffix = '' if step_suffix is  None else ' (%s)' % step_suffix
     self.m.python(
         'git setup%s' % step_suffix,

recipes/recipe_modules/git/examples/full.expected/platform_win.json

@@ -1,16 +1,4 @@
 [
-  {
-    "cmd": [
-      "python",
-      "-u",
-      "RECIPE_PACKAGE_REPO[depot_tools]\\bootstrap\\win\\git_bootstrap.py",
-      "--verbose"
-    ],
-    "cwd": "RECIPE_PACKAGE_REPO[depot_tools]",
-    "infra_step": true,
-    "name": "ensure git tooling on windows",
-    "timeout": 300
-  },
   {
     "cmd": [
       "python",
@@ -19,15 +7,13 @@
       "--path",
       "[START_DIR]\\src",
       "--url",
-      "https://chromium.googlesource.com/chromium/src.git",
+      "https://chromium.googlesource.com/chromium/src.git"
-      "--git_cmd_path",
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat"
     ],
     "name": "git setup"
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "retry",
       "fetch",
       "origin",
@@ -43,7 +29,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "checkout",
       "-f",
       "FETCH_HEAD"
@@ -54,7 +40,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "rev-parse",
       "HEAD"
     ],
@@ -68,7 +54,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "clean",
       "-f",
       "-d",
@@ -80,7 +66,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "submodule",
       "sync"
     ],
@@ -90,7 +76,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "submodule",
       "update",
       "--init",
@@ -102,7 +88,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "-c",
       "foo=bar",
       "count-objects",
@@ -115,7 +101,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "config",
       "--get",
       "remote.origin.url"
@@ -130,7 +116,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "show",
       "HEAD",
       "--format=%at",
@@ -143,7 +129,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "fetch",
       "origin",
       "--tags"
@@ -154,7 +140,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "status"
     ],
     "cwd": "[START_DIR]\\src",
@@ -163,7 +149,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "status"
     ],
     "cwd": "[START_DIR]\\src",
@@ -172,7 +158,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "status"
     ],
     "cwd": "[START_DIR]\\src",
@@ -181,7 +167,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "new-branch",
       "refactor"
     ],
@@ -194,7 +180,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "new-branch",
       "feature",
       "--upstream",
@@ -209,7 +195,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "rebase",
       "origin/master"
     ],
@@ -219,7 +205,7 @@
   },
   {
     "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
+      "git",
       "bundle",
       "create",
       "[START_DIR]\\all.bundle",

recipes/recipe_modules/git/resources/git_setup.py

@@ -11,18 +11,17 @@ import os
 import subprocess
 import sys
 
+# Import "git_common" from "depot_tools" root.
+DEPOT_TOOLS_ROOT = os.path.abspath(os.path.join(
+    os.path.dirname(__file__), os.pardir, os.pardir, os.pardir, os.pardir))
+sys.path.insert(0, DEPOT_TOOLS_ROOT)
+import git_common
 
-def run_git(git_cmd, *args, **kwargs):
-  """Runs git with given arguments.
 
-  kwargs are passed through to subprocess.
+def run_git(*cmd, **kwargs):
-
+  kwargs['stdout'] = sys.stdout
-  If the kwarg 'throw' is provided, this behaves as check_call, otherwise will
+  kwargs['stderr'] = sys.stderr
-  return git's return value.
+  git_common.run(*cmd, **kwargs)
-  """
-  logging.info('Running: %s %s %s', git_cmd, args, kwargs)
-  func = subprocess.check_call if kwargs.pop('throw', True) else subprocess.call
-  return func((git_cmd,)+args, **kwargs)
 
 
 def main():
@@ -31,9 +30,6 @@ def main():
                       required=True)
   parser.add_argument('--url', help='URL of remote to make origin.',
                       required=True)
-  parser.add_argument('--git_cmd_path',
-                      help='Path to the git command to run.',
-                      default='git')
   parser.add_argument('--remote', help='Name of the git remote.',
                       default='origin')
   parser.add_argument('-v', '--verbose', action='store_true')
@@ -49,11 +45,10 @@ def main():
     os.makedirs(path)
 
   if os.path.exists(os.path.join(path, '.git')):
-    run_git(opts.git_cmd_path, 'config', '--remove-section',
+    run_git('config', '--remove-section', 'remote.%s' % remote, cwd=path)
-            'remote.%s' % remote, cwd=path)
   else:
-    run_git(opts.git_cmd_path, 'init', cwd=path)
+    run_git('init', cwd=path)
-  run_git(opts.git_cmd_path, 'remote', 'add', remote, url, cwd=path)
+  run_git('remote', 'add', remote, url, cwd=path)
   return 0
 
 

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

@@ -1,19 +1,7 @@
 [
   {
     "cmd": [
-      "python",
+      "git",
-      "-u",
-      "RECIPE_PACKAGE_REPO[depot_tools]\\bootstrap\\win\\git_bootstrap.py",
-      "--verbose"
-    ],
-    "cwd": "RECIPE_PACKAGE_REPO[depot_tools]",
-    "infra_step": true,
-    "name": "ensure git tooling on windows",
-    "timeout": 300
-  },
-  {
-    "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
       "diff",
       "--cached",
       "--name-only"

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

@@ -1,19 +1,7 @@
 [
   {
     "cmd": [
-      "python",
+      "git",
-      "-u",
-      "RECIPE_PACKAGE_REPO[depot_tools]\\bootstrap\\win\\git_bootstrap.py",
-      "--verbose"
-    ],
-    "cwd": "RECIPE_PACKAGE_REPO[depot_tools]",
-    "infra_step": true,
-    "name": "ensure git tooling on windows",
-    "timeout": 300
-  },
-  {
-    "cmd": [
-      "RECIPE_PACKAGE_REPO[depot_tools]\\git.bat",
       "diff",
       "--cached",
       "--name-only"