Prefer direct git installation for Windows

Bug: b/360206460
Change-Id: I7068b201b0b976ac619db16ff16d5ffbe5aea362
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5884196
Reviewed-by: Allen Li <ayatane@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Anne Redulla <aredulla@google.com>

bootstrap/bootstrap.py

@@ -40,8 +40,7 @@ class Template(
         collections.namedtuple('Template', (
             'PYTHON3_BIN_RELDIR',
             'PYTHON3_BIN_RELDIR_UNIX',
-            'GIT_BIN_RELDIR',
-            'GIT_BIN_RELDIR_UNIX',
+            'GIT_BIN_ABSDIR',
             'GIT_PROGRAM',
         ))):
     @classmethod
@@ -222,10 +221,10 @@ def _safe_rmtree(path):
 def clean_up_old_installations(skip_dir):
     """Removes Python installations other than |skip_dir|.
 
-    This includes an "in-use" check against the "python.exe" in a given directory
-    to avoid removing Python executables that are currently ruinning. We need
-    this because our Python bootstrap may be run after (and by) other software
-    that is using the bootstrapped Python!
+    This includes an "in-use" check against the "python.exe" in a given
+    directory to avoid removing Python executables that are currently running.
+    We need this because our Python bootstrap may be run after (and by) other
+    software that is using the bootstrapped Python!
     """
     root_contents = os.listdir(ROOT_DIR)
     for f in ('win_tools-*_bin', 'python27*_bin', 'git-*_bin',
@@ -246,6 +245,56 @@ def clean_up_old_installations(skip_dir):
 GIT_POSTPROCESS_VERSION = '2'
 
 
+def _within_depot_tools(path):
+    """Returns whether the given path is within depot_tools."""
+    return os.path.commonpath([os.path.abspath(path), ROOT_DIR]) == ROOT_DIR
+
+
+def _traverse_to_git_root(abspath):
+    """Traverses up the path to the closest "git" directory (case-insensitive).
+
+    Returns:
+        The path to the directory with name "git" (case-insensitive), if it
+        exists as an ancestor; otherwise, None.
+
+    Examples:
+      * "C:\Program Files\Git\cmd" -> "C:\Program Files\Git"
+      * "C:\Program Files\Git\mingw64\bin" -> "C:\Program Files\Git"
+    """
+    head, tail = os.path.split(abspath)
+    while tail:
+        if tail.lower() == 'git':
+            return os.path.join(head, tail)
+        head, tail = os.path.split(head)
+    return None
+
+
+def search_win_git_directory():
+    """Searches for a git directory outside of depot_tools.
+
+    As depot_tools will soon stop bundling Git for Windows, this function logs
+    a warning if git has not yet been directly installed.
+    """
+    # Look for the git command in PATH outside of depot_tools.
+    for p in os.environ.get('PATH', '').split(os.pathsep):
+        if _within_depot_tools(p):
+            continue
+
+        for cmd in ('git.exe', 'git.bat'):
+            if os.path.isfile(os.path.join(p, cmd)):
+                git_root = _traverse_to_git_root(p)
+                if git_root:
+                    return git_root
+
+    # Log deprecation warning.
+    logging.warning(
+        'depot_tools will soon stop bundling Git for Windows.\n'
+        'To prepare for this change, please install Git directly. See\n'
+        'https://chromium.googlesource.com/chromium/src/+/main/docs/windows_build_instructions.md#Install-git\n'
+    )
+    return None
+
+
 def git_get_mingw_dir(git_directory):
     """Returns (str) The "mingw" directory in a Git installation, or None."""
     for candidate in ('mingw64', 'mingw32'):
@@ -255,17 +304,19 @@ def git_get_mingw_dir(git_directory):
     return None
 
 
-def git_postprocess(template, git_directory):
-    # Update depot_tools files for "git help <command>"
-    mingw_dir = git_get_mingw_dir(git_directory)
-    if mingw_dir:
-        docsrc = os.path.join(ROOT_DIR, 'man', 'html')
-        git_docs_dir = os.path.join(mingw_dir, 'share', 'doc', 'git-doc')
-        for name in os.listdir(docsrc):
-            maybe_copy(os.path.join(docsrc, name),
-                       os.path.join(git_docs_dir, name))
-    else:
-        logging.info('Could not find mingw directory for %r.', git_directory)
+def git_postprocess(template, bootstrap_git_dir, add_docs):
+    if add_docs:
+        # Update depot_tools files for "git help <command>".
+        mingw_dir = git_get_mingw_dir(template.GIT_BIN_ABSDIR)
+        if mingw_dir:
+            docsrc = os.path.join(ROOT_DIR, 'man', 'html')
+            git_docs_dir = os.path.join(mingw_dir, 'share', 'doc', 'git-doc')
+            for name in os.listdir(docsrc):
+                maybe_copy(os.path.join(docsrc, name),
+                           os.path.join(git_docs_dir, name))
+        else:
+            logging.info('Could not find mingw directory for %r.',
+                         template.GIT_BIN_ABSDIR)
 
     # Create Git templates and configure its base layout.
     for stub_name, relpath in WIN_GIT_STUBS.items():
@@ -289,7 +340,8 @@ def git_postprocess(template, git_directory):
         _check_call(
             [git_bat_path, 'config', '--system', 'protocol.version', '2'])
 
-    call_if_outdated(os.path.join(git_directory, '.git_postprocess'),
+    os.makedirs(bootstrap_git_dir, exist_ok=True)
+    call_if_outdated(os.path.join(bootstrap_git_dir, '.git_postprocess'),
                      GIT_POSTPROCESS_VERSION, configure_git_system)
 
 
@@ -306,9 +358,7 @@ def main(argv):
     template = Template.empty()._replace(
         PYTHON3_BIN_RELDIR=os.path.join(args.bootstrap_name, 'python3', 'bin'),
         PYTHON3_BIN_RELDIR_UNIX=posixpath.join(args.bootstrap_name, 'python3',
-                                               'bin'),
-        GIT_BIN_RELDIR=os.path.join(args.bootstrap_name, 'git'),
-        GIT_BIN_RELDIR_UNIX=posixpath.join(args.bootstrap_name, 'git'))
+                                               'bin'))
 
     bootstrap_dir = os.path.join(ROOT_DIR, args.bootstrap_name)
 
@@ -316,7 +366,16 @@ def main(argv):
     clean_up_old_installations(bootstrap_dir)
 
     if IS_WIN:
-        git_postprocess(template, os.path.join(bootstrap_dir, 'git'))
+        bootstrap_git_dir = os.path.join(bootstrap_dir, 'git')
+        # Avoid messing with system git docs.
+        add_docs = False
+        git_dir = search_win_git_directory()
+        if not git_dir:
+            # git not found in PATH - fall back to depot_tools bundled git.
+            git_dir = bootstrap_git_dir
+            add_docs = True
+        template = template._replace(GIT_BIN_ABSDIR=git_dir)
+        git_postprocess(template, bootstrap_git_dir, add_docs)
         templates = [
             ('git-bash.template.sh', 'git-bash', ROOT_DIR),
             ('python3.bat', 'python3.bat', ROOT_DIR),

bootstrap/git-bash.template.sh

@@ -5,8 +5,13 @@ UNIX_BASE=`cygpath "$WIN_BASE"`
 export PATH="$PATH:$UNIX_BASE/${PYTHON3_BIN_RELDIR_UNIX}:$UNIX_BASE/${PYTHON3_BIN_RELDIR_UNIX}/Scripts"
 export PYTHON_DIRECT=1
 export PYTHONUNBUFFERED=1
+
+WIN_GIT_PARENT=`dirname "${GIT_BIN_ABSDIR}"`
+UNIX_GIT_PARENT=`cygpath "$WIN_GIT_PARENT"`
+BASE_GIT=`basename "${GIT_BIN_ABSDIR}"`
+UNIX_GIT="$UNIX_GIT_PARENT/$BASE_GIT"
 if [[ $# > 0 ]]; then
-  $UNIX_BASE/${GIT_BIN_RELDIR_UNIX}/bin/bash.exe "$@"
+  "$UNIX_GIT/bin/bash.exe" "$@"
 else
-  $UNIX_BASE/${GIT_BIN_RELDIR_UNIX}/git-bash.exe &
+  "$UNIX_GIT/git-bash.exe" &
 fi

bootstrap/git.template.bat

@@ -1,5 +1,10 @@
 @echo off
 setlocal
 if not defined EDITOR set EDITOR=notepad
-set PATH=%~dp0${GIT_BIN_RELDIR}\cmd;%~dp0;%PATH%
-"%~dp0${GIT_BIN_RELDIR}\${GIT_PROGRAM}" %*
+:: Exclude the current directory when searching for executables.
+:: This is required for the SSO helper to run, which is written in Go.
+:: Without this set, the SSO helper may throw an error when resolving
+:: the `git` command (see https://pkg.go.dev/os/exec for more details).
+set "NoDefaultCurrentDirectoryInExePath=1"
+set "PATH=${GIT_BIN_ABSDIR}\cmd;%~dp0;%PATH%"
+"${GIT_BIN_ABSDIR}\${GIT_PROGRAM}" %*

git_common.py

@@ -59,7 +59,7 @@ IS_WIN = sys.platform == 'win32'
 TEST_MODE = False
 
 
-def win_find_git():
+def win_find_git() -> str:
     for elem in os.environ.get('PATH', '').split(os.pathsep):
         for candidate in ('git.exe', 'git.bat'):
             path = os.path.join(elem, candidate)
@@ -70,16 +70,37 @@ def win_find_git():
                 # so we want to avoid it whenever possible, by extracting the
                 # path to git.exe from git.bat in depot_tools.
                 if candidate == 'git.bat':
-                    git_bat = open(path).readlines()
-                    new_path = os.path.join(elem, git_bat[-1][6:-5])
-                    if (git_bat[-1].startswith('"%~dp0')
-                            and git_bat[-1].endswith('" %*\n')
-                            and new_path.endswith('.exe')):
-                        path = new_path
+                    path = _extract_git_path_from_git_bat(path)
                 return path
     raise ValueError('Could not find Git on PATH.')
 
 
+def _extract_git_path_from_git_bat(path: str) -> str:
+    """Attempts to extract the path to git.exe from git.bat.
+
+    Args:
+        path: the absolute path to git.bat.
+
+    Returns:
+        The absolute path to git.exe if extraction succeeded,
+        otherwise returns the input path to git.bat.
+    """
+    with open(path, 'r') as f:
+        git_bat = f.readlines()
+        if git_bat[-1].endswith('" %*\n'):
+            if git_bat[-1].startswith('"%~dp0'):
+                # Handle relative path.
+                new_path = os.path.join(os.path.dirname(path),
+                                        git_bat[-1][6:-5])
+            elif git_bat[-1].startswith('"'):
+                # Handle absolute path.
+                new_path = git_bat[-1][1:-5]
+
+            if new_path.endswith('.exe'):
+                return new_path
+    return path
+
+
 GIT_EXE = 'git' if not IS_WIN else win_find_git()
 
 # The recommended minimum version of Git, as (<major>, <minor>, <patch>).

tests/git_common_test.inputs/testGitBatAbsolutePath/git.bat

@@ -0,0 +1,4 @@
+@echo off
+setlocal
+:: This is a test git.bat with an absolute path to git.exe.
+"C:\Absolute\Path\To\Git\cmd\git.exe" %*

tests/git_common_test.inputs/testGitBatNonExe/git.bat

@@ -0,0 +1,4 @@
+@echo off
+setlocal
+:: This is a test git.bat with an absolute path to git.cmd.
+"C:\Absolute\Path\To\Git\cmd\git.cmd" %*

tests/git_common_test.inputs/testGitBatRelativePath/git.bat

@@ -0,0 +1,4 @@
+@echo off
+setlocal
+:: This is a test git.bat with a relative path to git.exe.
+"%~dp0Relative\Path\To\Git\cmd\git.exe" %*

tests/git_common_test.inputs/testGitBatUnexpectedFormat/git.bat

@@ -0,0 +1,4 @@
+@echo off
+setlocal
+:: This is a test git.bat which does not forward all command line args.
+"C:\Absolute\Path\To\Git\cmd\git.exe"

tests/git_common_test.py

@@ -1319,6 +1319,42 @@ class RunWithStderr(GitCommonTestBase):
         self.assertEqual(run_mock.call_count, 1)  # 1 + 0 (retry)
 
 
+class ExtractGitPathFromGitBatTest(GitCommonTestBase):
+
+    def test_unexpected_format(self):
+        git_bat = os.path.join(DEPOT_TOOLS_ROOT, 'tests',
+                               'git_common_test.inputs',
+                               'testGitBatUnexpectedFormat', 'git.bat')
+        actual = self.gc._extract_git_path_from_git_bat(git_bat)
+        self.assertEqual(actual, git_bat)
+
+    def test_non_exe(self):
+        git_bat = os.path.join(DEPOT_TOOLS_ROOT, 'tests',
+                               'git_common_test.inputs', 'testGitBatNonExe',
+                               'git.bat')
+        actual = self.gc._extract_git_path_from_git_bat(git_bat)
+        self.assertEqual(actual, git_bat)
+
+    def test_absolute_path(self):
+        git_bat = os.path.join(DEPOT_TOOLS_ROOT, 'tests',
+                               'git_common_test.inputs',
+                               'testGitBatAbsolutePath', 'git.bat')
+        actual = self.gc._extract_git_path_from_git_bat(git_bat)
+        expected = 'C:\\Absolute\\Path\\To\\Git\\cmd\\git.exe'
+        self.assertEqual(actual, expected)
+
+    def test_relative_path(self):
+        git_bat = os.path.join(DEPOT_TOOLS_ROOT, 'tests',
+                               'git_common_test.inputs',
+                               'testGitBatRelativePath', 'git.bat')
+        actual = self.gc._extract_git_path_from_git_bat(git_bat)
+        expected = os.path.join(DEPOT_TOOLS_ROOT, 'tests',
+                                'git_common_test.inputs',
+                                'testGitBatRelativePath',
+                                'Relative\\Path\\To\\Git\\cmd\\git.exe')
+        self.assertEqual(actual, expected)
+
+
 if __name__ == '__main__':
     sys.exit(
         coverage_utils.covered_main(