git: Use Popen_safe to avoid encoding issues

2 files changed, 48 insertions, 44 deletions

diff --git a/mesonbuild/mesonlib.py b/mesonbuild/mesonlib.py
index 540d718..943aa42 100644
--- a/mesonbuild/mesonlib.py
+++ b/mesonbuild/mesonlib.py
@@ -72,38 +72,41 @@ class MesonException(Exception):
     lineno = None  # type: T.Optional[int]
     colno = None   # type: T.Optional[int]
 
-
 class EnvironmentException(MesonException):
     '''Exceptions thrown while processing and creating the build environment'''
 
+class GitException(MesonException):
+    def __init__(self, msg: str, output: T.Optional[str] = None):
+        super().__init__(msg)
+        self.output = output.strip() if output else ''
+
 GIT = shutil.which('git')
-def git(cmd: T.List[str], workingdir: str, **kwargs: T.Any) -> subprocess.CompletedProcess:
-    pc = subprocess.run([GIT, '-C', workingdir] + cmd,
-                        # Redirect stdin to DEVNULL otherwise git messes up the
-                        # console and ANSI colors stop working on Windows.
-                        stdin=subprocess.DEVNULL, **kwargs)
-    # Sometimes git calls git recursively, such as `git submodule update
-    # --recursive` which will be without the above workaround, so set the
-    # console mode again just in case.
-    mlog.setup_console()
-    return pc
+def git(cmd: T.List[str], workingdir: str, check: bool = False, **kwargs: T.Any) -> T.Tuple[subprocess.Popen, str, str]:
+    cmd = [GIT] + cmd
+    p, o, e = Popen_safe(cmd, cwd=workingdir, **kwargs)
+    if check and p.returncode != 0:
+        raise GitException('Git command failed: ' + str(cmd), e)
+    return p, o, e
 
-def quiet_git(cmd: T.List[str], workingdir: str) -> T.Tuple[bool, str]:
+def quiet_git(cmd: T.List[str], workingdir: str, check: bool = False) -> T.Tuple[bool, str]:
     if not GIT:
-        return False, 'Git program not found.'
-    pc = git(cmd, workingdir, universal_newlines=True,
-             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if pc.returncode != 0:
-        return False, pc.stderr
-    return True, pc.stdout
+        m = 'Git program not found.'
+        if check:
+            raise GitException(m)
+        return False, m
+    p, o, e = git(cmd, workingdir, check)
+    if p.returncode != 0:
+        return False, e
+    return True, o
 
 def verbose_git(cmd: T.List[str], workingdir: str, check: bool = False) -> bool:
     if not GIT:
+        m = 'Git program not found.'
+        if check:
+            raise GitException(m)
         return False
-    try:
-        return git(cmd, workingdir, check=check).returncode == 0
-    except subprocess.CalledProcessError:
-        raise WrapException('Git command failed')
+    p, _, _ = git(cmd, workingdir, check, stdout=None, stderr=None)
+    return p.returncode == 0
 
 def set_meson_command(mainfile: str) -> None:
     global python_command
diff --git a/mesonbuild/msubprojects.py b/mesonbuild/msubprojects.py
index 50ffb3e..1eecc17 100755
--- a/mesonbuild/msubprojects.py
+++ b/mesonbuild/msubprojects.py
@@ -2,7 +2,7 @@ import os, subprocess
 import argparse
 
 from . import mlog
-from .mesonlib import git, Popen_safe
+from .mesonlib import quiet_git, verbose_git, GitException, Popen_safe
 from .wrap.wrap import API_ROOT, Resolver, WrapException
 from .wrap import wraptool
 
@@ -42,13 +42,12 @@ def update_file(wrap, repo_dir, options):
     return True
 
 def git_output(cmd, workingdir):
-    return git(cmd, workingdir, check=True, universal_newlines=True,
-               stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout
+    return quiet_git(cmd, workingdir, check=True)[1]
 
 def git_stash(workingdir):
     # Don't pipe stdout here because we want the user to see his changes have
     # been saved.
-    git(['stash'], workingdir, check=True, universal_newlines=True)
+    verbose_git(['stash'], workingdir, check=True)
 
 def git_show(repo_dir):
     commit_message = git_output(['show', '--quiet', '--pretty=format:%h%n%d%n%s%n[%an]'], repo_dir)
@@ -58,10 +57,9 @@ def git_show(repo_dir):
 def git_rebase(repo_dir, revision):
     try:
         git_output(['-c', 'rebase.autoStash=true', 'rebase', 'FETCH_HEAD'], repo_dir)
-    except subprocess.CalledProcessError as e:
-        out = e.output.strip()
+    except GitException as e:
         mlog.log('  -> Could not rebase', mlog.bold(repo_dir), 'onto', mlog.bold(revision))
-        mlog.log(mlog.red(out))
+        mlog.log(mlog.red(e.output))
         mlog.log(mlog.red(str(e)))
         return False
     return True
@@ -72,10 +70,9 @@ def git_reset(repo_dir, revision):
         # avoid any data lost by mistake.
         git_stash(repo_dir)
         git_output(['reset', '--hard', 'FETCH_HEAD'], repo_dir)
-    except subprocess.CalledProcessError as e:
-        out = e.output.strip()
+    except GitException as e:
         mlog.log('  -> Could not reset', mlog.bold(repo_dir), 'to', mlog.bold(revision))
-        mlog.log(mlog.red(out))
+        mlog.log(mlog.red(e.output))
         mlog.log(mlog.red(str(e)))
         return False
     return True
@@ -89,10 +86,9 @@ def git_checkout(repo_dir, revision, create=False):
         # avoid any data lost by mistake.
         git_stash(repo_dir)
         git_output(cmd, repo_dir)
-    except subprocess.CalledProcessError as e:
-        out = e.output.strip()
+    except GitException as e:
         mlog.log('  -> Could not checkout', mlog.bold(revision), 'in', mlog.bold(repo_dir))
-        mlog.log(mlog.red(out))
+        mlog.log(mlog.red(e.output))
         mlog.log(mlog.red(str(e)))
         return False
     return True
@@ -126,7 +122,14 @@ def update_git(wrap, repo_dir, options):
     # Fetch only the revision we need, this avoids fetching useless branches and
     # is needed for http case were new remote branches wouldn't be discovered
     # otherwise. After this command, FETCH_HEAD is the revision we want.
-    git_output(['fetch', 'origin', revision], repo_dir)
+    try:
+        git_output(['fetch', 'origin', revision], repo_dir)
+    except GitException as e:
+        mlog.log('  -> Could not fetch revision', mlog.bold(revision), 'in', mlog.bold(repo_dir))
+        mlog.log(mlog.red(e.output))
+        mlog.log(mlog.red(str(e)))
+        return False
+
     if branch == '':
         # We are currently in detached mode
         if options.reset:
@@ -234,17 +237,15 @@ def foreach(wrap, repo_dir, options):
     if not os.path.isdir(repo_dir):
         mlog.log('  -> Not downloaded yet')
         return True
-    try:
-        out = subprocess.check_output([options.command] + options.args,
-                                      stderr=subprocess.STDOUT,
-                                      cwd=repo_dir).decode()
-        mlog.log(out, end='')
-    except subprocess.CalledProcessError as e:
-        err_message = "Command '{}' returned non-zero exit status {}.".format(" ".join(e.cmd), e.returncode)
-        out = e.output.decode()
+    cmd = [options.command] + options.args
+    p, out, _ = Popen_safe(cmd, stderr=subprocess.STDOUT, cwd=repo_dir)
+    if p.returncode != 0:
+        err_message = "Command '{}' returned non-zero exit status {}.".format(" ".join(cmd), p.returncode)
         mlog.log('  -> ', mlog.red(err_message))
         mlog.log(out, end='')
         return False
+
+    mlog.log(out, end='')
     return True
 
 def add_common_arguments(p):